Commit Graph

4398 Commits

Author SHA1 Message Date
Gerald Monaco 456f18be2e refactor(core): add internal API to enable i18n hydration (#54784)
Add an internal API to enable and use i18n hydration for testing and development. This helps ensure that we don't accidentally break the current behavior until we are completely ready to roll out i18n support.

PR Close #54784
2024-03-12 11:38:59 -07:00
Paul Gschwendtner 1294b9a02a refactor(core): report subscription errors for OutputEmitterRef to ErrorHandler (#54821)
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
2024-03-12 10:21:48 -07:00
Andrew Scott 8735af08b9 feat(router): Add ability to return UrlTree with NavigationBehaviorOptions from guards (#45023)
Returning `UrlTree` from a guard was a convenient new feature added to
the `Router`. However, it does not have feature-parity with the old
`router.navigate(...); return false;` pattern. The most common use-case
for this feature is to redirect to a new page _without_ updating the URL
from the initially attempted navigation. For example, rendering a 404
page when the user does not have access privelages to a route.

Fixes #17004
Fixes #27148

BREAKING CHANGE: Guards can now return `RedirectCommand` for redirects
in addition to `UrlTree`. Code which expects only `boolean` or `UrlTree`
values in `Route` types will need to be adjusted.

PR Close #45023
2024-03-12 09:19:14 -07:00
cexbrayat 87cae55e55 docs: typo in output documentation (#54773)
PR Close #54773
2024-03-12 09:15:04 -07:00
Kristiyan Kostadinov 018f8266b3 fix(core): ensure all initializer functions run in an injection context (#54761)
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
2024-03-12 09:08:06 -07:00
Pawel Kozlowski 47f79e78c0 refactor(core): assert presence of the track function (#54814)
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
2024-03-11 16:33:01 -07:00
Paul Gschwendtner 142825d7b1 perf(core): speed up retrieval of DestroyRef in EventEmitter (#54748)
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
2024-03-11 16:31:03 -07:00
Andrew Scott 64f870c12b fix(core): ApplicationRef.tick should respect OnPush for host bindings (#53718) (#53718)
This commit updates `ApplicationRef.tick` to use `detectChangesInternal` for root
views rather than go through the `ChangeDetectorRef.detectChanges` API
which refreshes the host view without first looking at whether the view
is `OnPush` and not dirty. The current behavior would hide errors in
`OnPush` components that do not correctly get marked for check and would
break when migrating to zoneless change detection because `markForCheck`
was never called so change detection was never scheduled.
The error would be surprising and blamed on switching to zoneless when in
reality the issue already exists and is a problem with the component not
calling `markForCheck`. However, this error is hidden today because
`ApplicationRef.tick` refresh host bindings unconditionally.

BREAKING CHANGE: `OnPush` views at the root of the application need to
be marked dirty for their host bindings to refresh. Previously, the host
bindings were refreshed for all root views without respecting the
`OnPush` change detection strategy.

PR Close #53718

PR Close #53718
2024-03-11 13:46:11 -07:00
Andrew Kushnir eaff724b77 fix(core): prevent infinite loops in clobbered elements check (#54425)
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
2024-03-11 12:46:16 -07:00
Gerald Monaco 280a3a2d62 refactor(core): add i18nNodes in preparation for i18n hydration (#54750)
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
2024-03-11 11:12:43 -07:00
Kristiyan Kostadinov 5ae2bf4806 fix(compiler): handle two-way bindings to signal-based template variables in instruction generation (#54714)
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
2024-03-11 11:01:43 -07:00
Kristiyan Kostadinov 5b927c094d build: update to TypeScript 5.4 stable (#54743)
Updates the repo to the stable version of TypeScript 5.4.

PR Close #54743
2024-03-11 09:16:55 -07:00
JoostK d269c88ae4 refactor(core): avoid additional closure for queued microtask callback (#54801)
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
2024-03-11 08:59:55 -07:00
Andrew Scott 37d1f713a4 Revert "fix(core): ApplicationRef.tick should respect OnPush for host bindings (#53718)" (#54774)
This reverts commit d888da4606.
2024-03-08 05:57:15 -08:00
Andrew Scott 2787c50107 refactor(core): render hooks should always run on node attach or detach (#54083)
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
2024-03-07 12:59:10 -08:00
Andrew Scott d4247af8c5 refactor(core): Skip refresh views if render hooks are the only notification source (#54083)
Do not refresh views if the only thing that notified the scheduler was
registration of a new render hook.

PR Close #54083
2024-03-07 12:59:10 -08:00
Andrew Scott 53e743938f refactor(core): registering afterRender hooks notify scheduler (#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
2024-03-07 12:59:09 -08:00
Gerald Monaco 43ab781b70 refactor(core): refactor i18n node creation to prepare for hydration (#54722)
Adds a `locateOrCreateNode` helper and uses it for all DOM operations in preparation for supporting hydration.

PR Close #54722
2024-03-07 12:41:43 -08:00
Andrew Scott d888da4606 fix(core): ApplicationRef.tick should respect OnPush for host bindings (#53718)
This commit updates `ApplicationRef.tick` to use `detectChangesInternal` for root
views rather than go through the `ChangeDetectorRef.detectChanges` API
which refreshes the host view without first looking at whether the view
is `OnPush` and not dirty. The current behavior would hide errors in
`OnPush` components that do not correctly get marked for check and would
break when migrating to zoneless change detection because `markForCheck`
was never called so change detection was never scheduled.
The error would be surprising and blamed on switching to zoneless when in
reality the issue already exists and is a problem with the component not
calling `markForCheck`. However, this error is hidden today because
`ApplicationRef.tick` refresh host bindings unconditionally.

BREAKING CHANGE: `OnPush` views at the root of the application need to
be marked dirty for their host bindings to refresh. Previously, the host
bindings were refreshed for all root views without respecting the
`OnPush` change detection strategy.

PR Close #53718
2024-03-07 11:13:03 -08:00
Gerald Monaco e0c1f1e183 refactor(core): don't hydrate detached nodes (#54723)
In preparation for hydration support, make sure that we don't attempt to hydrate detached nodes and instead, always newly create them.

PR Close #54723
2024-03-07 11:01:29 -08:00
Gerald Monaco 9104c62ab3 refactor(core): add i18n AST to prepare for hydration (#54724)
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
2024-03-07 11:00:36 -08:00
Andrew Scott cd242a11f5 refactor(core): Remove isInternal flag (#54740)
This is not needed any longer since the changes that used it have
landed.

PR Close #54740
2024-03-07 09:07:01 -08:00
Andrew Scott ad045efd4b fix(core): Ensure views marked for check are refreshed during change detection (#54735)
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.

fixes #52928
fixes #15634

BREAKING CHANGE: Newly created and views marked for check and reattached
during change detection are now guaranteed to be refreshed in that same
change detection cycle. Previously, if they were attached at a location
in the view tree that was already checked, they would either throw
`ExpressionChangedAfterItHasBeenCheckedError` or not be refreshed until
some future round of change detection. In rare circumstances, this
correction can cause issues. We identified one instance that relied on
the previous behavior by reading a value on initialization which was
queued to be updated in a microtask instead of being available in the
current change detection round. The component only read this value during
initialization and did not read it again after the microtask updated it.

PR Close #54735
2024-03-06 15:44:09 -08:00
Andrew Scott ba8e465974 fix(core): Change Detection will continue to refresh views while marked for check (#54734)
When the `ApplicationRef` refreshes attached views, it will continue to
do so while there is still one marked for check after the refresh
completes.

BREAKING CHANGE: When Angular runs change detection, it will continue to
refresh any views attached to `ApplicationRef` that are still marked for
check after one round completes. In rare cases, this can result in infinite
loops when certain patterns continue to mark views for check using
`ChangeDetectorRef.detectChanges`. This will be surfaced as a runtime
error with the `NG0103` code.

PR Close #54734
2024-03-06 15:43:16 -08:00
Andrew Scott 24bc0ed4f2 fix(core): ComponentFixture autodetect should detect changes within ApplicationRef.tick (#54733)
The current behavior of `autoDetect` in `ComponentFixture` does not
match production very well. It has several flaws that make it an
insufficient change detection mechanism:

* It runs change detection for the component under test _after_ views
  attached to the `ApplicationRef`. This can cause real behavior
  differences that break in production, because tests can observe view
  refreshes in the incorrect order (for example, a dialog refreshing
  before the component which opened it).
* Because of the above ordering, render hooks registered during change
  detection of the fixture views _will not execute at all_ because
  `ApplicationRef.tick` already happen.
* It does not rerun change detection on the view tree if there are more
  dirty views to refresh after the render hooks complete.
* It effectively hides/swallows errors from change detection inside the
  `onMicrotaskEmpty` subscription by not reporting them to the error
  handler. Instead, this error ends up being unhandled in the
  subscription and rxjs throws these in a `setTimeout`.

All of the above are problematic but this commit _does not_ fix the
final point. Ideally, we can land this in a future change but this
requires additional internal fixes. In the meantime, we have to juggle
special-case handling of the component fixture views within
`ApplicationRef.tick` using some special events to retain current
behavior and avoid errors from the fixture propagating to the `ErrorHandler`.

BREAKING CHANGE: The `ComponentFixture.autoDetect` feature now executes
change detection for the fixture within `ApplicationRef.tick`. This more
closely matches the behavior of how a component would refresh in
production. The order of component refresh in tests may be slightly
affected as a result, especially when dealing with additional components
attached to the application, such as dialogs. Tests sensitive to this
type of change (such as screenshot tests) may need to be updated.
Concretely, this change means that the component will refresh _before_
additional views attached to `ApplicationRef` (i.e. dialog components).
Prior to this change, the fixture component would refresh _after_ other
views attached to the application.

PR Close #54733
2024-03-06 13:33:54 -08:00
Matthieu Riegler 7243c704cf fix(core): return a readonly signal on asReadonly. (#54706)
Previous `asReadonly()` returned the signal value and not the signal itself.

Fixes #54704

PR Close #54706
2024-03-06 14:41:12 +01:00
Paul Gschwendtner d4154f9e3c refactor(core): revert listener subscription typeof check (#54650)
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
2024-03-06 12:34:39 +01:00
Paul Gschwendtner 500a13e77d refactor(core): enforce model() and output() is used in an injection context (#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
2024-03-06 12:34:39 +01:00
Paul Gschwendtner db7962adb2 test: add runtime tests for output() function API (#54650)
Adds runtime acceptance tests for `output()` and
`outputFromObservable()`.

PR Close #54650
2024-03-06 12:34:39 +01:00
Paul Gschwendtner 866271a1c6 refactor(core): EventEmitter implements OutputRef. (#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
2024-03-06 12:34:39 +01:00
Paul Gschwendtner 30355f6719 refactor(core): model() implements OutputRef (#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
2024-03-06 12:34:38 +01:00
Paul Gschwendtner aff65fd1f4 feat(core): introduce outputToObservable interop helper (#54650)
This commit introduces an addition to `output()` and
`outputFromObservable`()` —called `outputToObservable()`.

The helper lives in the RxJS interop package and allows agnostic
programmatic subscriptions to `OutputRef`s by converting the output
to an observable with `.pipe` etc.

The function is ideally used in all places where you subscribe to an
output programmatically. Those outputs in the future, with the new APIs,
may not be actual RxJS constructs, but abstract `OutputRef`'s that
simply expose a `.subscribe` method. The helper allows you to
agnostically convert outputs to RxJS observables that you can safely
interact with.

The observables are also completed automatically, if possible, when the
owning directive/component is destroyed— Something that is not
guaranteed right now.

PR Close #54650
2024-03-06 12:34:38 +01:00
Paul Gschwendtner c809069f21 feat(core): introduce outputFromObservable() interop function (#54650)
Introduces a second API in addition to the new `output()` function.

The new function `outputFromObservable()` can be used to declare outputs
using the new `OutputRef` API and `output()` API, while using a custom
RxJS observable as data source.

This is something that is currently possible in Angular and we would
like to keep possible- even though we never intended to support custom
observables aside from RxJS-based `EventEmitter`.

The interop bridges the gap and allows you to continue using
`Subject`, `ReplaySubject`, `BehaivorSubjct,` - or cold custom
observables for outputs. You can still trigger logic only when
the output is subscribed- unlike with imperative `emit`s of
`EventEmitter` or the new `OutputEmitterRef`.

A notable difference is that you need two class members where you
previously could access the `Subject` directly. This is an intentional
trade-off we've made to ensure that all new outputs implement the
`OutputRef` interface and we are exposing a minimal API surface to
consumers of components that currently access the output
programmatically.

PR Close #54650
2024-03-06 12:34:38 +01:00
Paul Gschwendtner c687b8f453 feat(core): expose new output() API (#54650)
This commit exposes the new `output()` API with numerous benefits:

- Symmetrical API to `input()`, `model()` etc.
- Fixed types for `EventEmitter.emit`— current `emit` method of
  `EventEmitter` is broken and accepts `undefined` via `emit(value?: T)`
- Removal of RxJS specific concepts from outputs. error channels,
  completion channels etc. We now have a simple consistent
  interface.
- Automatic clean-up of subscribers upon directive/component destory-
  when subscribed programmatically.

```ts
@Directive({..})
export class MyDir {
  nameChange = output<string>();     // OutputEmitterRef<string>
  onClick = output();                // OutputEmitterRef<void>
}
```

Note: RxJS custom observable cases will be handled in future commits via
explicit helpers from the interop.

PR Close #54650
2024-03-06 12:34:38 +01:00
Paul Gschwendtner 9ce627746c refactor(core): create OutputRef runtime construct (#54650)
This commit creates the proposed `OutputRef` interface along
with `OutputEmitterRef`:

- `OutputRef` is the consistent interface for all Angular outputs.
- `OutputEmitterRef` is an extension for emitting values. Like
  `EventEmitter`.
- subscriptions are cleaned up automatically upon directive/component
  destroy.
- emitting is an error when destroyed
- subscribing programmatically is an error when already destroyed.

This commit will also implement the shared output ref runtime construct,
that can be used by `output()`, `outputFromObservable()` and `model()`.

We will manage subscriptions in a simple way, manually, without RxJS.

PR Close #54650
2024-03-06 12:34:38 +01:00
Paul Gschwendtner 2564b45b47 test: replace fake_core with real @angular/core output (#54650)
This commit replaces `fake_core` with the real `@angular/core`
output. See previous commit for reasons.

Overall, this commit:

* Replaces references of `fake_core`
* Fixes tests that were testing Angular compiler detection that _would_
  already be flagged by type-checking of TS directly. We keep these
  tests for now, and add `@ts-ignore` to verify the Angular checks, in
  case type checking is disabled in user applications- but it's worth
  considering to remove these tests. Follow-up question/non-priority.
* Adds `@ts-ignore` to the tests for `defer` 1P because the property is
  marked as `@internal` and now is (correctly) causing failures in the
  compiler test environment.
* Fixes a couple of tests with typos, wrong properties etc that
  previously weren't detected! A good sign.

PR Close #54650
2024-03-06 12:34:38 +01:00
Matthieu Riegler 40424fc4b6 docs: fix label on API link. (#54621)
Remove the unecessary double quotes on a @see link.

PR Close #54621
2024-03-05 18:35:04 +01:00
Andrew Kushnir 33a6fab094 fix(core): apply TestBed provider overrides to @defer dependencies (#54667)
This commit updates TestBed logic to take into account situations when dependencies loaded within `@defer` blocks may import NgModules with providers. For such components, we invoke provider override function, which recursively inspects and applies the necessary changes.

PR Close #54667
2024-03-05 09:21:22 +01:00
Paul Gschwendtner 78e69117f0 fix(core): generic inference for signal inputs may break with --strictFunctionTypes (#54652)
This commit fixes that the generic inference for signal inputs may break
with `--strictFunctionTypes`.

We are not using --strictFunctionTypes` in all tests, so function parameters are always
checked bivariantly- while in 1P function parameters are checked contravariantly. This
breaks sub-typing and e.g. `InputSignal<number>` is not a subtype of `InputSignalWithTransform<unknown, number>`.

Root cause is that the input signal captures the equality function that
is exclusively concerned with the `ReadT`. And `ReadT` is never equal, or a
supertype of unknown.

https://www.typescriptlang.org/play?#code/KYDwDg9gTgLgBASwHY2FAZgQwMbDgSSTAFcYBlBAcyUwBsB1BGACwBUpMkBndaAWwA8AJWCYAJqwA0celCbBWAPjgBvAFBw4Aei1xWACXxlEXOCzxQIEeNkzEuwAIRmAnmDxhMHPsFRRTXnjYzMDYANbAYnDYECgcAG5eCJwwtC7SAHIA8qxwAEYIALSJcilpAHQacMAAjsR0AFxwABSYTSLiUvntohIAlHAAvMp5VrSiSADcalUA+rNQvaw9ndOa8wDucqjLMtsK0wC+aqCQsIgoaFi4BESkFNR0wkuKVaCoSGKmhCTkVDQMJhsDjcXhQQQdCTSSFKVTHNQxbjwEBNH73f50RgsdicHj8ATEJBhJAQDZIaRIYh8PJoZSDKqU2i0ZyYb53P6PWgCSnU2nTKpaABUM00rDceAA5GiOQDuVSaVBFBKTHASfBWVwMXlxmYIK53HApeyHgCscDcWDBITiaTyar5bSJZVReLTBB0HAwJZ3LAXIbavVaMrAhcYnxPDAENrgM7NHpxYbWk0eQrpHlkw6oANhvkxhNlQhTGq4BqtTqYHqYAmJUm4NaSWS00167bsyM85wnVU4-H3G6PZ5vL40KYJZhg59DeOS4tQxBw5hI9HYz2XQaJS2yQWi9YS1xNY9o7r9ZKU2gnc0AEwzDTZVgAUSaABF7wAFe8ZF8ZXJZDJwAAGhSFFwMByNgMAAGKEuBCCxGKfb-nA6C0JglArqugpaEAA

PR Close #54652
2024-03-04 19:07:39 +01:00
Andrew Scott 07173fd011 refactor(core): Pending effects should make the application unstable (#53835)
In zone-full applications, this is already true because effects are
scheduled inside a microtask and tracked by ZoneJS. When not using
zones, this should stay consistent on principle and for testability
reasons. A general pattern with zoneless testing will be to update state
and `await` some promise (i.e. `fixture.whenStable`, which will be
linked to `ApplicationRef.isStable`).

PR Close #53835
2024-03-04 10:35:38 +01:00
Andrew Scott 66d78a7dcc refactor(core): ComponentFixture autodetect should detect changes within ApplicationRef.tick (#54354)
The current behavior of `autoDetect` in `ComponentFixture` does not
match production very well. It has several flaws that make it an
insufficient change detection mechanism:

* It runs change detection for the component under test _after_ views
  attached to the `ApplicationRef`. This can cause real behavior
  differences that break in production, because tests can observe view
  refreshes in the incorrect order (for example, a dialog refreshing
  before the component which opened it).
* Because of the above ordering, render hooks registered during change
  detection of the fixture views _will not execute at all_ because
  `ApplicationRef.tick` already happen.
* It does not rerun change detection on the view tree if there are more
  dirty views to refresh after the render hooks complete.
* It effectively hides/swallows errors from change detection inside the
  `onMicrotaskEmpty` subscription by not reporting them to the error
  handler. Instead, this error ends up being unhandled in the
  subscription and rxjs throws these in a `setTimeout`.

All of the above are problematic but this commit _does not_ fix the
final point. Ideally, we can land this in a future change but this
requires additional internal fixes. In the meantime, we have to juggle
special-case handling of the component fixture views within
`ApplicationRef.tick` using some special events to retain current
behavior and avoid errors from the fixture propagating to the `ErrorHandler`.

breaking note for future when isG3 flag condition is removed for v18:
The `ComponentFixture.autoDetect` feature now executes
change detection for the fixture within `ApplicationRef.tick`. This more
closely matches the behavior of how a component would refresh in
production. The order of component refresh in tests may be slightly
affected as a result, especially when dealing with additional components
attached to the application, such as dialogs. Tests sensitive to this
type of change (such as screenshot tests) may need to be updated.

PR Close #54354
2024-02-29 20:53:09 -08:00
Swami b3220795d5 refactor(core): remove unused EffectRef import statement (#53863)
Remove unused, leftover imports.

PR Close #53863
2024-02-29 14:54:50 +01:00
Alex Rickabaugh ffad7b8ea9 fix(core): untrack various core operations (#54614)
One downside of implicit dependency tracking in `effect()`s is that it's easy
to for downstream code to end up running inside the effect context by accident.
For example, if an effect raises an event (e.g. by `next()`ing a `Subject`), the
subscribers to that `Observable` will run inside the effect's reactive context,
and any signals read within the subscriber will end up as dependencies of the
effect. This is why the `untracked` function is useful, to run certain
operations without incidental signal reads ending up tracked.

However, knowing when this is necessary is non-trivial. For example, injecting
a dependency might cause it to be instantiated, which would run the constructor
in the effect context unless the injection operation is untracked.

Therefore, Angular will automatically drop the reactive context within a number
of framework APIs. This commit addresses these use cases:

* creating and destroying views
* creating and destroying DI injectors
* injecting dependencies
* emitting outputs

Fixes #54548

There are likely other APIs which would benefit from this approach, but this
is a start.

PR Close #54614
2024-02-29 11:38:54 +01:00
Kristiyan Kostadinov fb540e169a feat(core): add migration for invalid two-way bindings (#54630)
As a part of #54154, an old parser behavior came up where two-way bindings were parsed by appending `= $event` to the event side. This was problematic, because it allowed some non-writable expressions to be passed into two-way bindings.

These changes introduce a migration that will change the two-way bindings into two separate input/output bindings that represent the old behavior so that in a future version we can throw a parser error for the invalid expressions.

```ts
// Before
@Component({
  template: `<input [(ngModel)]="a && b"/>`
})
export class MyComp {}

// After
@Component({
  template: `<input [ngModel]="a && b" (ngModelChange)="a && (b = $event)"/>`
})
export class MyComp {}
```

PR Close #54630
2024-02-28 18:06:23 +01:00
Kristiyan Kostadinov 331b16efd2 feat(core): add API to inject attributes on the host node (#54604)
Angular has the `@Attribute` decorator that allows for attributes to be injected from the host node, but we don't have an equivalent for the `inject` function. These changes introduce the new `HostAttributeToken` class that can be used to inject attributes similarly to `@Attribute`. It can be used as follows:

```typescript
import {HostAttributeToken, inject} from '@angular/core';

class MyDir {
  someAttr = inject(new HostAttributeToken('some-attr'));
}
```

The new API works similarly to `@Attribute` with one key exception: it will throw a DI error when the attribute doesn't exist, instead of returning `null` like `@Attribute`. We made this change to align its behavior closer to other injection tokens.

PR Close #54604
2024-02-27 15:18:41 -08:00
Andrew Scott 53820dd896 refactor(core): Internal render hooks trigger view refresh before executing user hooks (#54224)
This commit ensures that any internal render hooks that cause views to
become dirty again first refresh those dirty views before running user
render hooks. This ensures that user render hooks have the most complete
render state possible and stops them from needlessly executing multiple
times. This is important to maintain the goal of the public render
hooks, which is to provide the safest place to place code that depends
on DOM state, especially in ways that may force a browser paint.

PR Close #54224
2024-02-26 18:31:13 -08:00
Andrew Scott 46617ce44e refactor(core): Add helper function for queuing state updates (#54224)
This adds a helper function to defer application state updates to the
first possible "safe" moment. If application-wide change detection
(ApplicationRef.tick) is currently executing when this function is used,
the callback will execute as soon as all views attached to the
`ApplicationRef` have been refreshed. Refreshing the application views
will happen again before `checkNoChanges` executes.

When a change detection is _not_ running, this state update will execute
in the microtask queue.

This function is necessary as a replacement for current
`Promise.resolve().then(() => stateUpdate())` to be zoneless compatible
while ensuring those state updates are synchronized to the DOM before
the browser repaint. Without this, updates done in `Promise.resolve(...)` would
queue another round of change detection in zoneless applications, and
this change detection could happen in the next browser frame, and cause
noticeable flicker for the user.

Additionally, this function provides a way to perform state updates that
will run on the server as well as in the browser.

Last, current applications using `ngZone: 'noop'` may not be
calling `ApplicationRef.tick` at all so this function provides a
mechanism to ensure the state update still happens by racing
a microtask with `afterNextRender` (which might never execute).

PR Close #54224
2024-02-26 18:31:13 -08:00
Kristiyan Kostadinov f5c566c079 fix(compiler-cli): identify aliased initializer functions (#54609)
Fixes that initializer functions weren't being recognized if they are aliased (e.g. `import {model as alias} from '@angular/core';`).

To do this efficiently, I had to introduce the `ImportedSymbolsTracker` which scans the top-level imports of a file and allows them to be checked quickly, without having to go through the type checker. It will be useful in the future when verifying that that initializer APIs aren't used in unexpected places.

I've also introduced tests specifically for the `tryParseInitializerApiMember` function so that we can test it in isolation instead of going through the various functions that call into it.

PR Close #54609
2024-02-26 18:27:15 -08:00
Dylan Hunn d4343b53de Revert "fix(compiler-cli): identify aliased initializer functions (#54480)" (#54595)
This reverts commit f04ecc0cda.

PR Close #54595
2024-02-26 08:36:49 -08:00
Andrew Kushnir dcb9deb363 fix(core): collect providers from NgModules while rendering @defer block (#52881)
Currently, when a `@defer` block contains standalone components that import NgModules with providers, those providers are not available to components declared within the same NgModule. The problem is that the standalone injector is not created for the host component (that hosts this `@defer` block), since dependencies become defer-loaded, thus no information is available at host component creation time.

This commit updates the logic to collect all providers from all NgModules used as a dependency for standalone components used within a `@defer` block. When an instance of a defer block is created, a new environment injector instance with those providers is created.

Resolves #52876.

PR Close #52881
2024-02-23 12:30:05 -08:00