Currently if an `(output)` listener fails, it will be handled gracefully
by Angular and reported to the `ErrorHandler`.
For programmatic subscriptions with `OutputEmitterRef`, this is not the case.
Instead, as soon as any subscription is failing, all other subsequent
subscription callbacks are not firing anymore.
This commit intends to make this more consistent by gracefully
reporting errors from `OutputEmitterRef#emit` to `ErrorHandler`,
allowing for listener execution to continue.
PR Close#54821
Ensures that all of the functions intended to be run in initializers are in an injection context. This is a stop-gap until we have a compiler diagnostic for it.
PR Close#54761
This commits assert that the repeater instruction gets a reference
to a tracking function. This change will allow us to better track
occurences of https://github.com/angular/angular/issues/53628 -
in certain situations a reference to a tracking function might be
undefiened.
We are not fixing the underlying issue here, just getting better
visibility.
PR Close#54814
Speeds up the retrieval of `DestroyRef` in `EventEmitter` because
`try/catch` is expensive if there is no injection context.
We saw a script time regression in Cloud.
The goldens had to be updated because `getInjectImplementation` is now
referenced. `inject` also references the underlying field, but directly.
This is super minimal overhead of a function exposing the internal
field.
PR Close#54748
This commit updates HTML sanitization logic to avoid infinite loops in case clobbered elements contain fields like `nextSibling` or `parentNode`. Those fields are used for DOM traversal and this update makes sure that those calls return valid results.
Also this commit fixes an issue when clobbering `nodeName` causes JS exceptions.
PR Close#54425
An i18n message effectively acts as a dynamic template: two elements with contiguous instruction indices won't necessarily be contiguous in the DOM.
For that reason, we need to maintain a mapping from instruction index to a physical DOM node in order to hydrate views with i18n, pointing to where hydration for that view should begin.
PR Close#54750
Updates the instruction generation for two-way bindings to only emit the `twoWayBindingSet` call when writing to template variables. Since template variables are constants, it's only allowed to write to them when they're signals. Non-signal values are flagged during template type checking.
Fixes#54670.
PR Close#54714
We have a diagnostic that reports writes to template variables which worked both for regular event bindings and two-way bindings, however the latter was broken by #54154 because two-way bindings no longer had a `PropertyWrite` AST.
These changes fix the diagnostic and expand it to allow two-way bindings to template variables that are signals.
PR Close#54714
Moves the check which ensures that there are no writes to template variables into the `TemplateSemanticsChecker` to prepare for the upcoming changes.
PR Close#54714
Introduces a new `TemplateSemanticsChecker` that will be used to flag semantic errors in the user's template. Currently we do some of this in the type check block, but the problem is that it doesn't have access to the template type checker which prevents us from properly checking cases like #54670. This pass is also distinct from the extended template checks, because we don't want users to be able to turn the checks off and we want them to run even if `strictTemplates` are disabled.
PR Close#54714
The `runCallbackOnce` closure is declared not to have any parameters itself, so it is
compatible as `queueMicrotask` callback without the extra closure. This reduces the call
stack by a frame and avoids the extra closure allocation.
PR Close#54801
This commit addresses a typing mismatch, where these functions were declared to return whichever
value their callback returned, but this was inaccurate: it's always a test callback function
with `done` argument.
PR Close#54801
The assertion in `packages/core/test/acceptance/after_render_hook_spec.ts:165` was prone to flakes,
where Jasmine could frequently report an error:
```
Error: 'expect' was used when there was no current spec, this could be because an asynchronous test timed out
at Env.expect (node_modules/jasmine-core/lib/jasmine-core/jasmine.js:1945:15)
at expect (node_modules/jasmine-core/lib/jasmine-core/jasmine.js:8267:18)
at file:///packages/core/test/acceptance/after_render_hook_spec.ts:165:12
```
This happens because `wrapTestFn` checks for an exact type of `Promise`, which may have been patched by zone.js
such that the `instanceof` condition is dependent on whether zone.js has patched the `Promise` constructor.
PR Close#54801
There is an edge case where synchronous navigations caused in
response to navigation events can result in a previous navigation not
being unsubscribed from. b/328219996
PR Close#54710
This commit ensures that render hooks are rerun when a node is attached
or detached. We do not necessarily need to run change detection but DOM
did change so render hooks should execute.
PR Close#54083
This commit updates the `afterRender` and `afterNextRender` hooks to
notify the scheduler (which subsequently schedules change detection)
when created. This makes the hooks similar to `requestAnimationFrame`,
which requests that the browser schedule a rendering operation. This
reqeust is not conditional. Even if there was nothing to repaint, the
`requestAnimationFrame` callback will execute.
In Angular, this is useful because callers of `afterNextRender` don't
necessarily have any way of knowing whether a change detection is even
scheduled. For example, the anchor scrolling with the Angular Router
needs to wait for rendering to complete before attempting to scroll
because rendering can affect the size of the page. However, if the user
is already on the page that the navigation is targeting, such as
navigating to an anchor on the page, there is nothing new for the Router
to render so a render might not even be scheduled.
Related to https://github.com/angular/angular/issues/53985, which
could use `afterNextRender` instead of `setTimeout` to ensure the
scrolling happens in the same frame as the page rendering, but would not
necessarily work without this change (as described above). Note that the
scrolling _cannot_ use a microtask to ensure scrolling happens in the
same frame because `NgZone` will ensure microtasks flush before
change detection, so it would cause the scroll to happen before rendering.
PR Close#54083
In order to serialize and hydrate i18n blocks, we need to be able to walk an AST for the translated message. This AST is generated during normal parsing of the message.
PR Close#54724
For model signals we introduced some sniffing on the return type of a
`.subscribe` invocation- allowing for subscribe to _just_ return a
callback directly to unsubscribe.
This works in practice, but the positive `tCleanup` indices have more
meaning, especially in the context of `DebugElement`. A positive index
indicates a DOM event- so we need to revert this change. This now
surfaced as we made `EventEmitter` return a function + the subscription
via a proxy that ended up `typeof function` --> and broke some tests
where debug element incorrectly invoked non-dom outputs as dom
listeners. We don't need this change with current unsubscribe function
concept.
PR Close#54650
Currently the `makeProgram` utility from `ngtsc/testing` does not use
the test host by default- optimizing for source file caching.
Additionally, the host can be updated to attempt caching of the `.d.ts`
files from `@angular/core`— whether that's fake core, or the real core-
is irrelevant. We are never caching if these changes between tests, so
correctness is guaranteed.
This commit reduces the type check test times form 80s to just 11
seconds, faster than what it was before with `fake_core`. The ngtsc
tests also run significantly faster. From 40s to 30s
PR Close#54650
The `inject` global augmentation from upgrade tests, leak into
all source files for IDEs, making it easy to run into issues
when actually trying to deal with `inject` from Angular core for DI.
PR Close#54650
Technically `model()` and `output()` already need to be defined in an
injection context- because `OutputRef` requires this.
To improve the error messaging, this commit asserts this as part of the
top-level entry functions for `model()` and `output()`. Without this
change, the error would mention the `_createOutputRef` internal
function.
PR Close#54650
An `EventEmitter` is a construct owned by Angular that should be
used for outputs as of right now.
As we are introducing the new `OutputRef` interface for the new output
function APIs, we also think `EventEmitter` should implement
`OutputRef`— ensuring all "known" outputs follow the same contract.
This commit ensures `EventEmitter` implements an `OutputRef`
Note: An output ref captures the destroy ref from the current injection
context for clean-up purposes. This is also done for `EventEmitter` in a
backwards compatible way:
- not requiring an injection context. EventEmitter may be used
elsewhere.
- not cleaning up subscriptions/completing the emitter when the
directive/component is destroyed. This would be a change in behavior.
Note 2: The dependency on `DestroyRef` causes it to be retained in all
bundling examples because ironically `NgZone` uses `EventEmitter`- not
for outputs. The code is pretty minimal though, so that should be
acceptable.
`EventEmitter` will now always retain `NgZone. This increases the
payload size slightly around 800b for AIO. Note that the other increases
were coming from previous changes. This commit just pushed it over the
threshold.
PR Close#54650
A model signal is technically an output, at runtime and conceptually.
This commit re-uses the shared output ref logic and ensures the
interfaces match.
PR Close#54650
Paul Gschwendtner