Commit graph

324 commits

Author SHA1 Message Date
cexbrayat
31fb9d0d52 docs: mention autoDetectChanges parameter default value (#58092)
It was unclear whether the parameter was necessary, as its default value was not mentioned in the jdsoc.

PR Close #58092
2024-10-09 13:50:32 +00:00
Joey Perrott
03ac3c299d refactor: update license text to point to angular.dev (#57902)
Update license text to point to angular.dev instead of angular.io

PR Close #57902
2024-09-24 15:28:46 +02:00
Andrew Scott
7919982063 feat(core): Add whenStable helper on ApplicationRef (#57190)
This commit adds a `whenStable` function to `ApplicationRef` to cover
the most common use-case for the `isStable` observable.

PR Close #57190
2024-08-06 21:28:16 +00:00
Andrew Scott
f7918f5272 feat(core): Add 'flush' parameter option to fakeAsync to flush after the test (#57239)
From the internal issue on the matter:

> When using the standard Jasmine version of it promises returned by the body function are automatically awaited. The Catalyst version of it is fake-async, so awaiting the promise does not make sense; however it would be nice if Catalyst automatically flushed the promise to replicate the experience of using standard it. This would allow users to do the following:

```
it('should fail later', async () => {
  await new Promise(r => setTimeout(r));
  fail('failure');
});
```
> In Catalyst today the above test will pass. If this proposal to automatically flush the resulting promise were implemented it would fail.

Flushing after the tests complete has been the default behavior inside
Google since 2020. Very few tests remain that use the old behavior of
only flushing microtasks. The example above would actually fail with
`fakeAsync` due to the pending timer, but the argument still remains the
same. We might as well just flush if we're going to fail the test
anyways by throwing if there's no flush at the end.

PR Close #57239
2024-08-05 17:46:04 +00:00
Andrew Scott
b3836c2f1c refactor(core): Private option to rethrow ApplicationRef.tick errors in tests (#57153)
This creates a private option that can be used internally while we
migrate this to the default and only behavior. ~200 tests in TGP have errors
that are being swallowed (console.log) and not causing the test to fail.
We can first explicitly opt those out, flip the default internally, then
"fix" them by adding expect...toThrow.

PR Close #57153
2024-07-30 18:04:27 +00:00
Andrew Scott
3a63c9ebbe fix(core): errors during ApplicationRef.tick should be rethrown for zoneless tests (#56993)
The behavior of `ComponentFixture` for zoneless tests was decided somewhat through guesswork, trial, and error. In addition, working on the zoneless fixture revealed oddities in the behavior of the zone-based fixture, or behaviors that we felt were counterintuitive. The most consequential difference is how change detection works: `detectChanges` goes through ApplicationRef.tick in zoneless while it is `changeDetectorRef.detectChanges` in the zone fixture.

We felt that running change detection through `ApplicationRef.tick` was important for several reasons:
* Aligning application behavior more closely with the test behavior (almost all views are attached to application ref in reality)
* Ensuring that afterRender* hooks are executed when calling `fixture.detectChanges`
* Ensuring that the change detection runs again if render hooks update state

This change, however, has some noticeable consequences that will break some tests, mostly around how errors are handled. `ApplicationRef.tick` catches errors that happen during change detection and reports them to the ErrorHandler from DI. The default error handler only logs the error to the console. This will break tests which have `expect(() => fixture.detectChanges()).toThrow()`. In addition, it allows tests to pass when there are real errors encountered during change detection.

This change ensures that errors from `ApplicationRef.tick` are rethrown
and will fail the test. We should also do a follow-up investigation to
determine whether we can/should also do this for the zone-based
`ComponentFixture`.

fixes #56977

PR Close #56993
2024-07-29 13:49:00 -07:00
Andrew Scott
3b4b05d2f7 refactor(core): Remove hybrid mode flag and move scheduler provider location (#55722)
The flag is not used anymore and, as a result, is easier to move the
scheduler provider.

PR Close #55722
2024-05-23 18:15:53 +02:00
Andrew Scott
4bc05410dd refactor(core): calling autoDetectChanges without params works for zoneless (#55800)
This was mistakenly implemented automatically by the override without
filling in the default value of `true` like it is for the zone-based
fixture.

PR Close #55800
2024-05-15 08:46:30 -07:00
Kristiyan Kostadinov
aa8df1d029 refactor(core): clean up clang comments and workarounds (#55750)
Since we aren't using clang anymore, we can remove the comments and the workarounds that were in place to prevent it from doing the wrong thing.

PR Close #55750
2024-05-13 11:10:36 -07:00
Andrew Scott
abbaf8f639 refactor(core): Throw a runtime error if both zone and zoneless are provided (#55410)
This commit adds a dev-mode error if both the zone and zoneless
providers are used together.

PR Close #55410
2024-05-07 13:37:42 -07:00
Andrew Scott
7330b6944d fix(core): TestBed should not override NgZone from initTestEnvironment (#55226)
Prior to this change, `NgZone` was provided by default in TestBed in a
location that would override anything configured in
`TestBed.initTestEnvironment`. This change moves the default `NgZone`
provider to the `RootScopeModule` and these providers can be overridden
by the ones in `additionalModuleTypes`, which are assigned from the
first argument of `initTestEnvironment`. This makes it possible to
configure Zone globally for all tests as opposed to needing to repeat it
in `configureTestingModule` of each suite.

PR Close #55226
2024-05-06 13:33:16 -07:00
Andrew Kushnir
7645325d46 refactor(core): mark @defer APIs as stable (#55625)
This commit removes the `@developerPreview` annotation from the `@defer` APIs, effectively promoting them (and the entire feature!) to stable.

PR Close #55625
2024-05-02 14:42:00 -07:00
Joey Perrott
31fdf0fbea refactor: migrate core to prettier formatting (#55488)
Migrate formatting to prettier for core from clang-format

PR Close #55488
2024-04-29 09:49:19 -07:00
Andrew Scott
be17de53d4 refactor(core): Permit disabling autoDetect for zoneless fixture (#55494)
The caveat here is that this needs to be done before creating the
fixture. There are some technical issues to overcome with disabling it
after it was already enabled, related to detaching from `ApplicationRef`
without other side effects.

PR Close #55494
2024-04-26 09:42:15 -07:00
Andrew Scott
c175bca364 fix(core): DeferBlockFixture.render should not wait for stability (#55271)
The `DeferBlockFixture.render` function should not await the
`whenStable` promise of the fixture. This does not allow developers to
test any intermediate states that might occur between rendering the
initial content and the full app stability.

fixes #55235

PR Close #55271
2024-04-23 15:12:39 -07:00
Joey Perrott
5a10f405d3 fix(core): complete the removal of deprecation async function (#55491)
Remove the `async` function in favor of using `waitForAsync` instead.

BREAKING CHANGE: `async` has been removed, use `waitForAsync` instead.

PR Close #55491
2024-04-23 13:29:46 -07:00
Drew Mares
c773238055 docs: Fix typo in waitForAsync example (#55407)
The previous example had an incomplete code snippet that would not work if copied and pasted as is. There were two missing closing parentheses, one for the `inject` function, and the other for the `waitForAsync` function.

PR Close #55407
2024-04-23 13:16:15 -07:00
Andrew Scott
ff686f3ca5 refactor(core): Permit detectChanges with zoneless ComponentFixture (#55325)
This commit loosens the restrictions on calling `ComponentFixture.detectChanges`
when using the zoneless scheduler. While we don't necessarily think it's
a good idea, the thought is that it's unnecessary mental overhead to
diverge behaviors in the API when zoneless is enabled versus when it is
not. Instead, these opinionated restrictions can be considered when we
look at new testing APIs.

PR Close #55325
2024-04-17 08:07:52 +02:00
Andrew Scott
acf5f73a7b Revert "fix(core): TestBed should not override NgZone from initTestEnvironment (#55226)" (#55286)
This reverts commit e9a0c86766.
flakes and broken tests. needs more investigation.

PR Close #55286
2024-04-10 10:37:42 -07:00
Andrew Scott
e9a0c86766 fix(core): TestBed should not override NgZone from initTestEnvironment (#55226)
Prior to this change, `NgZone` was provided by default in TestBed in a
location that would override anything configured in
`TestBed.initTestEnvironment`. This change moves the default `NgZone`
provider to the `RootScopeModule` and these providers can be overridden
by the ones in `additionalModuleTypes`, which are assigned from the
first argument of `initTestEnvironment`. This makes it possible to
configure Zone globally for all tests as opposed to needing to repeat it
in `configureTestingModule` of each suite.

PR Close #55226
2024-04-08 11:28:07 -07:00
Andrew Scott
3974c2184e refactor(core): All views attached to application are treated as OnPush in zoneless (#55099)
This change treats all views attached to `ApplicationRef` as `OnPush`,
meaning that they have to be explicitly marked for check in order to be
refreshed when a tick happens. This prevents "accidentally" refreshing
views which have `Default` change detection as a side effect of running
change detection from an unrelated notification.

In addition, this change helps us achieve one of the big goals of the
project: that we can provide a testing experience which gives developers
more confidence that a component is zoneless-compatible. Because
`ComponentFixture` change detection is run through `ApplicationRef`
instead of `ChangeDetectorRef` when zoneless is enabled, this ensures
that the component under test has correctly been marked for check in
order to be updated. Without this, calling
`ComponentFixture.detectChanges` would allow a test to _force_ change
detection on a view when Angular would have otherwise not known that it
needed to be updated. Calling `ComponentFixture.detectChanges` on a component
which is not marked for check will now omit refreshing component view.

PR Close #55099
2024-03-29 10:42:21 -07:00
Andrew Scott
0b450ffa6f refactor(core): Move private export testing file to src (#55103)
This matches the private export approach for files in other packages.

PR Close #55103
2024-03-28 16:35:25 -07:00
Andrew Scott
22dd52f460 refactor(core): re-export private exports from testing (#55101)
This commit re-exports private exports  from testing package.

PR Close #55101
2024-03-28 13:54:16 -07:00
Andrew Scott
edfc5a9c3b refactor(core): private export token to disable fixture.detectChanges error (#55098)
While we do want to discourage `fixture.detectChanges`, it is yet to be
determined how this should be accomplished (warning, documentation, etc.). At
the moment, not being able to disable the error at all is entirely
prohibitive for existing test suites to use zoneless change detection in
tests. We would much rather allow existing test suites to disable the
error and use the zoneless change detection than prevent them from using
it entirely until all the `detectChanges` calls have been fixed.
Calling `detectChanges` manually can hide errors related to change
detection timing, but it is even worse when the "PseudoApplication"
fixture used (since it runs change detection _only_ on the component
rather than through `ApplicationRef`).

PR Close #55098
2024-03-28 13:14:27 -07:00
Andrew Scott
914e4530b0 fix(core): test cleanup should not throw if Zone is not present (#55096)
The test hooks should not throw if applications choose not to load ZoneJS.

fixes #48198

PR Close #55096
2024-03-28 12:12:30 -07:00
Andrew Scott
80f96e7c51 refactor(core): Provide scheduler for use and coordination with zone change detection (#54952)
This commit makes the zoneless scheduler (privately) available to applications that
have ZoneJS-based change detection. This would catch any changes of
interest (signal updates, `markForCheck` calls, attaching `Dirty` views)
that happen outside the Angular Zone.

See #53844 for additional information about why this is important.
More details to come in the a future commit that makes this a public option.

PR Close #54952
2024-03-27 11:36:56 -07:00
Andrew Scott
658cf8c384 fix(core): ComponentFixture stability should match ApplicationRef (#54949)
This change aligns the stability of `ComponentFixture` with that of
`ApplicationRef`, preventing confusing differences between the two as
more APIs start using the `PendingTasks` that may not be tracked by
`NgZone`.

BREAKING CHANGE: `ComponentFixture.whenStable` now matches the
`ApplicationRef.isStable` observable. Prior to this change, stability
of the fixture did not include everything that was considered in
`ApplicationRef`. `whenStable` of the fixture will now include unfinished
router navigations and unfinished `HttpClient` requests. This will cause
tests that `await` the `whenStable` promise to time out when there are
incomplete requests. To fix this, remove the `whenStable`,
instead wait for another condition, or ensure `HttpTestingController`
mocks responses for all requests. Try adding `HttpTestingController.verify()`
before your `await fixture.whenStable` to identify the open requests.
Also, make sure your tests wait for the stability promise. We found many
examples of tests that did not, meaning the expectations did not execute
within the test body.

In addition, `ComponentFixture.isStable` would synchronously switch to
true in some scenarios but will now always be asynchronous.

PR Close #54949
2024-03-27 11:01:28 -07:00
Andrea Canciani
f3b624553a refactor: fix a number of typos throughout the codebase (#55018)
Fix some typos detected using spellchecking tools, both in
documentation and in code (comments, identifiers).

PR Close #55018
2024-03-27 10:54:31 -07:00
Andrew Scott
8cad4e8cbe fix(core): ComponentFixture autoDetect respects OnPush flag of host view (#54824)
This is a follow-up to #53718 that applies the same logic to the
`autoDetect` feature of the fixture's host view. This now unifies the
logic between `ApplicationRef` and `ComponentFixture` autodetect.

BREAKING CHANGE: The `ComponentFixture` `autoDetect` feature will no
longer refresh the component's host view when the component is `OnPush`
and not marked dirty. This exposes existing issues in components which
claim to be `OnPush` but do not correctly call `markForCheck` when they
need to be refreshed. If this change causes test failures, the easiest
fix is to change the component to `ChangeDetectionStrategy.Default`.

PR Close #54824
2024-03-13 08:32: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 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
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
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
Andrew Scott
3cf612c857 fix(core): update imports to be compatible with rxjs 6 (#54193)
Peer dependency range allows for rxjs 6. We cannot use features only
available in rxjs 7 unless that changes.

fixes #54192

PR Close #54193
2024-02-01 20:48:19 +00:00
Andrew Scott
3ca34e606d refactor(core): Update ComponentFixture behavior when using zoneless scheduler (#54024)
When the zoneless scheduler is provided, we want to update the behavior
of `ComponentFixture` to address common issues and painpoints in testing.
Developers should never have to call `detectChanges` on a fixture
manually. Instead of calling `detectChanges` after performing an
action that updates state and requies a template refresh, developers
should wait for change detection to run because the update needs to also have
notified the scheduler. If this was not the case, the component would
not work correctly in the application. Calling `detectChanges` to force
an update could hide real bugs.

This commit also updates the zoneless tests to uses `ComponentFixture`
instead of manually attaching to the `ApplicationRef` and rewriting a
lot of the helpers (`getDebugNode`, `isStable` as a value, `whenStable` as a
Promise).

PR Close #54024
2024-01-29 20:21:47 +00:00
cexbrayat
037b79b72e fix(core): change defer block fixture default behavior to playthrough (#54088)
This is a followup to #53956

The default behavior needs to be changed in `TestBedCompiler` as well to have an effect.

PR Close #54088
2024-01-26 15:44:40 +00:00
Jessica Janiuk
df6c2057f2 fix(core): Change defer block fixture default behavior to playthrough (#53956)
This inverts the default behavior of test bed to use playthrough for defer blocks instead of manual.

fixes: #53686

PR Close #53956
2024-01-17 10:45:42 -08:00
Andrew Scott
1f8c53cd0c fix(core): TestBed should still use the microtask queue to schedule effects (#53843)
Prior to this commit, `TestBed` would require tests call `flushEffects`
or `fixture.detectChanges` in order to execute effects. In general, we
want to discourage authoring tests like this because it makes the timing
of change detection and effects differ from what happens in the
application. Instead, developers should perform actions and `await` (or
`flush`/`tick` when using `fakeAsync`) some `Promise` so that Angular
can react to the changes in the same way that it does in the
application.

Note that this still _allows_ developers to flush effects synchronously
with `flushEffects` and `detectChanges` but also enables the <action>,
`await` pattern described above.

PR Close #53843
2024-01-11 12:05:57 -08:00
Andrew Scott
dfcf0d5882 fix(core): afterRender hooks now only run on ApplicationRef.tick (#52455)
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 #52429
fixes #53232

PR Close #52455
2024-01-08 11:30:27 -08:00
Jeremy Elbourn
91f250dab7 build: configure cross-pkg resolution for api extraction (#52499)
This commit adds path mapping and source dependencies necessary to fully
resolve types during api doc extraction.

PR Close #52499
2024-01-05 11:27:34 -08:00
Andrew Scott
c59a4dcd77 refactor(core): Use NoopNgZone in componentFixture (#53670)
The `ComponentFixture` code needlessly dances around the `ngZone` being
`null` when the `ComponentFixtureNoNgZone` option is set. Instead, it
can use the `NoopNgZone` to get the same effect without needing to have
checks all over the place for its presence.

PR Close #53670
2024-01-03 11:28:08 -08:00
cexbrayat
160363a626 refactor(core): remove no longer needed FlushableEffectRunner private export (#53525)
It is no longer needed after 70a442eb02

PR Close #53525
2023-12-13 09:22:42 -08:00
Andrew Scott
8cccc7c5be refactor(core): Clean up subscription handling in ComponentFixture (#53426)
The subscriptions can be managed by a single subscription and unsubscribe on destroy

PR Close #53426
2023-12-08 14:30:14 -08:00
Andrew Scott
70a442eb02 refactor(core): Run ComponentFixture constructor in injection context (#53400)
The component fixture dependencies have to be passed in manually. This
is a bit annoying to manage as we expand which dependencies are needed.
Instead, we can run the constructor in the TestBed injection context and
move the dependencies into the component fixture code, as is done with
other constructors in Angular.

PR Close #53400
2023-12-07 09:35:24 -08:00
Andrew Kushnir
58cf389d80 fix(core): avoid stale provider info when TestBed.overrideProvider is used (#52918)
This commit updates the logic to preserve previous value of cached TView before applying overrides. This helps ensure that the next tests that uses the same component has correct provider info.

PR Close #52918
2023-11-29 09:48:53 +01:00
Matthieu Riegler
e00ae2d07a refactor(core): replace runInContext by runInInjectionContext (#53035)
Saves a few bytes since function names can be mangled.

PR Close #53035
2023-11-20 12:05:00 -08:00
Andrew Kushnir
ee892ee294 fix(core): reset cached scope for components that were overridden using TestBed (#52916)
Currently, when a component is overriden using `TestBed.overrideComponent`, Angular retains calculated scope for that component (a set of components and directives used within a component). This may cause stale information to be used in tests in some cases. This commit updates the logic to reset overridden component scope, so it gets re-computed during the next invocation.

Resolves #52817.

PR Close #52916
2023-11-15 14:03:55 +00:00
Andrew Kushnir
55d2c427c2 refactor(core): delay applying component metadata until it's needed in tests (#52708)
When a component contains `@defer` blocks, Angular compiler generates the code to apply component metadata (from the `@Component` decorator) after resolving all dynamic dependencies. Currently, this function is invoked eagerly at runtime, which causes dynamic imports to be kicked off earlier than expected. With the change in this commit, Angular will start resolving async metadata when it becomes necessary during testing.

PR Close #52708
2023-11-09 16:58:06 +00:00
Alex Rickabaugh
01b79356fb Revert "refactor(core): rename ViewRef<T> to InternalViewRef<T> and remove existing InternalViewRef (#52430)" (#52484)
This reverts commit a568bc5d97 as it breaks
tests in g3.

PR Close #52484
2023-11-01 10:13:12 -07:00
Alex Rickabaugh
ee9605f3c8 fix(core): effects wait for ngOnInit for their first run (#52473)
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
2023-11-01 08:07:35 -07:00