This commit updates synthetic host property and listener instruction names to better align with other instructions.
The `ɵɵupdateSyntheticHostBinding` instruction was renamed to `ɵɵsyntheticHostProperty` (to match the `ɵɵhostProperty`
instruction name) and `ɵɵcomponentHostSyntheticListener` was renamed to `ɵɵsyntheticHostListener` since this
instruction is generated for both Components and Directives (so 'component' is removed from the name).
This PR is a followup after PR #35568.
PR Close#38150
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#38119
We currently use 16 bits to store information about nodes in a view.
The 16 bits give us 65536 entries in the array, but the problem is that while
the number is large, it can be reached by ~4300 directive instances with host
bindings which could realistically happen is a very large view, as seen in #37876.
Once we hit the limit, we end up overflowing which eventually leads to a runtime error.
These changes bump to using 20 bits which gives us around 1048576 entries in
the array or 16 times more than the current amount which could still technically
be reached, but is much less likely and the user may start hitting browser limitations
by that point.
I picked the 20 bit number since it gives us enough buffer over the 16 bit one,
while not being as massive as a 24 bit or 32 bit.
I've also added a dev mode assertion so it's easier to track down if it happens
again in the future.
Fixes#37876.
PR Close#38014
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
The `inertDocument` member is only needed when using the InertDocument
strategy. By separating the DOMParser and InertDocument strategies into
separate classes, we can easily avoid creating the inert document
unnecessarily when using DOMParser.
PR Close#37783
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
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
If we detect that an injectable class is inheriting from another injectable, we generate code that looks something like this:
```
const baseFactory = ɵɵgetInheritedFactory(Child);
@Injectable()
class Parent {}
@Injectable()
class Child extends Parent {
static ɵfac = (t) => baseFactory(t || Child)
}
```
This usually works fine, because the `ɵɵgetInheritedFactory` resolves to the factory of `Parent`, but the logic can break down if the `Child` class has a custom decorator. Custom decorators can return a new class that extends the original once, which means that the `ɵɵgetInheritedFactory` call will now resolve to the factory of the `Child`, causing an infinite loop.
These changes fix the issue by changing the inherited factory resolution logic so that it walks up the prototype chain class-by-class, while skipping classes that have the same factory as the class that was passed in.
Fixes#35733.
PR Close#37022
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
Dynamic embedded views were conceptually different from inline embedded views, but we have since
removed the inline embedded views so we now only have "embedded views".
See related refactoring work to remove inline embedded views in #34715
and #37073.
PR Close#37117
The _tViewNode field (that was marked as internal) on the ViewRef is not
necessery as a reference to a relevant TView is available as a local
variable.
PR Close#36814
The ActiveIndexFlag is no longer needed because we no longer have "inline embedded views".
There is only one type of embedded view so we do not need complex tracking for
inline embedded views.
HAS_TRANSPLANTED_VIEWS now takes the place of the ACTIVE_INDEX slot as a
simple boolean rather than being a shifted flag inside the ACTIVE_INDEX bits.
PR Close#37073
In 420b9be1c14ec6900692b641a3d89b99eb9c6e5b 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
Prior to this change, the `template` instruction logic was located in the `instructions/container.ts` file alongside embedded view instructions. Since unused embedded view instructions are removed in a previous commit, this commit renames `container.ts` -> `template.ts`, since only template-related instructions were retained.
PR Close#34715
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
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
Prior to this change, there was a problem while matching template attributes, which mistakenly took i18n attributes (that might be present in attrs array after template ones) into account. This commit updates the logic to avoid template attribute matching logic from entering the i18n section and as a result this also allows generating proper i18n attributes sections instead of keeping these attribute in plain form (with their values) in attribute arrays.
PR Close#36422
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
Prior to this commit, the `packages/core/src/render3/interfaces/query.ts` file used to import `QueryList` using `../../linker`, which contains a lot of re-exports and as a result, this one import caused a lot of circular deps cycles reported by the tool that checks such deps. In other places in the code the `QueryList` is imported using more narrow import (`linker/query_list`), so this commit uses the same pattern. This change allowed to reduce the number of known cycles from 343 to 207, the golden file was updated accordingly.
PR Close#36286
This commit augments the `FactoryDef` declaration of Angular decorated
classes to contain information about the parameter decorators used in
the constructor. If no constructor is present, or none of the parameters
have any Angular decorators, then this will be represented using the
`null` type. Otherwise, a tuple type is used where the entry at index `i`
corresponds with parameter `i`. Each tuple entry can be one of two types:
1. If the associated parameter does not have any Angular decorators,
the tuple entry will be the `null` type.
2. Otherwise, a type literal is used that may declare at least one of
the following properties:
- "attribute": if `@Attribute` is present. The injected attribute's
name is used as string literal type, or the `unknown` type if the
attribute name is not a string literal.
- "self": if `@Self` is present, always of type `true`.
- "skipSelf": if `@SkipSelf` is present, always of type `true`.
- "host": if `@Host` is present, always of type `true`.
- "optional": if `@Optional` is present, always of type `true`.
A property is only present if the corresponding decorator is used.
Note that the `@Inject` decorator is currently not included, as it's
non-trivial to properly convert the token's value expression to a
type that is valid in a declaration file.
Additionally, the `ComponentDefWithMeta` declaration that is created for
Angular components has been extended to include all selectors on
`ng-content` elements within the component's template.
This additional metadata is useful for tooling such as the Angular
Language Service, as it provides the ability to offer suggestions for
directives/components defined in libraries. At the moment, such
tooling extracts the necessary information from the _metadata.json_
manifest file as generated by ngc, however this metadata representation
is being replaced by the information emitted into the declaration files.
Resolves FW-1870
PR Close#35695
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
This commit performs a few updates to internal functions that would be required in upcoming changes to support synthetic host bindings in Directives.
* the `elementPropertyInternal` function was refactored to accept renderer as an argument (prior to that, there was a function that loads the renderer in some specific way for animation bindings)
* `elementPropertyInternal`, `elementAttributeInternal` and `listenerInternal` functions were updated to have a fixed set of arguments (for better performance)
* `elementPropertyInternal` and `elementAttributeInternal` functions were updated to take `tNode` as an argument instead of passing node index (that was used to retrieve `tNode` internally), in some cases we already have `tNode` available or we can retrieve it from the state
The refactoring was triggered by the need to pass different renderers to the `elementPropertyInternal` to support synthetic host bindings in Directives (see this comment for additional context: https://github.com/angular/angular/pull/35568/files#r388034584).
PR Close#35884
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 00f3c58bb929d9afa10ab3c4f1b9ee7a0077ba57.
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
`ɵɵgetInheritedFactory()` is called from generated code for a component which extends another class. This function is detected by Closure to have a side effect and is not able to tree shake the component as a result. Marking it with `noSideEffects()` tells Closure it can remove this function under the relevant tree shaking conditions.
PR Close#35769
`ɵɵNgOnChangesFeature()` would set `ngInherit`, which is a side effect and also not necessary. This was pulled out to module scope so the function itself can be pure. Since it only curries another function, the call is entirely unnecessary. Updated the compiler to only generate a reference to this function, rather than a call to it, and removed the extra curry indirection.
PR Close#35769
This marks the function are "pure" and eligible to be tree shaken by Closure. Without this, initializing `ngDevMode` is considered a side effect which prevents this function from being tree shaken and also any component which calls it.
PR Close#35769
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
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
