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
DebugElement.query also matches elements that may have been created
outside of Angular (ex: with `document.appendChild`). If those matched
DebugElements are in turn used to query for more elements, an error
occurs because the first step in queryAll is to load the LContext.
PR Close#34687
Default currency code in CurrencyPipe is currently hardcoded to USD
and is not configurable. This commit allows the default currency code
to be configurable by adding a DEFAULT_CURRENCY_CODE injection token.
Example:
```
providers: [{ provide: DEFAULT_CURRENCY_CODE, useValue: "GBP" }]
...
{{ 123.45 | currency }} // outputs £123.45 as opposed to always $123.45 before
```
Closes: #25461
PR Close#32584
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
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#34589
This commit improves `ExpressionChangedAfterChecked` error message for attributes by including attribute name and the content of the entire expression that contains interpolation(s). In order to achieve that, metadata is now stored in `TData` array when `attribute` and `attributeInterpolate` instructions are being called (similar to `property` and `propertyInterpolate` instructions).
PR Close#34505
Fixes classes with trailing or leading space that are passed to `ngClass` (e.g. `{'foo ': bar}`) not being applied in Ivy. The issue comes from the fact that when the styling differ builds up the style map it uses the trimmed key to look up the value in the map that uses non-trimmed keys.
Fixes#34476.
PR Close#34539
Follow-up from [this discussion](https://github.com/angular/angular/pull/33419#discussion_r339296216). In Ivy we don't use the schema to validate tag names, but instead we use feature detection to figure out whether an element is supported. While this should generally be more accurate, it'll also end up throwing for some more innocent cases. E.g. now Ivy throws an error for `main` elements in IE which is accurate since IE doesn't support the element, but is annoying since there is no functionality attached.
These changes switch to logging a warning instead, similarly to what we're doing for unknown properties.
PR Close#34524
The `getProjectAsAttrValue` in `node_selector_matcher` finds the
ProjectAs marker and then additionally checks that the marker appears in
an even index of the node attributes because "attribute names are stored
at even indexes". This is true for "regular" attribute bindings but
classes, styles, bindings, templates, and i18n do not necessarily follow
this rule because there can be an uneven number of them, causing the
next "special" attribute "name" to appear at an odd index. To address
this issue, ensure ngProjectAs is placed right after "regular"
attributes.
PR Close#34617
Prior to this commit, there were no `advance` instructions generated before `i18nExp` instructions and as a result, lifecycle hooks for components used inside i18n blocks were flushed too late. This commit adds the logic to generate `advance` instructions in front of `i18nExp` ones (similar to what we have in other places like interpolations, property bindings, etc), so that the necessary lifecycle hooks are flushed before expression value is captured.
PR Close#34436
The main logic of the `InheritDefinitionFeature` is to go through the prototype chain of a given Component and merge all Angular-specific information onto that Component def. The problem happens in case there is a Component in a hierarchy that also contains the `InheritDefinitionFeature` (i.e. it extends some other Component), so it inherits all Angular-specific information from its super class. As a result, the root Component may end up having duplicate information inherited from different Components in hierarchy.
Let's consider the following structure: `GrandChild` extends `Child` that extends `Base` and the `Base` class has a `HostListener`. In this scenario `GrandChild` and `Child` will have `InheritDefinitionFeature` included into the `features` list. The processing will happend in the following order:
- `Child` inherits `HostListener` from the `Base` class
- `GrandChild` inherits `HostListener` from the `Child` class
- since `Child` has a parent, `GrandChild` also inherits from the `Base` class
The result is that the `GrandChild` def has duplicated host listener, which is not correct.
This commit introduces additional logic that checks whether we came across a def that has `InheritDefinitionFeature` feature (which means that this def already inherited information from its super classes). If that's the case, we skip further fields-related inheritance logic, but keep going though the prototype chain to look for super classes that contain other features (like NgOnChanges), that we need to invoke for a given Component def.
PR Close#34244
Fixes an error that is thrown when a provider is overridden in `TestBed`, if the module definition of one of the imported modules uses a function for the `imports` that is set via `setNgModuleScope`. The problem was that we have a `for...of` loop that assumes that the imports are an array, but they can also be a function. This was handled correctly in other places, but this one was missed.
Note that the above-mentioned error is only thrown at runtime when the code is transpiled to es6. In es5 TS generates a call to a helper that handles the error silently so the attached unit test only fails in es6.
Fixes#34623.
PR Close#34629
Prior to this change, the ExpressionChangedAfterChecked error thrown in Ivy was missing useful information that was available in View Engine, specifically: missing property name for proprty bindings and also the content of the entire property interpolation (only a changed value was displayed) if one of expressions was changed unexpectedly. This commit improves the error message by including the mentioned information into the error text.
PR Close#34381
The ordering matters because we don't currently throw if multiple
configurations are provided (i.e. provider has *both* useExisting and
useFactory). We should actually throw an error in this case, but to
avoid another breaking change in v9, this PR simply aligns the Ivy
behavior with ViewEngine.
PR Close#34433
Currently we only run Saucelabs on PRs using the legacy View Engine
build. Switching that build to Ivy is not trivial and there are various
options:
1. Updating the R3 switches to use POST_R3 by default. At first glance,
this doesn't look easy because the current ngtsc switch logic seems to
be unidirectional (only PRE_R3 to POST_R3).
2. Updating the legacy setup to run with Ivy. This sounds like the easiest
solution at first.. but it turns out to be way more complicated. Packages
would need to be built with ngtsc using legacy tools (i.e. first building
the compiler-cli; and then building packages) and View Engine only tests
would need to be determined and filtered out. Basically it will result in
re-auditing all test targets. This is contradictory to the fact that we have
this information in Bazel already.
3. Creating a new job that runs tests on Saucelabs with Bazel. We specify
fine-grained test targets that should run. This would be a good start
(e.g. acceptance tests) and also would mean that we do not continue maintaining
the legacy setup..
This commit implements the third option as it allows us to move forward
with the general Bazel migration. We don't want to spend too much time
on our legacy setup since it will be removed anyway in the future.
PR Close#34277
When we log DI errors we get the name of the provider via `SomeClass.name`. In IE functions that inherit from other functions don't have their own `name`, but they take the `name` from the lowest parent in the chain, before `Function`. I've added some changes to fall back to parsing out the function name from the function's string form.
PR Close#34305
The way definitions are added in JIT mode is through `Object.defineProperty`, but the problem is that in IE10 properties defined through `defineProperty` won't be inherited which means that inheriting injectable definitions no longer works. These changes add a workaround only for JIT mode where we define a fallback method for retrieving the definition. This isn't ideal, but it should only be required until v10 where we'll no longer support inheriting injectable definitions from undecorated classes.
PR Close#34305
We've got some tests that assert that the generate DOM looks correct. The problem is that IE changes the attribute order in `innerHTML` which caused the tests to fail. I've reworked the relevant tests not to assert directly against `innerHTML`.
PR Close#34305
This commit fixes a couple issues that prevent `class_binding` benchmark from running: moving constants requires by the `benchmark` function before function declaration and referencing correct consts in template instructions.
PR Close#34242
The `ModuleWithProviders` type has an optional type parameter that
should be specified to indicate what NgModule class will be provided.
This enables the Ivy compiler to statically determine the NgModule type
from the declaration files. This type parameter will become required in
the future, however to aid in the migration the compiler will detect
code patterns where using `ModuleWithProviders` as return type is
appropriate, in which case it transforms the emitted .d.ts files to
include the generic type argument.
This should reduce the number of occurrences where `ModuleWithProviders`
is referenced without its generic type argument.
Resolves FW-389
PR Close#34235
Prior to this commit, values wrapped into SafeStyle were not handled correctly in [style.prop] bindings in case style sanitizer is present (when template contains some style props that require sanitization). Style sanitizer was not unwrapping values in case a given prop doesn't require sanitization.As a result, wrapped values were used as final styling values (see https://github.com/angular/angular/blob/master/packages/core/src/render3/styling/bindings.ts#L620). This commit updates the logic to unwrap safe values in sanitizer in case no sanitization is required.
PR Close#34286
Occasionally a factory function needs to be generated for an "invalid"
constructor (one with parameters types which aren't injectable). Typically
this happens in JIT mode where understanding of parameters cannot be done in
the same "up-front" way that the AOT compiler can.
This commit changes the JIT compiler to generate a new `invalidFactoryDep`
call for each invalid parameter. This instruction will error at runtime if
called, indicating both the index of the invalid parameter as well as (via
the stack trace) the factory function which was generated for the type being
constructed.
Fixes#33637
PR Close#33739
In Ivy, if you do:
`TestBed.configureTestingModule({providers: [{provide: Service}]});`
the injector will attempt to inject Service as if it was simply listed
in the providers array like `{providers: [Service]}`
This fixes an inconsistency when similarly providing an override with no
`useValue` or `useFactory`.
PR Close#33769
Prior to this commit, calling change detection for destroyed views resulted in errors being thrown in some cases. This commit adds a check to make sure change detection is invoked for non-destroyed views only.
PR Close#34241
Prior to this commit, there was no check in R3TestBed to verify that metadata is resolved using a given Type. That leads to some cryptic error messages (when TestBed tries to compile a Type without having metadata) in case TestBed override functions receive unexpected Types (for example a Directive is used in `TestBed.overrideComponent` call). This commit adds the necessary checks to verify metadata presence before TestBed tries to (re)compile a Type.
PR Close#34204
Prior to this commit, if a template (for example, generated using structural directive such as *ngIf) contains `ngProjectAs` attribute, it was not included into attributes array in generated code and as a result, these templates were not matched at runtime during content projection. This commit adds the logic to append `ngProjectAs` values into corresponding element's attribute arrays, so content projection works as expected.
PR Close#34200
Prior to this commit, i18n runtime code failed with the exception saying that no provider was found for ChangeDetectorRef for a pipe used in ICU. The problem happened because the underlying `createViewRef` function was not taking into account IcuContainer as a valid TNodeType. This commit updates the `createViewRef` function to return corresponding ViewRef for TNodeType.IcuContainer.
PR Close#34198
Fixes ngtsc incorrectly logging an unknown element diagnostic for HTML elements that are inside an SVG `foreignObject` with the `xhtml` namespace.
Fixes#34171.
PR Close#34178
Prior to this commit, the unknown element can happen twice for AOT-compiled components: once during compilation and once again at runtime. Due to the fact that `schemas` information is not present on Component and NgModule defs after AOT compilation, the second check (at runtime) may fail, even though the same check was successful at compile time. This commit updates the code to avoid the second check for AOT-compiled components by checking whether `schemas` information is present in a logic that executes the unknown element check.
PR Close#34024
Use angular_ivy_enabled to determine if Ivy is being used for the ivy_test_selector.ts symbols.
Additionally, remove the reflect_metadata genrules as we not longer have a "jit" compile option
so all possible invocations result in the same generated file. Instead we can just commit this
file.
PR Close#33983
Since config=ivy now sets the define=compile flag and the define=angular_ivy_enabled
flag to cause usage of Ivy, we can update all of the documentation and scripts that
reference compile=aot to use config=ivy.
PR Close#33983
Prior to this commit, there was a runtime check in i18n logic to make sure "other" case is always present in an ICU. That was not a requirement in View Engine, so ICUs that previously worked may produce errors. This commit removes that restriction and adds support for ICUs without "other" cases.
PR Close#34042
When inserting a `viewRef` it is possible to not provide
an `index`, which is regarded as appending to the end of
the container.
If the `viewRef` already exists in the container, then
this results in a move. But there was a fault in the logic
that computed where to insert the `viewRef` that did not
account for the fact that the `viewRef` was already in
the container, so the insertion `index` was outside the
bounds of the array.
Fixes#33924
PR Close#34052
Prior to this commit, all styles extracted from Component's template (defined using <style> tags) were ignored by JIT compiler, so only `styles` array values defined in @Component decorator were used. This change updates JIT compiler to take styles extracted from the template into account. It also ensures correct order where `styles` array values are applied first and template styles are applied second.
PR Close#34017
This commit fixes a compatibility bug where pre-order lifecycle
hooks (onInit, doCheck, OnChanges) for directives on the same
host node were executed based on the order the directives were
matched, rather than the order the directives were instantiated
(i.e. injection order).
This discrepancy can cause issues with forms, where it is common
to inject NgControl and try to extract its control property in
ngOnInit. As the NgControl directive is injected, it should be
instantiated before the control value accessor directive (and
thus its hooks should run first). This ensures that the NgControl
ngOnInit can set up the form control before the ngOnInit
for the control value accessor tries to access it.
Closes#32522
PR Close#34026
Micro-benchmarks were broken after we've introduced concept of
DECLARATION_COMPONENT_VIEW on LView (after this change embedded
views must have a pointer to a parent LView).
PR Close#34031
The root view case is already covered by the existing code in the
getRenderParent function so no need to have an explicit checks
(and associated memory reads) again.
PR Close#33988
The assertion that we have in the `directiveInject` instruction is too restrictive and we came across some pattern where it throws unnecessarily. This commit removes that assertion for now and more detailed investigation is needed to decide is we need to restrict the set of TNodeType again.
This commit also adds a test which triggered the TNodeType.View to come up in the `directiveInject` instruction, so it might be useful to avoid regressions during further refactoring.
PR Close#33948
NgModule compilation in JIT mode (that is also used in TestBed) caches module scopes on NgModule defs (using `transitiveCompileScopes` field). Module overrides (defined via TestBed.overrideModule) may invalidate this data by adding/removing items in `declarations` list. This commit forces TestBed to recalculate transitive scopes in case module overrides are present, so TestBed always gets the most up-to-date information.
PR Close#33787
This change enables "var(--my-var)" to pass through the style sanitizer.
After consulation with our security team, allowing these doesn't create
new attack vectors, so the sanitizer doesn't need to strip them.
Fixes parts of #23485 related to the sanitizer, other use cases discussed
there related to binding have been addressed via other changes to the
class and style handling in the runtime.
Closes#23485
PR Close#33841
Originally, QueryList implemented Iterable and provided a Symbol.iterator
on its prototype. This caused issues with tree-shaking, so QueryList was
refactored and the Symbol.iterator added in its constructor instead. As
part of this change, QueryList no longer implemented Iterable directly.
Unfortunately, this meant that QueryList was no longer assignable to
Iterable or, consequently, NgIterable. NgIterable is used for NgFor's input,
so this meant that QueryList was not usable (in a type sense) for NgFor
iteration. View Engine's template type checking would not catch this, but
Ivy's did.
As a fix, this commit adds the declaration (but not the implementation) of
the Symbol.iterator function back to QueryList. This has no runtime effect,
so it doesn't affect tree-shaking of QueryList, but it ensures that
QueryList is assignable to NgIterable and thus usable with NgFor.
Fixes#29842
PR Close#33536
This refactorings clearly separates the first and subsequent creation execution
of the `template` instruction. This approach has the following benefits:
- it is clear what happens during the first vs. subsequent executions;
- we can avoid several memory reads and checks after the first creation pass
(there is measurable performance improvement on various benchmarks);
- the template instructions becomes smaller and should become a candidate
for optimisations / inlining faster;
PR Close#33856
When asking for a ViewContainerRef on <ng-container> we do reuse <ng-container> comment
node as a LContainer's anachor. Before this fix the act of re-using a <ng-container>'s
comment node would result in this comment node being re-appended to the DOM in the wrong
place. With the fix in this PR we make sure that re-using <ng-container>'s comment node
doesn't result in unwanted DOM manipulation (ng-gontainer's comment node is already part
of the DOM and doesn't have to be re-created / re-appended).
PR Close#33816
Prior to this commit the assert that we have in `directiveInject` (assert introduced recently) didn't include IcuContainer TNode type and as a result, the error is thrown in case pipes with dependencies are used inside ICUs. This commit extends the assert to allow for IcuContainer TNode types.
PR Close#33832
Currently if a consumer does something like the following, the object literal will be shared across the two elements and any instances of the component template. The same applies to array literals:
```
<div [someDirective]="{}"></div>
<div [someDirective]="{}"></div>
```
These changes make it so that we generate a pure function even if an object is constant so that each instance gets its own object.
Note that the original design for this fix included moving the pure function factories into the `consts` array. In the process of doing so I realized that pure function are also used inside of directive host bindings which means that we don't have access to the `consts`.
These changes also:
* Fix an issue that meant that the `pureFunction0` instruction could only be run during creation mode.
* Make the `getConstant` utility slightly more convenient to use. This isn't strictly required for these changes to work, but I had made it as a part of a larger refactor that I ended up reverting.
PR Close#33705
This is a breaking change in nodejs rules 0.40.0 as part of the API review & cleanup for the 1.0 release. Their APIs are identical as ts_web_test was just karma_web_test without the config_file attribute.
PR Close#33802
Prior to this change, setting style prop value to undefined or empty string would not result in resetting prop value in case the style prop is defined using [style.prop.px] syntax. The problem is that the check for empty value (and thus reseting the value) considered successful only in case of `null` value. This commit updates the check to use `isStylingValueDefined` function that also checks for undefined and empty string.
PR Close#33780
Prior to this change, ComponentFactory.create function invocation in Ivy retained the content of the host element (in case host element reference or CSS seelctor is provided as an argument). This behavior is different in View Engine, where the content of the host element was cleared, except for the case when ShadowDom encapsulation is used (to make sure native slot projection works). This commit aligns Ivy and View Engine and makes sure the host element is cleared before component content insertion.
PR Close#33487
The `ngInjectableDef` property was renamed to `ɵprov`, but core must
still support both because there are published libraries that use the
older term.
We are only interested in such properties that are defined directly on
the type being injected, not on base classes. So there is a check that
the defintion is specifically for the given type.
Previously if you tried to inject a class that had `ngInjectableDef` but
also inherited `ɵprov` then the check would fail on the `ɵprov` property
and never even try the `ngInjectableDef` property resulting in a failed
injection.
This commit fixes this by attempting to find each of the properties
independently.
Fixes https://github.com/angular/ngcc-validation/pull/526
PR Close#33732
Chains multiple listener instructions on a particular element into a single call which results in less generated code. Also handles listeners on templates, host listeners and synthetic host listeners.
PR Close#33720
Most of the use of `document` in the framework is within
the DI so they just inject the `DOCUMENT` token and are done.
Ivy is special because it does not rely upon the DI and must
get hold of the document some other way. There are a limited
number of places relevant to ivy that currently consume a global
document object.
The solution is modelled on the `LOCALE_ID` approach, which has
`getLocaleId()` and `setLocaleId()` top-level functions for ivy (see
`core/src/render3/i18n.ts`). In the rest of Angular (i.e. using DI) the
`LOCALE_ID` token has a provider that also calls setLocaleId() to
ensure that ivy has the same value.
This commit defines `getDocument()` and `setDocument() `top-level
functions for ivy. Wherever ivy needs the global `document`, it calls
`getDocument()` instead. Each of the platforms (e.g. Browser, Server,
WebWorker) have providers for `DOCUMENT`. In each of those providers
they also call `setDocument()` accordingly.
Fixes#33651
PR Close#33712
While overriding providers in Ivy TestBed (via TestBed.overrideProvider call), the old providers were retained in the list, since the override takes precedence. However, presence of providers in the list might have side-effect: if a provider has the `ngOnDestroy` lifecycle hook, this hook will be registered and invoked later (when component is destroyed). This commit updates TestBed logic to clear provider list by removing the ones which have overrides.
PR Close#33706
crisbeto
Pawel Kozlowski
Andrew Scott
Pete Bacon Darwin
Hayouung
Paul Gschwendtner
atscott
Greg Magolan
Andrew Kushnir
JoostK
Alex Rickabaugh
Danny Skoog
Miško Hevery
Christopher Dahm
Joey Perrott
Kara Erickson
Igor Minar
horn
mohax
JiaLiPassion