Commit Graph

2809 Commits

Author SHA1 Message Date
Andrew Scott 12181b9914 refactor(core): Use single source of truth for ApplicationRef.isStable (#53576)
This commit updates the `ApplicationRef.isStable` implementation to use
a single `Observable` to manage the state. This simplifies the mental
model quite a bit and removes the need for rx operators like
`distinctUntilChanged` and `combineLatest`.

PR Close #53576
2023-12-19 12:42:45 -08:00
Andrew Scott c16e9a7cf0 refactor(core): No default for zone stableness token (#53505)
When an application does not use zones, it does not need a default value
for the zone stableness token. This will allow zoneless applications to
tree-shake a lot of rxjs operators out of `ApplicationRef`.

Note that at the moment, `provideZoneChangeDetection` is included in all
applications as well as the `TestBed` environment. It is not currently
possible to remove the zone stable code as a result. This will be
possible only when we make zones an opt-in rather than opt-out.

PR Close #53505
2023-12-19 09:59:12 -08:00
Kristiyan Kostadinov df8a825910 fix(compiler): project empty block root node (#53620)
Expands the workaround from #52414 to allow for the root nodes of `@empty` blocks to be content projected.

Fixes #53570.

PR Close #53620
2023-12-19 08:37:33 -08:00
Andrew Scott 6c8faaa769 refactor(core): Rename InitialRenderPendingTasks and restructure isStable observable (#53534)
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
2023-12-19 08:36:28 -08:00
Kristiyan Kostadinov 872e7f25fe fix(core): tree shake version class (#53598)
Core bundles were retaining the `Version` class and `VERSION` constant, because we stamp out the current version in the DOM. This shouldn't be necessary, because any usage of `0.0.0-PLACEHOLDER` will be replaced with the current version at build time. These changes remove the reference so it can be tree shaken away.

PR Close #53598
2023-12-18 16:26:36 +00:00
Charles Lyding e149ebf228 build: update rxjs build version to v7 (#53500)
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
2023-12-18 16:25:37 +00:00
Matthieu Riegler d7e7409e81 refactor(animations): drop convertToMap & copyStyles and use native functions. (#52441)
`copyStyles` was side-effectful and also returned a value : not great.

PR Close #52441
2023-12-14 12:33:24 -08:00
Matthieu Riegler d28ce0a4cf refactor(animations): use a spread operator instead of custom copyObj (#52441)
Using the spread operator is the fastest way to shallow copy an object.

PR Close #52441
2023-12-14 12:33:24 -08:00
Matthieu Riegler 6a347d36eb refactor(animations): use a spread operator to convert an iterable to an array. (#52441)
Using the spread operator is the fastest way to convert an iterable to an array.

PR Close #52441
2023-12-14 12:33:24 -08:00
Andrew Scott 606de5166e refactor(core): newly created and any dirty views should get refreshed during CD (#53022)
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
2023-12-14 09:17:49 -08:00
Paul Gschwendtner abdc7e4578 feat(compiler-cli): support type-checking for generic signal inputs (#53521)
This commit adds the last remaining piece for signal input
type-checking. Bound values to signal inputs are already checked
properly at this point, but inference of generic directive/component
types through their inputs is not implemented.

This commit fixes this. To achieve this, there are a couple of potential
solutions. The generics of a directive are inferred based on input
value expressions using a so-called type constructor. The constructor
looks something like this:

```
const _ctor = <T>(v: Pick<Dir<T>, 'input1', 'input2'>) => Dir<T>;

_ctor({input1: expr1, input2: expr2});
```

This works very well for non-signal inputs where the class member is
directly holding the input values. For signal inputs, this does NOT
work because the class member will actually hold the `InputSignal`
instance. There are a couple of solutions to this:

1. Calling `_ctor` with an `InputSignal<typeof value>`
2. Converting the `_ctor` input signal fields to their write types
   (unwrapping the input signals).

We've decided to go with the second option as TypeScript is very
sensitive with assignments and its checks. i.e. co-variance,
contravariance or bivariance. Semantically it makes more sense to unwrap
the input signal "write type" directly and "assign to it". This is safer
and conceptually also easier to follow. A type constructor continues to
only receive the "expresison values". This simplifies code as well.

It's worth noting that the unwrapping as per option 2 also comes at a
cost. We need to be able to generate imports in type constructors. This
was not possible until the previous commit because inline type constructors
did not have an associated type-check block `Environment` and we were
missing access to expression translation and correct import generation.

Overall, solution 2 is now implemented as works as expected. This commit
adds additional unit tests to ensure this.

PR Close #53521
2023-12-13 15:44:00 -08:00
Paul Gschwendtner 8908fcd809 refactor(compiler-cli): initial type-checking for signal inputs (#53521)
This commit introduces the initial type-checking for signal inputs.
To enable type-checking od signal inputs, there are a couple of tricks
needed. It's not trivial as it would look like at first glance.

Initial attempts could have been to generate additional statements in
type-checking blocks for signal inputs to simply call a method like
`InputSignal#applyNewValue`. This would seem natural, as it would match
what will happen at runtime, but this would break the language-service
auto completion in a highly subtle way. Consider the case where multiple
directives match the same input. Consider the directives have some
overlap in accepted input values, but they also have distinct diverging
values, like:

```ts
class DirA {
  value = input<'apple'|'shared'>();
}

class DirB {
  value = input<'orange'|'shared'>();
}
```

In such cases, auto completion for the binding expression should suggest
the following values: `apple`, `shared`, `orange` and `undefined`.

The language service achieves this by getting completions in the
type-check block where the user expression would live. This BREAKS if
we'd have multiple places where the expression from the user is used.

Two different places, or more, surface additional problems with
diagnostic collection. Previously diagnostics would surface the union
type of allowed values, but with multiple places, we'd have to work with
potentially 1+ diagnostics. This is non-ideal.

Another important consideration is test coverage. It might sound
problematic to consider the existing test infrastructure as relevant,
but in practice, we have thousands of diagnostic type check block tests
that would greatly benefit if the general emit structure would still
match conceptually. This is another bonus argument on why changing the
way inputs are applied is probably an option we should consider as a
last resort.

Ultimately, there is a good solution where we unwrap directive signal
inputs, based on metadata, and access a brand type field on the
`InputSignal`. This ensures auto-completion continues to work as is, and
also the structure of type check blocks doesn't change conceptually. In
future commits we also need to handle type-inference for generic signal
inputs.

Note: Another alternative considered, in terms of using metadata or not.
We could have type helpers to unwrap signal inputs using type helpers
like: `T extends InputSignal<any, WriteT> ? WriteT : T`. This would
allow us to drop the input signal metadata dependency, but in reality,
this has a few issues:

- users might have `@Input`'s passing around `InputSignal`'s. This is
  unlikely, but shows that the solution would not be fully correct.
- we need the metadata regardless, as we plan on accessing it at runtime
  as well, to distinguish between signal inputs and normal inputs when
  applying new values. This was not clear when this option was
  considered initially.

PR Close #53521
2023-12-13 15:44:00 -08:00
Paul Gschwendtner bbd918acd3 refactor(core): introduce signal input() function and compiler detection (#53521)
This commit introduces a function for declaring inputs in
components. The function is called `input`. It comes in two flavors:

- `input` for optional inputs with initial values
- `input.required` for required inputs

Inputs are declared as class members, like with `@Input`- except that
the class field will no longer hold the input value directly. Angular
takes control over the input field and exposes the input value as a
signal. The runtime implementation will follow in future commits.

This commit simply introduces:

- initial compiler detection to recognize such inputs in classes
- the initial signature of `input` and `input.required`.

Note: the defer size test is flawed and there is no minification- hence
this commit also needs to incorporate the new dependency graph changes.

PR Close #53521
2023-12-13 15:44:00 -08:00
Andrew Scott 629343f247 fix(core): Multiple subscribers to ApplicationRef.isStable should all see values (#53541)
The behavior of `ApplicationRef.isStable` changed in 16.1 due to
https://github.com/angular/angular/commit/28c68f709cdc930e12bac51a266e7bf790656034.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.

PR Close #53541
2023-12-13 11:26:51 -08:00
Charles Lyding 89d17dc972 build: replace base64-js package with Node.js Buffer usage (#53464)
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
2023-12-11 14:04:06 -08:00
Pawel Kozlowski e3a6bf9b6c perf(core): optimize memory allocation when reconcilling lists (#52245)
This change replaces the implementation of the multi-map used to store
detached views while reconciling lists. The new implementation optimizes
memory allocation for such map and avoid arrays allocation when there are
no duplicated keys.

PR Close #52245
2023-12-08 15:00:13 -08:00
Andrew Scott 51ed3b510d refactor(core): Separate application and platform files even more (#53371)
This separates application and platform code into even more files. This now removes
the ciruclar dependency between scheduling and application ref.

PR Close #53371
2023-12-08 14:29:46 -08:00
Pawel Kozlowski 42f4f70e97 fix(core): remove signal equality check short-circuit (#53446)
The PR https://github.com/angular/angular/pull/52465 introduced short-circuit for
the signal equality invocation - with the reasoning that the equality function
should never return false for arguments with the same references. In practice it
turned out that it is rather surprising and the subsequent PR
https://github.com/angular/angular/pull/52532 added a warning when the short-circuit
was taking priority over the equality function.

Still, the presence of the short-circuit prevents people from mutating objects in
place and based on https://github.com/angular/angular/issues/52735 this is a common
and desired scenario. This change removes the short-circuit altogether and thus
fixes the mentioned issue.

We do recognize that removing short-circuit exposes developers to the potentially
surprising logic where mutated in-place change won't be propagated throug the
reactivity graph (due to the deault equality function). But we assume that this might
be less surprising / more desirable as compared to the short-circuit logic.

Fixes #52735

PR Close #53446
2023-12-08 07:05:34 -08:00
Andrew Scott 2565121851 fix(core): Avoid refreshing a host view twice when using transplanted views (#53021)
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
2023-12-07 12:05:23 -08:00
Andrew Scott a9872ccbb9 Revert "Revert "fix(router): Ensure canMatch guards run on wildcard routes (#53239)" (#53339)" (#53342)
This reverts commit ac8eb5b943.

PR Close #53342
2023-12-06 09:44:19 -08:00
Alex Rickabaugh 580af8e5f8 Revert "refactor(core): output an error guide link in prod mode (#53324)" (#53392)
This reverts commit f245aba782.

Reason: breaks g3

PR Close #53392
2023-12-06 07:23:23 -08:00
Andrew Kushnir f245aba782 refactor(core): output an error guide link in prod mode (#53324)
Currently, the link to an error guide is only included into an error message in dev mode. This change makes the `Find more at https://angular.io/errors/NG0XYZ` appear in the error message even in prod mode. Note: the rest of the error message is still tree-shaken away in prod mode (as it happens today).

PR Close #53324
2023-12-04 21:42:28 -08:00
JoostK 77939a3bd3 fix(core): cleanup signal consumers for all views (#53351)
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
2023-12-04 21:41:12 -08:00
Dylan Hunn d4b4236902 Revert "fix(router): Ensure canMatch guards run on wildcard routes (#53239)" (#53339)
This reverts commit 1940280d27.

PR Close #53339
2023-12-03 14:57:54 -08:00
Andrew Scott 1940280d27 fix(router): Ensure canMatch guards run on wildcard routes (#53239)
This commit makes sure that wildcard routes still run the `canMatch`
guards.

Fixes #49949

PR Close #53239
2023-11-30 09:45:40 -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
Kristiyan Kostadinov 8d43dbc7f3 test(core): add tests for control flow content projection with ng-container (#53190)
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
2023-11-28 11:18:43 +01:00
Kristiyan Kostadinov 162d940b72 refactor(platform-browser): clean up legacy way of getting a relative path (#53097)
Currently the way we extract the pathname of a URL is by creating an anchor node, assigning the URL to its `href` and reading the `pathname`. This is inefficient and it triggers an internal security check that doesn't allow the `href` attribute to be set which ends up blocking https://github.com/angular/components/pull/28155.

These changes switch to using the browser's built-in URL parsing instead.

PR Close #53097
2023-11-27 10:59:53 +01:00
Kristiyan Kostadinov c7c7ea9813 fix(core): inherit host directives (#52992)
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
2023-11-20 13:16:15 -08:00
Leonel Franchelli 29e0834c4d fix(router): Resolvers in different parts of the route tree should be able to execute together (#52934)
The following commit accidentally broken execution of resolvers when
two resolvers appear in different parts of the tree and do not share a
https://github.com/angular/angular/commit/327896606832bf6fbfc8f23989755123028136a8

This happens when there are secondary routes. This test ensures that all
routes with resolves are run.

fixes #52892

PR Close #52934
2023-11-20 12:07:10 -08:00
Andrew Scott b35c6731e5 fix(core): Reattached views that are dirty from a signal update should refresh (#53001)
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
2023-11-20 12:06:32 -08: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
Kristiyan Kostadinov 406049b95e fix(compiler): generate i18n instructions for blocks (#52958)
Adds support for generating i18n instructions inside of blocks.

Fixes #52540.
Fixes #52767.

PR Close #52958
2023-11-20 08:59:25 -08:00
Kristiyan Kostadinov 6f67d0c47c Revert "test(core): add tests for control flow content projection with ng-container (#52726)" (#53012)
This reverts commit 181f8e4b6c.

PR Close #53012
2023-11-17 11:52:02 -08:00
Kristiyan Kostadinov 181f8e4b6c test(core): add tests for control flow content projection with ng-container (#52726)
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
2023-11-17 08:08:40 -08:00
Andrew Scott 2d41b336a3 test(core): Remove manual ComponentFixture construction (#52983)
Tests still pass when using TestBed.createComponent directly

PR Close #52983
2023-11-16 15:12:32 -08:00
Andrew Kushnir 1ce31d819b fix(core): handle local refs when getDeferBlocks is invoked in tests (#52973)
This commit fixes an issue where having elements with local refs in some cases causes JS exception.

PR Close #52973
2023-11-16 12:20:54 -08:00
Kristiyan Kostadinov ec2d6e7b9c fix(compiler): changed after checked error in for loops (#52935)
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
2023-11-15 21:13:36 +00: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 Scott 0a976c86d6 refactor(core): Remove detectChanges private export (#52875)
detectChanges private export is not needed.

PR Close #52875
2023-11-14 21:05:42 +00:00
Andrew Scott c5ead61462 refactor(core): Move booleans in LContainer to flags slot (#52338)
There are now 2 booleans in the LContainer so this commit moves them to a shared
FLAGS slot like the LView.

PR Close #52338
2023-11-13 18:25:17 +00:00
Andrew Scott da616ee773 test(core): Ensure signals can be read after view creation during change detection (#52495)
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
2023-11-13 18:24:41 +00:00
AleksanderBodurri 6aef0f6fc8 fix(core): handle non-container environment injector cases (#52774)
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
2023-11-13 16:21:03 +00:00
Pawel Kozlowski 6c8776ff71 fix(core): limit rate of markers invocations (#52742)
This PR assures that the performance markers are invoked
only once for a given feature.

Closes #52524

PR Close #52742
2023-11-09 19:58:26 +00:00
Pawel Kozlowski ea8c9b61d2 fix(core): properly update collection with repeated keys in @for (#52697)
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
2023-11-09 15:44:53 +00:00
cexbrayat c745ca2a61 refactor(core): sanitize reactivity tests (#52632)
It was intriguing to see a double `fixture.detectChanges()` introduced by 38c9f08c8d
It turns out this is not needed.

PR Close #52632
2023-11-08 08:51:50 -08:00
Andrew Scott 33da677ecc refactor(core): Remove RootViewRef<T> because it is the same as ViewRef<T> (#52430)
`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
2023-11-08 08:50:50 -08:00
Andrew Scott 8592585f6a test(core): Add test to ensure writing to signals in afterRender hooks throws error (#52475)
We do not yet handle running change detection again if `afterRender`
hooks write to signals.

PR Close #52475
2023-11-08 08:48:44 -08:00
Andrew Scott a8071ffcbd refactor(core): Remove change detection flag used in g3 (#52530)
This has now been cleaned up internally and is no longer needed

PR Close #52530
2023-11-06 11:42:15 -08:00
Kristiyan Kostadinov 9cfd35a594 fix(compiler): ng-template directive invoke twice at the root of control flow (#52515)
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
2023-11-06 09:03:45 -08:00