Currently we convert objects to strings using `'' + value` which is quickest,
but it stringifies the value using its `valueOf`, rather than `toString`. These
changes switch to using `String(value)` which has identical performance
and calls the `toString` method as expected. Note that another option
was calling `toString` directly, but benchmarking showed it to be slower.
I've included the benchmark I used to verify the performance so we have it
for future reference and we can reuse it when making changes to `renderStringify`
in the future.
Also for reference, here are the results of the benchmark:
```
Benchmark: renderStringify
concat: 2.006 ns(0%)
concat with toString: 2.201 ns(-10%)
toString: 237.494 ns(-11741%)
toString with toString: 121.072 ns(-5937%)
constructor: 2.201 ns(-10%)
constructor with toString: 2.201 ns(-10%)
toString mono: 14.536 ns(-625%)
toString with toString mono: 9.757 ns(-386%)
```
Fixes#38839.
PR Close#39843
We currently only wrap the event listener in the function which ensures
ancestors are marked for check when the listener is placed on an element
that has a native method for listening to an event. We actually need to do
this wrapping in all cases so that events that are attached to non-rendered
template items (`ng-template` and `ng-container`) also mark ancestors for check
when they receive the event.
fixes#39832
PR Close#39833
This commit fixes a bug when `Attribute` DI decorator is used in the
`deps` section of a token that uses a factory function. The problem
appeared because the `Attribute` DI decorator was not handled correctly
while injecting factory function attributes.
Closes#36479
PR Close#37085
When a `ViewContainerRef` is injected, we dynamically create a comment node next to the host
so that it can be used as an anchor point for inserting views. The comment node is inserted
through the `appendChild` helper from `node_manipulation.ts` in most cases.
The problem with using `appendChild` here is that it has some extra logic which doesn't return
a parent `RNode` if an element is at the root of a component. I __think__ that this is a performance
optimization which is used to avoid inserting an element in one place in the DOM and then
moving it a bit later when it is projected. This can break down in some cases when creating
a `ViewContainerRef` for a non-component node at the root of another component like the following:
```
<root>
<div #viewContainerRef></div>
</root>
```
In this case the `#viewContainerRef` node is at the root of a component so we intentionally don't
insert it, but since its anchor element was created manually, it'll never be projected. This will
prevent any views added through the `ViewContainerRef` from being inserted into the DOM.
These changes resolve the issue by not going through `appendChild` at all when creating a comment
node for `ViewContainerRef`. This should work identically since `appendChild` doesn't really do
anything with the T structures anyway, it only uses them to reach the relevant DOM nodes.
Fixes#39556.
PR Close#39599
* Fixes that the Ivy styling logic wasn't accounting for `!important` in the property value.
* Fixes that the default DOM renderer only sets `!important` on a property with a dash in its name.
* Accounts for the `flags` parameter of `setStyle` in the server renderer.
Fixes#35323.
PR Close#39603
In ViewEngine, SelfSkip would navigate up the tree to get tokens from
the parent node, skipping the child. This restores that functionality in
Ivy. In ViewEngine, if a special token (e.g. ElementRef) was not found
in the NodeInjector tree, the ModuleInjector was also used to lookup
that token. While special tokens like ElementRef make sense only in a
context of a NodeInjector, we preserved ViewEngine logic for now to
avoid breaking changes.
We identified 4 scenarios related to @SkipSelf and special tokens where
ViewEngine behavior was incorrect and is likely due to bugs. In Ivy this
is implemented to provide a more intuitive API. The list of scenarios
can be found below.
1. When Injector is used in combination with @Host and @SkipSelf on the
first Component within a module and the injector is defined in the
module, ViewEngine will get the injector from the module. In Ivy, it
does not do this and throws instead.
2. When retrieving a @ViewContainerRef while @SkipSelf and @Host are
present, in ViewEngine, it throws an exception. In Ivy it returns the
host ViewContainerRef.
3. When retrieving a @ViewContainerRef on an embedded view and @SkipSelf
is present, in ViewEngine, the ref is null. In Ivy it returns the parent
ViewContainerRef.
4. When utilizing viewProviders and providers, a child component that is
nested within a parent component that has @SkipSelf on a viewProvider
value, if that provider is provided by the parent component's
viewProviders and providers, ViewEngine will return that parent's
viewProviders value, which violates how viewProviders' visibility should
work. In Ivy, it retrieves the value from providers, as it should.
These discrepancies all behave as they should in Ivy and are likely bugs
in ViewEngine.
PR Close#39464
Currently expressions `$event.foo()` and `this.$event.foo()`, as well as `$any(foo)` and
`this.$any(foo)`, are treated as the same expression by the compiler, because `this` is considered
the same implicit receiver as when the receiver is omitted. This introduces the following issues:
1. Any time something called `$any` is used, it'll be stripped away, leaving only the first parameter.
2. If something called `$event` is used anywhere in a template, it'll be preserved as `$event`,
rather than being rewritten to `ctx.$event`, causing the value to undefined at runtime. This
applies to listener, property and text bindings.
These changes resolve the first issue and part of the second one by preserving anything that
is accessed through `this`, even if it's one of the "special" ones like `$any` or `$event`.
Furthermore, these changes only expose the `$event` global variable inside event listeners,
whereas previously it was available everywhere.
Fixes#30278.
PR Close#39323
adds RuntimeError and code enum to improve debugging experience
refactor ExpressionChangedAfterItHasBeenCheckedError to code NG0100
refactor CyclicDependency to code NG0200
refactor No Provider to code NG0201
refactor MultipleComponentsMatch to code NG0300
refactor ExportNotFound to code NG0301
refactor PipeNotFound to code NG0302
refactor BindingNotKnown to code NG0303
refactor NotKnownElement to code NG0304
PR Close#39188
Currently `i18n` attributes are treated the same no matter if they have data bindings or not. This
both generates more code since they have to go through the `ɵɵi18nAttributes` instruction and
prevents the translated attributes from being injected using the `@Attribute` decorator.
These changes makes it so that static translated attributes are treated in the same way as regular
static attributes and all other `i18n` attributes go through the old code path.
Fixes#38231.
PR Close#39408
group together similar error messages as part of error code efforts
ProviderNotFound & NodeInjector grouped into throwProviderNotFoundError
Cyclic dependency errors grouped into throwCyclicDependencyError
PR Close#39251
IMPORTANT: `HEADER_OFFSET` should only be refereed to the in the `ɵɵ*` instructions to translate
instruction index into `LView` index. All other indexes should be in the `LView` index space and
there should be no need to refer to `HEADER_OFFSET` anywhere else.
PR Close#39233
When looking at `TView` debug template only Element nodes were displayed
as `TNode.Element` was used for both `RElement` and `RText`.
Additionally no text was stored in `TNode.value`. The result was that
the whole template could not be reconstructed. This refactoring creates
`TNodeType.Text` and store the text value in `TNode.value`.
The refactoring also changes `TNodeType` into flag-like structure make
it more efficient to check many different types at once.
PR Close#39233
Before this refactoring/fix the ICU would store the current selected
index in `TView`. This is incorrect, since if ICU is in `ngFor` it will
cause issues in some circumstances. This refactoring properly moves the
state to `LView`.
closes#37021closes#38144closes#38073
PR Close#39233
This commit updates core tests and removes the code needed to support IE 9 and IE 10 only.
The code is no longer needed since IE 9 and IE 10 support is removed in v11.
PR Close#39090
`TNodeType.View` was created to support inline views. That feature did
not materialize and we have since removed the instructions for it, leave
an unneeded `TNodeType.View` which was still used in a very
inconsistent way. This change no longer created `TNodeType.View` (and
there will be a follow up chang to completely remove it.)
Also simplified the mental model so that `LView[HOST]`/`LView[T_HOST]`
always point to the insertion location of the `LView`.
PR Close#38707
The `RefreshTransplantedView` flag is used to indicate that the view or one of its children
is transplanted and dirty, so it should still be refreshed as part of change detection.
This flag is set on the transplanted view itself as well setting a
counter on as its parents.
When a transplanted view is detached and still has this flag, it means
it got detached before it was refreshed. This can happen for "backwards
references" or transplanted views that are inserted at a location that
was already checked. In this case, we should decrement the parent
counters _and_ clear the flag on the detached view so it's not seen as
"transplanted" anymore (it is detached and has no parent counters to
adjust).
fixes#38619
PR Close#38768
When removal of one view causes removal of another one from the same
ViewContainerRef it triggers an error with views length calculation. This commit
fixes this bug by removing a view from the list of available views before invoking
actual view removal (which might be recursive and relies on the length of the list
of available views).
Fixes#38201.
PR Close#38317
Fixes an error if a CSS custom property, used inside a host binding, has a
number in its name. The error is thrown because the styling parser only
expects characters from A to Z,dashes, underscores and a handful of other
characters.
Fixes#37292.
PR Close#38432
When using the safe navigation operator in a binding expression, a temporary
variable may be used for storing the result of a side-effectful call.
For example, the following template uses a pipe and a safe property access:
```html
<app-person-view [enabled]="enabled" [firstName]="(person$ | async)?.name"></app-person-view>
```
The result of the pipe evaluation is stored in a temporary to be able to check
whether it is present. The temporary variable needs to be declared in a separate
statement and this would also cause the full expression itself to be pulled out
into a separate statement. This would compile into the following
pseudo-code instructions:
```js
var temp = null;
var firstName = (temp = pipe('async', ctx.person$)) == null ? null : temp.name;
property('enabled', ctx.enabled)('firstName', firstName);
```
Notice that the pipe evaluation happens before evaluating the `enabled` binding,
such that the runtime's internal binding index would correspond with `enabled`,
not `firstName`. This introduces a problem when the pipe uses `WrappedValue` to
force a change to be detected, as the runtime would then mark the binding slot
corresponding with `enabled` as dirty, instead of `firstName`. This results
in the `enabled` binding to be updated, triggering setters and affecting how
`OnChanges` is called.
In the pseudo-code above, the intermediate `firstName` variable is not strictly
necessary---it only improved readability a bit---and emitting it inline with
the binding itself avoids the out-of-order execution of the pipe:
```js
var temp = null;
property('enabled', ctx.enabled)
('firstName', (temp = pipe('async', ctx.person$)) == null ? null : temp.name);
```
This commit introduces a new `BindingForm` that results in the above code to be
generated and adds compiler and acceptance tests to verify the proper behavior.
Fixes#37194
PR Close#37911
In JIT compiled apps, component definitions are compiled upon first
access. For a component class `A` that extends component class `B`, the
`B` component is also compiled when the `InheritDefinitionFeature` runs
during the compilation of `A` before it has finalized. A problem arises
when the compilation of `B` would flush the NgModule scoping queue,
where the NgModule declaring `A` is still pending. The scope information
would be applied to the definition of `A`, but its compilation is still
in progress so requesting the component definition would compile `A`
again from scratch. This "inner compilation" is correctly assigned the
NgModule scope, but once the "outer compilation" of `A` finishes it
would overwrite the inner compilation's definition, losing the NgModule
scope information.
In summary, flushing the NgModule scope queue could trigger a reentrant
compilation, where JIT compilation is non-reentrant. To avoid the
reentrant compilation, a compilation depth counter is introduced to
avoid flushing the NgModule scope during nested compilations.
Fixes#37105
PR Close#37795
Queries weren't matching directives that provide themselves via string
injection tokens, because the assumption was that any string passed to
a query decorator refers to a template reference.
These changes make it so we match both template references and
providers while giving precedence to the template references.
Fixes#38313.
Fixes#38315.
PR Close#38321
The currently selected ICU was incorrectly being stored it `TNode`
rather than in `LView`.
Remove: `TIcuContainerNode.activeCaseIndex`
Add: `LView[TIcu.currentCaseIndex]`
PR Close#38345
This change provides better typing for the `LView.debug` property which
is intended to be used by humans while debugging the application with
`ngDevMode` turned on.
In addition this chang also adds jasmine matchers for better asserting
that `LView` is in the correct state.
PR Close#38359
Currently we read lifecycle hooks eagerly during `ɵɵdefineComponent`.
The result is that it is not possible to do any sort of meta-programing
such as mixins or adding lifecycle hooks using custom decorators since
any such code executes after `ɵɵdefineComponent` has extracted the
lifecycle hooks from the prototype. Additionally the behavior is
inconsistent between AOT and JIT mode. In JIT mode overriding lifecycle
hooks is possible because the whole `ɵɵdefineComponent` is placed in
getter which is executed lazily. This is because JIT mode must compile a
template which can be specified as `templateURL` and those we are
waiting for its resolution.
- `+` `ɵɵdefineComponent` becomes smaller as it no longer needs to copy
lifecycle hooks from prototype to `ComponentDef`
- `-` `ɵɵNgOnChangesFeature` feature is now always included with the
codebase as it is no longer tree shakable.
Previously we have read lifecycle hooks from prototype in the
`ɵɵdefineComponent` so that lifecycle hook access would be monomorphic.
This decision was made before we had `T*` data structures. By not
reading the lifecycle hooks we are moving the megamorhic read form
`ɵɵdefineComponent` to instructions. However, the reads happen on
`firstTemplatePass` only and are subsequently cached in the `T*` data
structures. The result is that the overall performance should be same
(or slightly better as the intermediate `ComponentDef` has been
removed.)
- [ ] Remove `ɵɵNgOnChangesFeature` from compiler. (It will no longer
be a feature.)
- [ ] Discuss the future of `Features` as they hinder meta-programing.
Fix#30497
PR Close#35464
Fixes the following issues related to how we validate properties during JIT:
- The invalid property warning was printing `null` as the node name
for `ng-content`. The problem is that when generating a template from
`ng-content` we weren't capturing the node name.
- We weren't running property validation on `ng-container` at all.
This used to be supported on ViewEngine and seems like an oversight.
In the process of making these changes, I found and cleaned up a
few places where we were passing in `LView` unnecessarily.
PR Close#37773
One of the ivy acceptance tests currently fails in IE10. This
is because we recently added a new test that asserts that injecting
`ViewRef` results in a `NullInjectorError`.
Due to limitations in TypeScript and in polyfills for `setPrototypeOf`,
the error cannot be thrown as `ViewRef` is always considered injectable.
In reality, `ViewRef` should not be injectable, as explicitly noted
in c00f4ab2ae.
There seems no way to simulate the proper prototype chain in such
browsers that do not natively support `__proto__`, so TypeScript
and `core-js` polyfills simply break the prototype chain and
assign inherited properties directly on `ViewRef`. i.e. so that
`ViewRef.__NG_ELEMENT_ID__` exists and DI picks it up.
There is a way for TypeScript to theoretically generate proper
prototype chain in ES5 output, but they intend to only bother
about the proper prototype chain in ES6 where `setPrototypeOf`
etc. are offically standarized. See the response:
https://github.com/microsoft/TypeScript/issues/1601#issuecomment-94892833.
PR Close#37892
Currently when the `plural` or `select` keywords in an ICU contain trailing spaces (e.g. `{count, select , ...}`), these spaces are also included into the key names in ICU vars (e.g. "VAR_SELECT "). These trailing spaces are not desirable, since they will later be converted into `_` symbols while normalizing placeholder names, thus causing mismatches at runtime (i.e. placeholder will not be replaced with the correct value). This commit updates the code to trim these spaces while generating an object with placeholders, to make sure the runtime logic can replace these placeholders with the right values.
PR Close#37866
Invoking a callback registered through `ViewRef.onDestroy` throws an error, because we weren't registering it correctly in the internal data structure. These changes also remove the `storeCleanupFn` function, because it was mostly identical to `storeCleanupWithContext` and was only used in one place.
Fixes#36213.
PR Close#37543
Special DI tokens like `ChangeDetectorRef` and `ElementRef` can provide a factory via `NG_ELEMENT_ID`. The problem is that we were reading it off the token as `token[NG_ELEMENT_ID]` which will go up the prototype chain if it couldn't be found on the current token, resulting in the private `ViewRef` API being exposed, because it extends `ChangeDetectorRef`.
These changes fix the issue by guarding the property access with `hasOwnProperty`.
Fixes#36235.
PR Close#37574
This commit replaces an assert with more descriptive error message that is thrown in case `<ng-template>` or `<ng-container>` is used as host element for a Component.
Resolves#35240.
PR Close#35916
Currently when bootstrapped component is being removed using `ComponentRef.destroy` or `NgModuleRef.destroy` methods, DOM nodes may be retained in the DOM tree. This commit fixes that problem by always attaching host element of the internal root view to the component's host view node, so the cleanup can happen correctly.
Resolves#36449.
PR Close#37600
Currently Angular internally already handles `InjectionToken` as
predicates for queries. This commit exposes this as public API as
developers already relied on this functionality but currently use
workarounds to satisfy the type constraints (e.g. `as any`).
We intend to make this public as it's low-effort to support, and
it's a significant key part for the use of light-weight tokens as
described in the upcoming guide: https://github.com/angular/angular/pull/36144.
In concrete, applications might use injection tokens over classes
for both optional DI and queries, because otherwise such references
cause classes to be always retained. This was also an issue in View
Engine, but now with Ivy, this pattern became worse, as factories are
directly attached to retained classes (ultimately ending up in the
production bundle, while being unused).
More details in the light-weight token guide and in: https://github.com/angular/angular-cli/issues/16866.
Closes#21152. Related to #36144.
PR Close#37506
This PR provides a more helpful error than the one currently present:
`el.setAttribute is not a function`. It is not valid to have directives with host bindings
on `ng-template` or `ng-container` nodes. VE would silently ignore this, while Ivy
attempts to set the attribute and throws an error because these are comment nodes
and do not have `setAttribute` functionality.
It is better to throw a helpful error than to silently ignore this because
putting a directive with host binding on an `ng-template` or `ng-container` is most often a mistake.
Developers should be made aware that the host binding will have no effect in these cases.
Note that an error is already thrown in Ivy, as mentioned above, so this
is not a breaking change and can be merged to both master and patch.
Resolves#35994
PR Close#37111
In v9, we started showing a console warning when
instantiating a token that inherited its @Injectable
decorator rather than providing its own. This warning
said that the pattern would become an error in v10.
However, we have decided to wait until at least v11
to throw in this case, so this commit updates the
warning to be less prescriptive about the exact
version when the pattern will no longer be supported.
PR Close#37383
Previously all static styling information (including the ones from component/directive host bindings) would get merged into a single value before it would be written into the `@Input('class'/'style')`. The new behavior specifically excludes host values from the `@Input` bindings.
Fix#35383
PR Close#35889
In 420b9be1c1 all style-based sanitization code was
disabled because modern browsers no longer allow for javascript expressions within
CSS. This patch is a follow-up patch which removes all traces of style sanitization
code (both instructions and runtime logic) for the `[style]` and `[style.prop]` bindings.
PR Close#36965
This patch is the first of many commits to disable sanitization for
[stlye.prop] and [style] bindings in Angular.
Historically, style-based sanitization has only been required for old
IE browsers (IE6 and IE7). Since Angular does not support these old
browsers at all, there is no reason for the framework to support
style-based sanitization.
PR Close#35621
This commit fixes 2 separate issues related to root nodes retrieval from
embedded views with `<ng-content>`:
1) we did not account for the case where there were no projectable nodes
for a given `<ng-content>`;
2) we did not account for the case where projectable nodes for a given
`<ng-content>` were represented as an array of native nodes (happens in
the case of dynamically created components with projectable nodes);
Fixes#35967
PR Close#36051
Previously we were passing a string form of the value to pluralize
to the `getLocalePluralCase()` function that is extracted from the
locale data. But some locales have functions that rely upon this
value being a number not a string.
Now we convert the value to a number before passing it to the
locale data function.
Fixes#36888
PR Close#36901
Changes the Ivy unknown element/property messages from being logged with `console.warn` to `console.error`. This should make them a bit more visible without breaking existing apps. Furthermore, a lot of folks filter out warning messages in the dev tools' console, whereas errors are usually still shown.
BREAKING CHANGE:
Warnings about unknown elements are now logged as errors. This won't break your app, but it may trip up tools that expect nothing to be logged via `console.error`.
Fixes#35699.
PR Close#36399
Only refresh transplanted views at the insertion location in Ivy.
Previously, Ivy would check transplanted views at both the insertion and
declaration points. This is achieved by adding a marker to the insertion
tree when we encounter a transplanted view that needs to be refreshed at
its declaration. We use this marker as an extra indication that we still
need to descend and refresh those transplanted views at their insertion
locations even if the insertion view and/or its parents are not dirty.
This change fixes several issues:
* Transplanted views refreshed twice if both insertion and declaration
are dirty. This could be an error if the insertion component changes
result in data not being available to the transplanted view because it
is slated to be removed.
* CheckAlways transplanted views not refreshed if shielded by
non-dirty OnPush (fixes#35400)
* Transplanted views still refreshed when insertion tree is detached
(fixes#21324)
PR Close#35968
If there's an error during the first creation pass of a `TView`, the data structure may be corrupted which will cause framework assertion failures downstream which can mask the user's error. These changes add a new flag to the `TView` that indicates whether the first creation pass was successful, and if it wasn't we try re-create the `TView`.
Fixes#31221.
PR Close#36381
Prior to this change, animations-related runtime logic assumed that the @HostBinding and @HostListener with synthetic (animations) props are used for Components only. However having @HostBinding and @HostListener with synthetic props on Directives is also supported by View Engine. This commit updates the logic to select correct renderer to execute instructions (current renderer for Directives and sub-component renderer for Components).
This PR resolves#35501.
PR Close#35568
Prior to this commit unbound attributes were treated as possible inputs to structural directives. Since structural directives can only accepts inputs defined using microsyntax expression (e.g. `<div *dir="exp">`), such unbound attributes should not be considered as inputs. This commit aligns Ivy and View Engine behavior and avoids using unbound attributes as inputs to structural directives.
PR Close#36441
The flag that determines whether something should be able to inject from `viewProviders` is opt-out and the pipes weren't opted out, resulting in them being able to see the viewProviders if they're placed on a component host node.
Fixes#36146.
PR Close#36512
Prior to this commit, the unknown property check was unnecessarily invoked for AOT-compiled components (for these components, the check happens at compile time). This commit updates the code to avoid unknown property verification for AOT-compiled components by checking whether schemas information is present (as a way to detect whether this is JIT or AOT compiled component).
Resolves#35945.
PR Close#36072
In certain use-cases it's useful to have an ability to use empty strings as translations. Currently Ivy fails at runtime if empty string is used as a translation, since some parts of internal data structures are not created properly. This commit updates runtime i18n logic to handle empty translations and avoid unnecessary extra processing for such cases.
Fixes#36476.
PR Close#36499
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes#35231.
PR Close#35840
When using `platformBrowserDynamic().bootstrapModule()`, it is possible
to set `defaultEncapsulation` and `preserveWhitespaces` as default
configuration to influence how components are compiled. When compiling
components in JIT with Ivy, these options were not taken into account.
This commit publishes the options to be globally available, so that the
lazy compilation of JIT components has access to the configured
bootstrap options. Note that this approach does not allow changing the
options once they have been set, as Ivy's compilation model does not
allow for multiple compilations to exist at the same time.
For applications that bootstrap multiple modules, it is now required
to provide the exact same bootstrap options. An error is logged if
incompatible bootstrap options are provided, in which case the updated
options will be ignored.
Fixes#35230
Resolved FW-1838
PR Close#35534
Prior to this commit, Ivy compiler didn't handle directive inputs with interpolations located on `<ng-template>` elements (e.g. `<ng-template dir="{{ field }}">`). That was the case for regular inputs as well as inputs that should be processed via i18n subsystem (e.g. `<ng-template i18n-dir dir="Hello {{ name }}">`). This commit adds support for such expressions for explicit `<ng-template>`s as well as a number of tests to confirm the behavior.
Fixes#35752.
PR Close#35984
Prior to this commit, i18n runtime logic relied on the assumption that provided translation is syntactically correct, specifically around ICU syntax. However provided translations might contain some errors that lead to parsing failure. Specifically when translation contains curly braces, runtime i18n logic tries to parse them as an ICU expression and fails. This commit validates ICU parsing result (making sure it was parsed correctly) and throws an error if parsing error happens. The error that is thrown also contains translated message text for easier debugging.
Note: the check and the error message introduced in this PR is a safeguard against the problem that led to unhandled i18n runtime logic crash. So the framework behavior remains the same, we just improve the error message and it should be safe to merge to the patch branch.
Resolves#35689.
PR Close#35923
Pure pipes are not invoked again until their arguments are modified. The same
rule should apply to pure pipes that throw an exception. This fix ensures that
a pure pipe is not re-invoked if it throws an exception and arguments are not
changed.
PR Close#35827
Prior to this commit, while calculating the scope for a module, Ivy compiler processed `declarations` field first and `imports` after that. That results in a couple issues:
* for Pipes with the same `name` and present in `declarations` and in an imported module, Pipe from imported module was selected. In View Engine the logic is opposite: Pipes from `declarations` field receive higher priority.
* for Directives with the same selector and present in `declarations` and in an imported module, we first invoked the logic of a Directive from `declarations` field and after that - imported Directive logic. In View Engine, it was the opposite and the logic of a Directive from the `declarations` field was invoked last.
In order to align Ivy and View Engine behavior, this commit updates the logic in which we populate module scope: we first process all imports and after that handle `declarations` field. As a result, in Ivy both use-cases listed above work similar to View Engine.
Resolves#35502.
PR Close#35850
This reverts commit 00f3c58bb9.
Rolling back because it could be breaking e2e tests that assert that
there are no errors in the console after the assertions have run. We can
re-add this in v10.
PR Close#35845
Changes the Ivy unknown element/property messages from being logged with `console.warn` to `console.error`. This should make them a bit more visible without breaking existing apps. Furthermore, a lot of folks filter out warning messages in the dev tools' console, whereas errors are usually still shown.
Fixes#35699.
PR Close#35798
This is a follow up to #35637 which resolved a similar issue for `ComponentFactoryResolver`, but not the root cause. When a `NgModuleRef` is created, it instantiates an `Injector` internally which in turn resolves all of injector types. This can result in a circular call that results in an error, because the module is one of the injector types being resolved.
These changes work around the issue by allowing the constructor to run before resolving the injector types.
Fixes#35677.
Fixes#35639.
PR Close#35731
Before this change ngIvy implementation of queries would throw upon
encountering null / undefined query result collected from an embedded
view. It turns out that we might have a provider that explicitly provides
a null / undefined value in a place of a token queried for.
This commit removes a check from the ngIvy query implementation that was
asserting on a query result to be defined.
Fixes#35673
PR Close#35796
Before this change `[class]` and `[className]` were both converted into `ɵɵclassMap`. The implication of this is that at runtime we could not differentiate between the two and as a result we treated `@Input('class')` and `@Input('className)` as equivalent.
This change makes `[class]` and `[className]` distinct. The implication of this is that `[class]` becomes `ɵɵclassMap` instruction but `[className]` becomes `ɵɵproperty' instruction. This means that `[className]` will no longer participate in styling and will overwrite the DOM `class` value.
Fix#35577
PR Close#35668
Prior to this commit, i18n attributes defined on `<ng-template>` tags were not processed by the compiler. This commit adds the necessary logic to handle i18n attributes in the same way how these attrs are processed for regular elements.
PR Close#35681
This commit extends the range of tNode types that may have local refs to include `TNodeType.Container` to account for `<ng-template>`s. Original changes in https://github.com/angular/angular/pull/33415 didn't include that type and as a result, an error is thrown at runtime in case an i18n block contains an `<ng-template>` with local refs.
PR Close#35758
If an injectable has a `useClass`, Ivy injects the token in `useClass`, rather than the original injectable, if the injectable is re-provided under a different token. The correct behavior is that it should inject the re-provided token, no matter whether it has `useClass`.
Fixes#34110.
PR Close#34574
When binding to `[style]` we correctly sanitized/unwrapped properties but we did not do it for the object itself.
```
@HostBinding("style")
style: SafeStyle = this.sanitizer.bypassSecurityTrustStyle(
"background: red; color: white; display: block;"
);
```
Above code would fail since the `[style]` would not unwrap the `SafeValue` and would treat it as object resulting in incorrect behavior.
Fix#35476 (FW-1875)
PR Close#35564
Currently we resolve the `NgModuleRef.componentFactoryResolver` by going through the injector, but the problem is that `ComponentFactoryResolver` has a dependency on `NgModuleRef`, which means that if the module that's attached to the ref tries to inject `ComponentFactoryResolver` in its constructor, we'll create a circular dependency which throws at runtime.
These changes resolve the issue by creating the `ComponentFactoryResolver` manually ahead of time without going through the injector. We can do this safely, because the only dependency for the resolver is the current module ref which is providing it.
Aside from fixing the issue, another advantage to this approach is that it should reduce the amount of generated JS, because it removes a getter and a provider definitio.
Fixes#35580.
PR Close#35637
In #33705 we made it so that we generate pure functions for object/array literals in order to avoid having them be shared across elements/views. The problem this introduced is that further down the line the `ContantPool` uses the generated literal in order to figure out whether to share an existing factory or to create a new one. `ConstantPool` determines whether to share a factory by creating a key from the AST node and using it to look it up in the factory cache, however the key generation function didn't handle function invocations and replaced them with `null`. This means that the key for `{foo: pureFunction0(...)}` and `{foo: null}` are the same.
These changes rework the logic so that instead of generating a `null` key
for function invocations, we generate a variable called `<unknown>` which
shouldn't be able to collide with anything.
Fixes#35298.
PR Close#35481
In View Engine, host element of dynamically created component received attributes and classes extracted from component's selector. For example, if component selector is `[attr] .class`, the `attr` attribute and `.class` class will be add to host element. This commit adds similar logic to Ivy, to make sure this behavior is aligned with View Engine.
PR Close#34481
Before this change content queries with the `descendants: false` option, as implemented in ivy,
would not descendinto `<ng-container>` elements. This behaviour was different from the way the
View Engine worked. This change alligns ngIvy and VE behaviours when it comes to queries and the
`<ng-container>` elements and fixes a common bugs where a query target was placed inside the
`<ng-container>` element with a * directive on it.
Before:
```html
<needs-target>
<ng-container *ngIf="condition">
<div #target>...</div> <!-- this node would NOT match -->
</ng-container>
</needs-target>
```
After:
```html
<needs-target>
<ng-container *ngIf="condition">
<div #target>...</div> <!-- this node WILL match -->
</ng-container>
</needs-target>
```
Fixes#34768
PR Close#35384
When the same provider is resolved multiple times on the same node, the first invocation had the correct context, but all subsequent ones were incorrect because we were registering the hook multiple times under different indexes in `destroyHooks`.
Fixes#35167.
PR Close#35249
Currently the logic that handles ICUs located outside of i18n blocks may throw exceptions at runtime. The problem is caused by the fact that we store incorrect TNode index for previous TNode (index that includes HEADER_OFFSET) and do not store a flag whether this TNode is a parent or a sibling node. As a result, the logic that assembles the final output uses incorrect TNodes and in some cases throws exceptions (when incompatible structure is extracted from tView.data due to the incorrect index). This commit adjusts the index and captures whether TNode is a parent to make sure underlying logic manipulates correct TNode.
PR Close#35347
Given:
```
<div class="s1" [class]="null" [ngClass]="exp">
```
Notice that `[class]` binding is not a `string`. As a result the existing logic would not concatenate `[class]` with `class="s1"`. The resulting falsy value would than be sent to `ngClass` which would promptly clear all styles on the `<div>`
The new logic correctly handles falsy values for `[class]` bindings.
Fix#35335
PR Close#35350
In the `loadRenderer` we make an assumption that the value will always be an `LView`, but if there's a directive on the same node which injects `ViewContainerRef` the `LView` will be wrapped in an `LContainer`. These changes add a call to unwrap the value before we try to read the value off of it.
Fixes#35342.
PR Close#35343
Prior to this change, element namespace was not set for host elements of dynamically created components that resulted in incorrect rendering in a browser. This commit adds the logic to pick and set correct namespace for host element when component is created dynamically.
PR Close#35136
Inside `*ngFor` the second run of the styling instructions can get into situation where it tries to read a value from a binding which has not yet executed. As a result the read is `NO_CHANGE` value and subsequent property read cause an exception as it is of wrong type.
Fix#35118
PR Close#35133
This change changes the priority order of static styling.
Current priority:
```
(least priority)
- Static
- Component
- Directives
- Template
- Dynamic Binding
- Component
- Map/Interpolation
- Property
- Directives
- Map/Interpolation
- Property
- Template
- Map/Interpolation
- Property
(highest priority)
```
The issue with the above priority is this use case:
```
<div style="color: red;" directive-which-sets-color-blue>
```
In the above case the directive will win and the resulting color will be `blue`. However a small change of adding interpolation to the example like so. (Style interpolation is coming in https://github.com/angular/angular/pull/34202)
```
<div style="color: red; width: {{exp}}px" directive-which-sets-color-blue>
```
Changes the priority from static binding to interpolated binding which means now the resulting color is `red`. It is very surprising that adding an unrelated interpolation and style can change the `color` which was not changed. To fix that we need to make sure that the static values are associated with priority of the source (directive or template) where they were declared. The new resulting priority is:
```
(least priority)
- Component
- Static
- Map/Interpolation
- Property
- Directives
- Static
- Map/Interpolation
- Property
- Template
- Static
- Map/Interpolation
- Property
(highest priority)
```
PR Close#34938
Previously we would write to class/style as strings `element.className` and `element.style.cssText`. Turns out that approach is good for initial render but not good for updates. Updates using this approach are problematic because we have to check to see if there was an out of bound write to style and than perform reconciliation. This also requires the browser to bring up CSS parser which is expensive.
Another problem with old approach is that we had to queue the DOM writes and flush them twice. Once on element advance instruction and once in `hostBindings`. The double flushing is expensive but it also means that a directive can observe that styles are not yet written (they are written after directive executes.)
The new approach uses `element.classList.add/remove` and `element.style.setProperty/removeProperty` API for updates only (it continues to use `element.className` and `element.style.cssText` for initial render as it is cheaper.) The other change is that the styling changes are applied immediately (no queueing). This means that it is the instruction which computes priority. In some circumstances it may result in intermediate writes which are than overwritten with new value. (This should be rare)
Overall this change deletes most of the previous code and replaces it with new simplified implement. The simplification results in code savings.
PR Close#34804
NOTE: This change must be reverted with previous deletes so that it code remains in build-able state.
This change deletes old styling code and replaces it with a simplified styling algorithm.
The mental model for the new algorithm is:
- Create a linked list of styling bindings in the order of priority. All styling bindings ere executed in compiled order and than a linked list of bindings is created in priority order.
- Flush the style bindings at the end of `advance()` instruction. This implies that there are two flush events. One at the end of template `advance` instruction in the template. Second one at the end of `hostBindings` `advance` instruction when processing host bindings (if any).
- Each binding instructions effectively updates the string to represent the string at that location. Because most of the bindings are additive, this is a cheap strategy in most cases. In rare cases the strategy requires removing tokens from the styling up to this point. (We expect that to be rare case)S Because, the bindings are presorted in the order of priority, it is safe to resume the processing of the concatenated string from the last change binding.
PR Close#34616
This change moves information from instructions to declarative position:
- `ɵɵallocHostVars(vars)` => `DirectiveDef.hostVars`
- `ɵɵelementHostAttrs(attrs)` => `DirectiveDef.hostAttrs`
When merging directives it is necessary to know about `hostVars` and `hostAttrs`. Before this change the information was stored in the `hostBindings` function. This was problematic, because in order to get to the information the `hostBindings` would have to be executed. In order for `hostBindings` to be executed the directives would have to be instantiated. This means that the directive instantiation would happen before we had knowledge about the `hostAttrs` and as a result the directive could observe in the constructor that not all of the `hostAttrs` have been applied. This further complicates the runtime as we have to apply `hostAttrs` in parts over many invocations.
`ɵɵallocHostVars` was unnecessarily complicated because it would have to update the `LView` (and Blueprint) while existing directives are already executing. By moving it out of `hostBindings` function we can access it statically and we can create correct `LView` (and Blueprint) in a single pass.
This change only changes how the instructions are generated, but does not change the runtime much. (We cheat by emulating the old behavior by calling `ɵɵallocHostVars` and `ɵɵelementHostAttrs`) Subsequent change will refactor the runtime to take advantage of the static information.
PR Close#34683
Fixes Ivy detecting changes inside child embedded views, even though they're detached.
Note that there's on subtlety here: I made the changes inside `refreshDynamicEmbeddedViews` rather than `refreshView`, because we support detecting changes on a detached view (evidenced by a couple of unit tests), but only if it's triggered directly from the view's `ChangeDetectorRef`, however we shouldn't be detecting changes in the detached child view when something happens in the parent.
Fixes#34816.
PR Close#34846
The major one that affects the angular repo is the removal of the bootstrap attribute in nodejs_binary, nodejs_test and jasmine_node_test in favor of using templated_args --node_options=--require=/path/to/script. The side-effect of this is that the bootstrap script does not get the require.resolve patches with explicitly loading the targets _loader.js file.
PR Close#34736
Currently ngtsc looks for the first `ConstructorDeclaration` when figuring out what the parameters are so that it can generate the DI instructions. The problem is that if a constructor has overloads, it'll have several `ConstructorDeclaration` members with a different number of parameters. These changes tweak the logic so it looks for the constructor implementation.
PR Close#34590
Unlike in View Engine, we currently reset the dirty state of
components in the check no changes change detection cycle.
This means that components cannot be marked as dirty from
view lifecycle hooks because the dirty state is reset and
the lifecycle hooks do not run in the check no changes CD cycle.
PR Close#34495
Kristiyan Kostadinov
Andrew Scott
Marcono1234
Sonu Kapoor
Jessica Janiuk
twerske
Misko Hevery
Andrew Kushnir
waterplea
JoostK
JiaLiPassion
Paul Gschwendtner
Andrew Scott
Kara Erickson
Joey Perrott
Matias Niemelä
Pawel Kozlowski
Pete Bacon Darwin
zhuyujie
Greg Magolan