In some situations, calling `markForCheck` can result in an infinite
loop in seemingly valid scenarios. When a transplanted view is inserted
before its declaration, it gets refreshed in the retry loop of
`detectChanges`. At this point, the `Dirty` flag has been cleared from
all parents. Calling `markForCheck` marks the insertion tree up to the
root `Dirty`. If the declaration is checked again as a result (i.e.
because it has default change detection) and is reachable because its
parent was marked `Dirty`, this can cause an infinite loop. The
declaration is refreshed again, so the insertion is marked for refresh
(again). We enter an infinite loop if the insertion tree always calls
`markForCheck` for some reason (i.e. `{{createReplayObservable() | async}}`).
While the case above does fall into an infinite loop, it also truly is a
problem in the application. While it's not an infinite synchronous loop,
the declaration and insertion are infinitely dirty and will be refreshed
on every change detection round.
Usually `markForCheck` does not have this problem because the `Dirty`
flag is not cleared until the very end of change detection. However, if
the view did not already have the `Dirty` flag set, it is never cleared
because we never entered view refresh. One solution to this problem
could be to clear the `Dirty` flag even after skipping view refresh but
traversing to children.
PR Close#54329
This test ensures that the `ExpressionChanged...` error does not happen
when signals are updated in a view that is attached to `ApplicationRef`
but was already checked. This was fixed in 432afd1ef4
which actually consequently fixes it for regular `markForCheck` as well.
PR Close#54206
It was always the intent to have `afterRender` hooks allow updating
state, as long as those state updates specifically mark views for
(re)check. This commit delivers that behavior. Developers can now use
`afterRender` hooks to perform further updates within the same change
detection cycle rather than needing to defer this work to another round
(i.e. `queueMicrotask(() => <<updateState>>)`). This is an important
change to support migrating from the `queueMicrotask`-style deferred
updates, which are not entirely compatible with zoneless or event `NgZone` run
coalescing.
When using a microtask to update state after a render, it
doesn't work with coalescing because the render may not have happened
yet (coalescing and zoneless use `requestAnimationFrame` while the
default for `NgZone` is effectively a microtask-based change detection
scheduler). Second, when using a `microtask` _during_ change detection to defer
updates to the next cycle, this can cause updates to be split across
multiple frames with run coalescing and with zoneless (again, because of
`requestAnimationFrame`/`macrotask` change detection scheduling).
PR Close#54074
Node removal is immediate and does not require change detection to run
when animations are not provided. This refactor makes the animation
engine notify the scheduler rather than doing it on all node removals.
PR Close#53857
Enables signal inputs for existing Zone based components.
This is a next step we are taking to bring signal inputs earlier to the Angular community.
The goal is to enable early access for the ecosystem to signal inputs, while we are continuing
development of full signal components as outlined in the RFC. This will allow the ecosystem
to start integrating signals more deeply, prepare for future migrations, and improves code quality
and DX for existing components (especially for OnPush).
Based on our work on full signal components, we've gathered more information and learned
new things. We've improved the API by introducing a way to intuitively declare required inputs,
as well as improved the API around initial values. We even support non-primitive initial values
as the first argument to the `input` function now.
```ts
@Directive({..})
export class MyDir {
firstName = input<string>(); // string|undefined
lastName = input.required<string>(); // string
age = input(0); // number
```
PR Close#53872
This allows us to ensure signal inputs and a potential JIT transform
remain single file compilation compatible. The consequences are that
options need to be statically analyzable more strictly, compared to
loosened restrictions with static interpretation where e.g. `alias`
can be defined through a shared variable.
PR Close#53872
Adds infrastructure to run signal input tests with JIT (using the
transform) and AOT. Acceptance tests for signal inputs will run with
both variants. In the future we can consider expanding this
infrastructure for all of our acceptance tests, but that's a different
story.
PR Close#53808
The `afterRender` hooks currently run after `ApplicationRef.tick` but
also run after any call to `ChangeDetectorRef.detectChanges`. This is
problematic because code which uses `afterRender` cannot expect the
component it's registered from to be rendered when the callback
executes. If there is a call to `ChangeDetectorRef.detectChanges` before
the global change detection, that will cause the hooks to run earlier
than expected.
This behavior is somewhat of a blocker for the zoneless project. There
is plenty of application code that do things like `setTimeout(() =>
doSomethingThatExpectsComponentToBeRendered())`, `NgZone.onStable(() =>
...)` or `ApplicationRef.onStable...`. `ApplicationRef.onStable` is a
should likely work similarly, but all of these are really wanting an API
that is `afterRender` with the requirement that the hook runs after the
global render, not an individual CDRef instance.
This change updates the `afterRender` hooks to only run when
`ApplicationRef.tick` happens.
fixes#52429fixes#53232
PR Close#52455
Currently when a base class defines an input with a transform, derived
classes re-defining the input via `@Input`, or `inputs: [<..>]`, end up
inherting the transform due to a bug in the inherit definitions feature.
This commit fixes this. We verified in the Google codebase that this is
an unlikely occurrence and it's trivial to fix on user side by removing
the re-declaration/override, or explictly adding the necessary
transform.
Conceptually, the behavior was quite inconsistent as everything else of
inputs was overridden as expected. i.e. alias, required state etc. The
exception were input transforms. This commit fixes this.
PR Close#53571
The InitialRenderPendingTasks currently attempts to only contribute to
ApplicationRef stableness one time to support SSR. This isn't actually
how the switchMap works in reality. This commit updates
the isStable observable to be more clear that it's always a combination
of the zone stableness and pending tasks.
In addition, this commit renames the service to just be PendingTasks
because it doesn't directly relate to rendering. While the purpose is
to track things that might cause rendering to happen, we don't know if the
tasks will affect rendering at all.
PR Close#53534
The version of rxjs used to build the repository has been updated to v7.
This required only minimal changes to the code. Most of which were type
related only due to more strict types in v7. The behavior in those cases
was left intact. The most common type related change was to handle the
possibility of `undefined` with `toPromise` which was always possible with
v6 but the types did not reflect the runtime behavior. The one change that
was not type related was to provide a parameter value to the `defaultIfEmpty`
operator. It no longer defaults to a value of `null` if no default is provided.
To provide the same behavior the value of `null` is now passed to the operator.
PR Close#53500
When a view has the `Dirty` flag and is reattached, we should ensure that it is
reached and refreshed during the next change detection run from above.
In addition, when a view is created and attached, we should ensure that it is reached
and refreshed during change detection. This can happen if the view is
created and attached outside a change run or when it is created and
attached after its insertion view was already checked. In both cases, we
should ensure that the view is reached and refreshed during either the
current change detection or the next one (if change detection is not
already running).
We can achieve this by creating all views with the `Dirty` flag set.
However, this does happen to be a breaking change in some scenarios.
The one identified internally was actually depending on change detection
_not_ running immediately because it relied on an input value that was
set using `ngModel`. Because `ngModel` sets its value in a `Promise`, it
is not available until the _next_ change detection cycle. Ensuring
created views run in the current change change detection will result in
different behavior in this case.
Making option the default is the solution to #52928. That will have to
wait for a major version.
PR Close#53022
The `base64-js` package was only used in tests that were run only on
Node.js. On Node.js, `Buffer` is available which can natively perform
base64 conversion. By using `Buffer in these Node.js only tests, the
`base64-js` package can be removed from the repository.
PR Close#53464
This change fixes and issue where the expectation was that change
detection always goes through `detectChangesInView`. In reality,
`detectChangesInternal` directly calls `refreshView`
and refreshes a view directly without checking if it was dirty (to my discontent).
This update changes the implementation of `detectChangesInternal` to
actually be "detect changes" not "force refresh of root view and detect
changes". In addition, it adds the refresh flag to APIs that were
previously calling `detectChangesInternal` so we get the same behavior
as before (host view is forced to refresh).
Note that the use of `RefreshView` instead of `Dirty` is _intentional_
here. The `RefreshView` flag is cleared before refreshing the view while
the `Dirty` flag is cleared at the very end. Using the `Dirty` flag
could have consequences because it is a more long-lasting change to the
view flags. Because `detectChangesInView` will immediately clear the
`RefreshView` flag, this change is much more limited and does not
result in a different set of flags during the view refresh.
PR Close#53021
This commit fixes a memory leak where signal consumers would not be cleaned up for
descendant views when a view is destroyed, because the cleanup logic was only invoked
for the view that is itself being destroyed.
PR Close#53351
The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.
PR Close#53190
Adds support for inheriting host directives from the parent class. This is consistent with how we inherit other features like host bindings.
Fixes#51203.
PR Close#52992
Related to #52928 but `updateAncestorTraversalFlagsOnAttach` is called
on view insertion and _should_ have made that work for views dirty from
signals but it wasn't updated to read the `dirty` flag when we changed
it from sharing the `RefreshView` flag.
For #52928, we've traditionally worked under the assumption that this is working
as expected. The created view is `CheckAlways`. There is a question of whether we
should automatically mark things for check when the attached view has
the `Dirty` flag and/or has the `FirstLViewPass` flag set (or other
flags that indicate it definitely needs to be prefreshed).
PR Close#53001
The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.
PR Close#52726
Reworks the `repeater` instruction to go through `advance`, instead of passing in the index directly. This ensures that lifecycle hooks run at the right time and that we don't throw "changed after checked" errors when we shouldn't be.
Fixes#52885.
PR Close#52935
These tests ensure signals can be read in a template after embedded
views are created in the middle of template execution of an update pass.
The embedded view templates are executed in create mode in the middle of
the component template being executed in update mode. This behavior was
found to not work correctly in past implementations of the reactive
template consumers.
PR Close#52495
Previously we had logic for a special case where a root injector in standalone apps would skip the import paths calculation step for the `getEnvironmentInjectorProviders` function.
This commit intends to fix this for two other cases, namely:
- When an injector is created by a route (via the `providers` field and lazy loading).
- When an injector is manually created and attached to the injector tree
It does this by assuming that any environment injector it cannot find a provider imports container for was created without one, and simply returns the raw provider records without the import paths calculation.
PR Close#52774
This change fixes a bug in the new list reconcilation algorithm
that could lead to an infinite loop in certain situations.
More specifically, it adjusts the internal MultiMap implementation
such that an entry returned from the .get call is the same entry
(for an identical key) removed by the .delete call.
The existing logic of the MultiMap was leading to a situation where
one view was requested and attached to LContainer, but a very different
view was removed from the MultiMap. This was leaving an attached LView
in a collection that was supposed to hold only detached views.
Closes#52524
PR Close#52697
`RootViewRef<T>` extends `ViewRef<T>` and overrides 3 methods with behavior
that is identical to `ViewRef<T>`. This commit removes `RootViewRef<T>`
because it is not needed.
PR Close#52430
Discovered this while validating #52414 against Angular Material. We were projecting `<ng-template>` nodes at the root of `@if` and `@for` with the `ng-template` tag name which enables directive matching and applies the directive to the control flow node.
These changes fix the issue by never passing along the `ng-template` tag name.
PR Close#52515
Previously, LViews were used here to be consistent with other debug APIs. Using LViews for tracking injector providers does not work because providers only get configured once per TNode type.
Now we use the TNode as the key to track element injector providers, allowing the injector for each item rendered in a list (`ngFor` or `@for`) to be targeted with debug APIs for inspecting providers
PR Close#52436
Producers represent values which can deliver change notifications.
When a producer value is changed, a change notification is propagated through the graph,
notifying live consumers which depend on the producer of the potential update.
Note here that this is a _potential_ update.
A producer may not have actually "changed" based on its equality function. With
this commit, before refreshing a view that is only marked for refresh
because its consumer is dirty, we poll producers for change to see if
they really have. If not, we can skip the refresh. The example test in this commit
shows that a `computed` which depends on a `signal` that is updated but
produces a value that is the same as before will _not_ cause the
component's template to refresh.
fixes#51797
PR Close#52476
This commit updates the reactive consumer used for `LView`s to be shared
between a component and its embedded views. This allows us to use the
consumer flag directly for a dirty indicator rather than needing to
find a component view for updating its flags.
In the future, this will also allow us to effectively poll producers to see if
they really changed before refreshing a view.
PR Close#52476
The significance of the combination of #51854 and #52302 went mostly
unnoticed. The first removed a unidirectional data flow constraint for
transplanted views and the second updated the signal implementation to
share transplanted view logic. The result is that we automatically get behavior
that (mostly) removes `ExpressionChangedAfterItWasCheckedError` when signals are
used to drive application state to DOM synchronization.
fixes#50320
PR Close#52476
When an effect is created in a component constructor, it might read signals
which are derived from component inputs. These signals may be unreliable or
(in the case of the proposed input signals) may throw if accessed before the
component is first change detected (which is what makes required inputs
available).
Depending on the scenario involved, the effect may or may not run before
this initialization takes place, which isn't a great developer experience.
In particular, effects created during CD (e.g. via control flow) work fine,
as do effects created in bootstrap thanks to the sync CD it performs. When
an effect is created through dynamic component creation outside of CD though
(such as on router navigations), it runs before the component is first CD'd,
causing the issue.
In fact, in the signal components RFC we described how effects would wait
until ngOnInit for their first execution for exactly this reason, but this
behavior was never implemented as it was thought our effect scheduling
design made it unnecessary. This is true of the regular execution of effects
but the above scenario shows that *creation* of the effect is still
vulnerable. Thus, this logic is needed.
This commit makes effects sensitive to their creation context, by injecting
`ChangeDetectorRef` optionally. An effect created with an injector that's
tied to a component will wait until that component is initialized before
initially being scheduled. TestBed effect flushing is also adjusted to
account for the additional interaction with change detection.
PR Close#52473
With the directive-based control flow users were able to conditionally project content using the `*` syntax. E.g. `<div *ngIf="expr" projectMe></div>` will be projected into `<ng-content select="[projectMe]"/>`, because the attributes and tag name from the `div` are copied to the template via the template creation instruction. With `@if` and `@for` that is not the case, because the conditional is placed *around* elements, rather than *on* them. The result is that content projection won't work in the same way if a user converts from `*ngIf` to `@if`.
These changes aim to cover the most common case by doing the same copying when a control flow node has *one and only one* root element or template node.
This approach comes with some caveats:
1. As soon as any other node is added to the root, the copying behavior won't work anymore. A diagnostic will be added to flag cases like this and to explain how to work around it.
2. If `preserveWhitespaces` is enabled, it's very likely that indentation will break this workaround, because it'll include an additional text node as the first child. We can work around it here, but in a discussion it was decided not to, because the user explicitly opted into preserving the whitespace and we would have to drop it from the generated code. The diagnostic mentioned point #1 will flag such cases to users.
Fixes#52277.
PR Close#52414
The `ViewRef<T>` interface extends `InternalViewRef` and is already not
part of the public API. There is no need for the extra `InternalViewRef`
interface. This confusing setup is likely leftover from the types
necessary to support both Ivy and ViewEngine.
PR Close#52430
This commit updates the reactive template and host binding consumers to
only mark their declaration components for refresh, but not parents/ancestors.
This also updates the `AfterViewChecked` hook to run when a component is
refreshed during change detection but its host is not. It is reasonable
to expect that the `ngAfterViewChecked` lifecycle hook will run when a
signal updates and the component is refreshed. The hooks are typically
run when the host is refreshed so without this change, the update to
not mark ancestors dirty would have caused `ngAfterViewChecked` to not
run.
resolves#14628resolves#22646resolves#34347 - this is not the direct request of the issue but
generally forcing change detection to run is necessary only because a
value was updated that needs to be synced to the DOM. Values that use
signals will mark the component for check automatically so accessing the
`ChangeDetectorRef` of a child is not necessary. The other part of this
request was to avoid the need to "mark all views for checking since
it wouldn't affect anything but itself". This is directly addressed by
this commit - updating a signal that's read in the view's template
will not cause ancestors/"all views" to be refreshed.
PR Close#52302
Issue #50320 shows that in some cases, updating a signal that's a dependency
of a template during change detection of that template can have several
adverse effects. This can happen, for example, if the signal is set during
the lifecycle hook of a directive within the same template that reads the
signal.
This can cause a few things to happen:
* Straightforwardly, it can cause `ExpressionChanged` errors.
* Surprisingly, it can cause an assertion within the `ReactiveLViewConsumer`
to fail.
* Very surprisingly, it can cause change detection for an `OnPush` component
to stop working.
The root cause of these later behaviors is subtle, and is ultimately a
desync between the reactive graph and the view tree's notion of "dirty" for
a given view. This will be fixed with further work planned for change
detection to handle such updates directly. Until then, this commit improves
the DX through two changes:
1. The mechanism of "committing" `ReactiveLViewConsumer`s to a view is
changed to use the `consumerOnSignalRead` hook from the reactive graph.
This prevents the situation which required the assertion in the first
place.
2. A `console.warn` warning is added when a view is marked dirty via a
signal while it's still executing.
The warning informs users that they're pushing data against the direction of
change detection, risking `ExpressionChanged` or other issues. It's a
warning and not an error because the check is overly broad and captures
situations where the application would not actually break as a result, such
as if a `computed` marked the template dirty but still returned the same
value.
PR Close#52234
Previously this case was missed by the default framework injector profiler. Now in ngDevMode this event emits correctly when a service is configured with `providedIn`. This includes the case where injection tokens are configured with a `providedIn`.
This commit also includes unit tests for this new case in the injector profiler.
PR Close#52365
This commit runs change detection in a loop while there are still dirty
views to be refreshed in the tree. At the moment, this only applies to
transplanted views but will also apply to views with changed signals.
fixes angular#49801
PR Close#51854
This commit updates the code to report errors via `ErrorHandler` instance.
For dependency loading problems, errors are reported only when `@error` block is not provided.
PR Close#52320
This commit adds the logic to cleanup all triggers once defer block is triggered.
When a trigger is created, its cleanup function is stored alongside other defer block info. Prefetch and regular triggers are store in different slots, since we need to invoke them at different time.
PR Close#52291
Public afterRender phases have specific API guarantees which can be invalidated if the internal framework is implemented using them. Instead, the framework should use dedicated internal functions.
PR Close#52145
Andrew Scott