In some cases ivy expects projectable nodes to be passed in a different order
to ViewEngine. Specifically, ivy expects the catch-all ("*") to be at index
0, whereas ViewEngine expects it to be at its position at which it was parsed
in the template.
This commit adds one test that breaks under ivy and others that just describe
more accurately what happens in corner cases.
PR Close#27791
This patch is the first of a few patches which separates the
styling logic between template bindings (e.g. <div [style])
from host bindings (e.g. @HostBinding('style')). This patch
in particular introduces a series of host-specific styling
instructions and changes the existing set of template styling
instructions not to accept directives. The underyling code (which
communicates with the styling algorithm) still works as it did
before.
This PR also separates the styling instruction code into a separate
file and moves over all other instructions into an dedicated
instructions directory.
PR Close#29292
Prior to this change, we always recompile all Components/Directives/Pipes even if they were AOT-compiled and had no overrides. This is causing problems in case we try to recompile a Component with "templateUrl" or "styleUrls" (which were already resolved in case of AOT) and generally this unnecessary work that TestBed was doing is not required. This commit adds extra logic to check whether a Component/Directive/Pipe already have compiled NG def (like ngComponentDef) and whether there are no overrides present - in this case recompilation is skipped. Recompilation is also skipped in case a Component/Directive has only Provider overrides - in this situation providers resolver function is patched to reflect overrides. Provider overrides are very common in g3, thus this code path ensures no full recompilation.
PR Close#29294
Just updating comments in query-related things to make it easier for the next person that has to grok this for the first time.
Also adds a demo from @mhevery to one of the query specs
Related #29031
PR Close#29342
Just updating comments in query-related things to make it easier for the next person that has to grok this for the first time.
Also adds a demo from @mhevery to one of the query specs
Related #29031
PR Close#29342
Fixes host listeners being inherited twice, if the sub class has its own `propMetadata`. This is related to #29170 which fixed something similar, however all of the test cases there had a super class with some metadata and a sub class that didn't have any. The issue manifested itself in the `MatTreeToggle` which inherits a listener from the `CdkTreeToggle` and adds an extra `Input` of its own, causing the listener to be added twice.
PR Close#29353
Angular Ivy interprets inline static style/class attribute values as instructions that
are processed whilst an element gets created. Because these inline style values are
referenced by style/class bindings, their inline style values are applied at a later
stage. Despite them being eventually applied, their values should be applied earlier
before any directives are instantiated so that directive code can rely on any inline
style/class changes.
This patch ensures that all static style/class attribute values are applied (rendered)
on the element before directives are instantiated.
Jira Issue: FW-1133
PR Close#29269
Following my previous change for placeholders removal, some special code that was used to find the last created node was no longer needed and had wrong interactions with the *ngFor directive.
Removing it fixed the issue.
PR Close#29308
When injecting with `@Attribute`, namespaced attributes should not match (in order to have feature parity with View Engine).
This PR resolves FW-1137
PR Close#29257
BREAKING CHANGE:
Certain elements (like `<tr>` or `<col>`) require parent elements to be of a certain type by the HTML specification
(ex. <tr> can only be inside <tbody> / <thead>). Before this change Angular template parser was auto-correcting
"invalid" HTML using the following rules:
- `<tr>` would be wrapped in `<tbody>` if not inside `<tbody>`, `<tfoot>` or `<thead>`;
- `<col>` would be wrapped in `<colgroup>` if not inside `<colgroup>`.
This meachanism of automatic wrapping / auto-correcting was problematic for several reasons:
- it is non-obvious and arbitrary (ex. there are more HTML elements that has rules for parent type);
- it is incorrect for cases where `<tr>` / `<col>` are at the root of a component's content, ex.:
```html
<projecting-tr-inside-tbody>
<tr>...</tr>
</projecting-tr-inside-tbody>
```
In the above example the `<projecting-tr-inside-tbody>` component culd be "surprised" to see additional
`<tbody>` elements inserted by Angular HTML parser.
PR Close#29219
Previously, ngtsc would resolve forward references while evaluating the
bootstrap, declaration, imports, and exports fields of NgModule types.
However, when generating the resulting ngModuleDef, the forward nature of
these references was not taken into consideration, and so the generated JS
code would incorrectly reference types not yet declared.
This commit fixes this issue by introducing function closures in the
NgModuleDef type, similarly to how NgComponentDef uses them for forward
declarations of its directives and pipes arrays. ngtsc will then generate
closures when required, and the runtime will unwrap them if present.
PR Close#29198
Prior to this change default selector for Components was not applied in case selector is missing or defined as an empty string. This update aligns this behavior between Ivy and VE: now default selector is used for Components when it's needed. Directives with empty selector are not allowed and trigger a compile-time error in both Ivy and VE.
PR Close#29239
Currently with ViewEngine, if someone runs the platform's
`bootstrapModule` method in order to boostrap a module in
JIT mode, external component resources are properly resolved
*automatically*.
Currently with Ivy, the developer would need to manually call
`resolveComponentResources` in order to asynchronously fetch
the determined external component resources. In order to make
this backwards compatible with ViewEngine, and also since
platforms can already specify a `ResourceLoader` compiler
provider, we need to automatically resolve all external
component resources on module bootstrap.
--
Since the `ResourceLoader` is part of the `@angular/compiler`,
because ViewEngine performed the factory creation in the compiler,
we can't access the `ResourceLoader` token from within core.
In order to workaround this without introducing a breaking change,
we just proxy the `ResourceLoader` token to `core` through the
compiler facade. In the future, we should be able to move the
`ResourceLoader` to core when ViewEngine code no longer exists in
the `@angular/compiler`.
PR Close#29083
Dynamic nodes are created at the end of the view stack, but we were removing all the placeholders between `i18nStart` and the last created node index, instead of removing everything between `i18nStart` and `i18nEnd`. This caused errors when dynamic nodes where created in multiple i18n blocks because we would remove all of the dynamic nodes created in the previous i18n blocks.
PR Close#29252
Prior to this commit, i18n instructions (i18n, i18nStart) were generated before listener instructions. As a result, event listeners were attached to the wrong element (text node, not the parent element). This change updates the order of instructions and puts i18n ones after listeners, to make sure listeners are attached to the right elements.
PR Close#29173
The content projection mechanism is static, in that it only looks at the static
template nodes before directives are matched and change detection is run.
When you have a selector-based content projection the selection is based
on nodes that are available in the template.
For example:
```
<ng-content selector="[some-attr]"></ng-content>
```
would match
```
<div some-attr="..."></div>
```
If you have an inline-template in your projected nodes. For example:
```
<div *ngIf="..." some-attr="..."></div>
```
This gets pre-parsed and converted to a canonical form.
For example:
```
<ng-template [ngIf]="...">
<div some-attr=".."></div>
</ng-template>
```
Note that only structural attributes (e.g. `*ngIf`) stay with the `<ng-template>`
node. The other attributes move to the contained element inside the template.
When this happens in ivy, the ng-template content is removed
from the component template function and is compiled into its own
template function. But this means that the information about the
attributes that were on the content are lost and the projection
selection mechanism is unable to match the original
`<div *ngIf="..." some-attr>`.
This commit adds support for this in ivy. Attributes are separated into three
groups (Bindings, Templates and "other"). For inline-templates the Bindings
and "other" types are hoisted back from the contained node to the `template()`
instruction, so that they can be used in content projection matching.
PR Close#29041
This commit adds a new `AttributeMarker` type that will be used, in a
future commit, to mark attributes as coming from an inline-template
expansion, rather than the element that is being contained in the template.
PR Close#29041
Some tests in exports spec rely on the exact output of innerHTML. In IE11 the order of attributes might change, thus causing tests to fail (in case an element contains more than one attribute). This commit avoids innerHTML usage and performs the necessary checks via element properties.
PR Close#29127
Some of the export tests had assertions that relied on capitalization
of attributes in the DOM. IE treats capitalization somewhat differently,
so our SauceLabs tests were failing.
This commit tweaks the tests so that the assertions do not rely on
attributes to be capitalized or not.
PR Close#29125
Angular supports having a component extend off of a parent component.
When this happens, all annotation-level data is inherited including styles
and classes. Up until now, Ivy only paid attention to static styling
values on the parent component and not the child component. This patch
ensures that both the parent's component and child component's styling
data is merged and rendered accordingly.
Jira Issue: FW-1081
PR Close#29015
Currently we only reset the `Attached` flag of a view if it is detached through its parent, however this means that if a root view is destroyed, its flag will never be reset. This manifested itself in one of the Material tests where we were destroying the root view.
This PR resolves FW-1130.
PR Close#29064
Currently if a user accidentally calls ViewContainerRef.insert() with
a view that has already been attached, we do not clean up the references
properly, so we create a view tree with a cycle. This causes an infinite
loop when the view is destroyed.
This PR ensures that we fall back to ViewContainerRef.move() behavior
if we try to insert a view that is already attached. This fixes the
cycle and honors the user intention.
PR Close#29047
When web-animations and/or CSS keyframes are used for animations certain
CSS style values (such as `display` and `position`) may be ignored by a
keyframe-based animation. Angular should special-case these styles to
ensure that they get applied as inline styles throughout the duration of
the animation.
Closes#24923Closes#25635
Jira Issue: FW-1091
Jira Issue: FW-1092
PR Close#28911
Prior to this change i18n block bindings were converted to Expressions right away (once we first access them), when in non-i18n cases we processed them differently: the actual conversion happens at instructions generation. Because of this discrepancy, the output for bindings in i18n blocks was generated incorrectly (with invalid indicies in pipeBindN fns and invalid references to non-existent local variables). Now the bindings processing is unified and i18nExp instructions should contain right bind expressions.
PR Close#28969
Karma is not configured to retrieve the imported scripts using those
absolute deep paths. Using relative paths instead.
See [here][1] for an example failing job.
[1]: https://circleci.com/gh/angular/angular/220751
PR Close#29009
In the past, the sanitizer would remove unsafe elements, but still
traverse and sanitize (and potentially preserve) their content. This was
problematic in the case of `<style></style>` tags, whose content would
be converted to HTML text nodes.
In order to fix this, the sanitizer's behavior was changed in #25879 to
ignore the content of _all_ unsafe elements. While this fixed the
problem with `<style></style>` tags, it unnecessarily removed the
contents for _any_ unsafe element. This was an unneeded breaking change.
This commit partially restores the old sanitizer behavior (namely
traversing content of unsafe elements), but introduces a list of
elements whose content should not be traversed if the elements
themselves are considered unsafe. Currently, this list contains `style`,
`script` and `template`.
Related to #25879 and #26007.
Fixes#28427
PR Close#28804
Currently if an embedded view contains projected nodes, its `rootNodes` array will include `null` instead of the root nodes inside the projection slot. This manifested itself in one of the Material unit tests where we stamp out a template and then move its `rootNodes` into the overlay container.
This PR is related to FW-1087.
PR Close#28951
For efficiency reasons we often put several different data types (`RNode`, `LView`, `LContainer`,
`StylingContext`) in same location in `LView`. This is because we don't want to pre-allocate
space
for it because the storage is sparse. This file contains utilities for dealing with such data
types.
How do we know what is stored at a given location in `LView`.
- `Array.isArray(value) === false` => `RNode` (The normal storage value)
- `Array.isArray(value) === true` => than the `value[0]` represents the wrapped value.
- `typeof value[TYPE] === 'object'` => `LView`
- This happens when we have a component at a given location
- `typeof value[TYPE] === 'number'` => `StylingContext`
- This happens when we have style/class binding at a given location.
- `typeof value[TYPE] === true` => `LContainer`
- This happens when we have `LContainer` binding at a given location.
NOTE: it is assumed that `Array.isArray` and `typeof` operations are very efficient.
PR Close#28947
`LView`, `LContainer`, `StylingContext` are all arrays which wrap either
an `HTMLElement`, `LView`, `LContainer`, `StylingContext`. It is often
necessary to retrieve the correct type of element from the location
which means that we often have to wrap the arrays. Logically it makes
more sense if the thing which we are wrapping is at `0` location. Also
it may be more performant since data is more local which may result in
more L2 cache hits in CPU.
PR Close#28947
This change contains conditionally attached classes which provide human readable (debug) level
information for `LView`, `LContainer` and other internal data structures. These data structures
are stored internally as array which makes it very difficult during debugging to reason about the
current state of the system.
Patching the array with extra property does change the array's hidden class' but it does not
change the cost of access, therefore this patching should not have significant if any impact in
`ngDevMode` mode. (see: https://jsperf.com/array-vs-monkey-patch-array)
So instead of seeing:
```
Array(30) [Object, 659, null, …]
```
```
LViewDebug {
views: [...],
flags: {attached: true, ...}
nodes: [
{html: '<div id="123">', ..., nodes: [
{html: '<span>', ..., nodes: null}
]}
]
}
```
PR Close#28945
Google3 detected circular references here, so splitting up this rather hodge-podge list of functions into slightly better organizational units.
PR Close#28382
- TView no longer stores childIndex
- LView now as CHILD_HEAD and CHILD_TAIL
TView used to store the head of the list, therefor all LViews had to have the same head, which is incorrect.
PR Close#28382
- Removes CONTAINER_INDEX
- LView[PARENT] now contains LContainer when necessary
- Removes now unused arguments to methods after refactor
PR Close#28382
During build time we remap particular property bindings, because their names don't match their attribute equivalents (e.g. the property for the `for` attribute is called `htmlFor`). This breaks down if the particular element has an input that has the same name, because the property gets mapped to something invalid.
The following changes address the issue by mapping the name during runtime, because that's when directives are resolved and we know all of the inputs that are associated with a particular element.
PR Close#28765
Fixes the `ngOnDestroy` hook on a component or directive being called twice, if the type is also registered as a provider.
This PR resolves FW-1010.
PR Close#28470
Prior to this fix, both the `NgStyle` and `NgClass` directives made use
of `Renderer2` and this dependency raised issues for future versions of
Angular that cannot inject it. This patch ensures that there are two
versions of both directives: one for the VE and another for Ivy.
Jira Issue: FW-882
PR Close#28711
I18n can change the order of the nodes, or insert new dynamic nodes. When that happens it can break the existing links (`TNode.next`) or even create loops:
```
div1 → div2 → div3 → div4 → div5
```
Can become:
```
div1 → div4 → div2 → div3 div5
🡑 │
└─────────────┘
```
This PR fixes this issue by recreating the broken links between those nodes.
PR Close#28827
This commit adds support for the `static: true` flag in `ContentChild`
queries. Prior to this commit, all `ContentChild` queries were resolved
after change detection ran. This is a problem for backwards
compatibility because View Engine also supported "static" queries which
would resolve before change detection.
Now if users add a `static: true` option, the query will be resolved in
creation mode (before change detection runs). For example:
```ts
@ContentChild(TemplateRef, {static: true}) template !: TemplateRef;
```
This feature will come in handy for components that need
to create components dynamically.
PR Close#28811
This commit adds support for the `static: true` flag in
`ViewChild` queries. Prior to this commit, all `ViewChild`
queries were resolved after change detection ran. This is
a problem for backwards compatibility because View Engine
also supported "static" queries which would resolve before
change detection.
Now if users add a `static: true` option, the query will be
resolved in creation mode (before change detection runs).
For example:
```ts
@ViewChild(TemplateRef, {static: true}) template !: TemplateRef;
```
This feature will come in handy for components that need
to create components dynamically.
PR Close#28811
Prior to this commit, the timing of `ViewChild`/`ContentChild` query
resolution depended on the results of each query. If any results
for a particular query were nested inside embedded views (e.g.
*ngIfs), that query would be resolved after change detection ran.
Otherwise, the query would be resolved as soon as nodes were created.
This inconsistency in resolution timing had the potential to cause
confusion because query results would sometimes be available in
ngOnInit, but sometimes wouldn't be available until ngAfterContentInit
or ngAfterViewInit. Code depending on a query result could suddenly
stop working as soon as an *ngIf or an *ngFor was added to the template.
With this commit, users can dictate when they want a particular
`ViewChild` or `ContentChild` query to be resolved with the `static`
flag. For example, one can mark a particular query as `static: false`
to ensure change detection always runs before its results are set:
```ts
@ContentChild('foo', {static: false}) foo !: ElementRef;
```
This means that even if there isn't a query result wrapped in an
*ngIf or an *ngFor now, adding one to the template later won't change
the timing of the query resolution and potentially break your component.
Similarly, if you know that your query needs to be resolved earlier
(e.g. you need results in an ngOnInit hook), you can mark it as
`static: true`.
```ts
@ViewChild(TemplateRef, {static: true}) foo !: TemplateRef;
```
Note: this means that your component will not support *ngIf results.
If you do not supply a `static` option when creating your `ViewChild` or
`ContentChild` query, the default query resolution timing will kick in.
Note: This new option only applies to `ViewChild` and `ContentChild`
queries, not `ViewChildren` or `ContentChildren` queries, as those types
already resolve after CD runs.
PR Close#28810
This commit turns off an unknown binding test that is specific
to the View Engine implementation. In Ivy, we do property validation
at runtime for jit, so this test does not apply.
PR Close#28740
Accounts for schemas in when validating properties in Ivy.
This PR resolves FW-819.
A couple of notes:
* I had to rework the test slightly, in order to have it fail when we expect it to. The one in master is passing since Ivy's validation runs during the update phase, rather than creation.
* I had to deviate from the design in FW-819 and not add an `enableSchema` instruction, because the schema is part of the `NgModule` scope, however the scope is only assigned to a component once all of the module's declarations have been resolved and some of them can be async. Instead, I opted to have the `schemas` on the component definition.
PR Close#28637
Previously `DebugNode.classes` and `DebugNode.styles` mistakenly used an
object that is only *sometimes* a `StylingContext`. Also fixes a mistake
in `debug_node_spec.ts` where a test component was not declared in the
testing module.
There is still a bug here where `DebugNode` is not exposing *static*
values. This will need to be fixed in a follow up.
PR Close#28709
Prior to this fix if a root component was instantiated it create host
bindings, but never render them once update mode ran unless one or more
slot-allocated bindings were issued. Since styling in Ivy does not make
use of LView slots, the host bindings function never ran on the root
component.
This fix ensures that the `hostBindings` function does run for a root
component and also renders the schedlued styling instructions when
executed.
Jira Issue: FW-1062
PR Close#28664
While marking a given views tree as dirty we should go all the way to the
root of the views tree and cross boundaries of dynamically inserted views.
In other words the markForCheck functionality should consider parents of
dynamically inserted views.
PR Close#28687
Prior to this change we used current injector implementation for module injector, which was causing problems and produces circular dependencies in case the same token is referenced (with @SkipSelf flag) in the `deps` array. The origin of the problem was that once `directiveInject` implementation becomes active, it was used for module injector as well, thus searching deps in Component/Directive DI scope. This fix sets `injectInjectorOnly` implementation for module injector to resolve the problem.
PR Close#28667
Prior to this update we had separate contentQueries and contentQueriesRefresh functions to handle creation and update phases. This approach was inconsistent with View Queries, Host Bindings and Template functions that we generate for Component/Directive defs. Now the mentioned 2 functions are combines into one (contentQueries), creation and update logic is separated with RenderFlags (similar to what we have in other generated functions).
PR Close#28503
For TypeScript compilation units that have the "strictFunctionTypes"
option enabled, an error would be produced for Ivy's definition fields
in declaration files in the case of inheritance across directives or
pipes.
This change loosens the definition types to allow for subtypes of the
defined type where necessary.
A test package that has the "strict" option enabled verifies that we
won't regress in environments where strict type checking is enabled.
Fixes#28079
PR Close#28634
Previously, using a pipe in an input binding on an ng-template would
evaluate the pipe in the context of node that was processed before the
template. This caused the retrieval of e.g. ChangeDetectorRef to be
incorrect, resulting in one of the following bugs depending on the
template's structure:
1. If the template was at the root of a view, the previously processed
node would be the component's host node outside of the current view.
Accessing that node in the context of the current view results in a crash.
2. For templates not at the root, the ChangeDetectorRef injected into the
pipe would correspond with the previously processed node. If that node
hosts a component, the ChangeDetectorRef would not correspond with the
view that the ng-template is part of.
The solution to the above problem is two-fold:
1. Template compilation is adjusted such that the template instruction
is emitted before any instructions produced by input bindings, such as
pipes. This ensures that pipes are evaluated in the context of the
template's container node.
2. A ChangeDetectorRef can be requested for container nodes.
Fixes#28587
PR Close#27565
There are some differences in how ivy maps template source
compared to View Engine. In this commit we recreate the View Engine
tests for ivy.
PR Close#28055
When we resolve a component `templateUrl` we copy the contents of the
resolved template file into the `template` property.
Previously we would then remove the `templateUrl` to indicate that
the component has been resolved. But this meant that we no longer had
access to the URL of the original template file. This is essential for
diagnostics messages about the template compilation.
Now the existence of the `template` property overrides the existence of
`templateUrl`, which allows us to keep the `templateUrl` property.
PR Close#28055
Due to the fact that host nodes no longer match in ContentChild queries in Ivy, we disable test that was enabled previously in other commit.
PR Close#28660
Up until now, `[style]` and `[class]` bindings (the map-based ones) have only
worked as template bindings and have not been supported at all inside of host
bindings. This patch ensures that multiple host binding sources (components and
directives) all properly assign style values and merge them correctly in terms
of priority.
Jira: FW-882
PR Close#28246
Before this fix our ViewRef implementation assumed that checkNoChanges can be
only called on component views. In reality checkNoChanges can be also called on
embedded views (ex.: when an embedded view is attached to ApplicationRef).
PR Close#28644
Previously, it wasn't possible to compile template that contains pipe in context of ternary operator `{{ 1 ? 2 : 0 | myPipe }}` due to the error `Error: Illegal state: Pipes should have been converted into functions. Pipe: async`.
This PR fixes a typo in expression parser so that pipes are correctly converted into functions.
PR Close#28635
Prior to this change in Ivy we had strict check that disabled non-unique #localRefs usage within a given template. While this limitation was technically present in View Engine, in many cases View Engine neglected this restriction and as a result, some apps relied on a fact that multiple non-unique #localRefs can be defined and utilized to query elements via @ViewChild(ren) and @ContentChild(ren). In order to provide better compatibility with View Engine, this commit removes existing restriction.
As a part of this commit, are few tests were added to verify VE and Ivy compatibility in most common use-cases where multiple non-unique #localRefs were used.
PR Close#28627
There is nothing browser specific in those tests and fakeAsync is supported on node.
Testing / debugging on node is often faster than on Karma.
PR Close#28593
This commit adds a devMode-only check which will throw if a user
attempts to bind a property that does not match a directive
input or a known HTML property.
Example:
```
<div [unknownProp]="someValue"></div>
```
The above will throw because "unknownProp" is not a known
property of HTMLDivElement.
This check is similar to the check executed in View Engine during
template parsing, but occurs at runtime instead of compile-time.
Note: This change uncovered an existing bug with host binding
inheritance, so some Material tests had to be turned off. They
will be fixed in an upcoming PR.
PR Close#28537
Pete Bacon Darwin
Kristiyan Kostadinov
Marc Laval
Greg Magolan
Ben Lesh
Matias Niemelä
Andrew Kushnir
Alex Eagle
Olivier Combe
Adam Plumer
cexbrayat
Pawel Kozlowski
Alex Rickabaugh
Paul Gschwendtner
Kara Erickson
George Kalpakas
Wassim Chegham
Misko Hevery
Jeremy Elbourn
Trotyl Yu
JoostK
Alexey Zuev