In #38762 we added a migration to replace the deprecated `preserveQueryParams`
option with `queryParamsHandling`, however due to a typo, we ended up replacing it
with `queryParamsHandler` which is invalid.
Fixes#39755.
PR Close#39763
The 15.x versions of `yargs` relied upon a version of `y18n` that
has a SNYK vulnerability.
This commit updates the overall project, and therefore also the
`localize` and `compiler-cli` packages to use the latest version
of `yargs` that does not depend upon the vulnerable `y18n`
version.
The AIO project was already on the latest `yargs` version and so
does not need upgrading.
Fixes#39743
PR Close#39749
Before this change, when trying to load a JSONP script that calls the JSONP callback inside a
microtask, it will fail in Internet Explorer 11 and EdgeHTML. This commit changes the onLoad cleanup
to be queued after the loaded endpoint executed any potential microtask itself. This ensures that
the aforementioned browsers will first evaluate the loaded script calling the JSONP callback and
only then run the cleanup inside onLoad.
Fixes#39496
PR Close#39512
The result of utf-8 encoding a string was represented in a string, where
each individual character represented a single byte according to its
character code. All usages of this data were interested in the byte
itself, so this required conversion from a character back to its code.
This commit simply stores the individual bytes in array to avoid the
conversion. This yields a ~10% performance improvement for i18n message
ID computation.
PR Close#39694
Message ID computation makes extensive use of big integer
multiplications in order to translate the message's fingerprint into
a numerical representation. In large compilations with heavy use of i18n
this was showing up high in profiler sessions.
There are two factors contributing to the bottleneck:
1. a suboptimal big integer representation using strings, which requires
repeated allocation and conversion from a character to numeric digits
and back.
2. repeated computation of the necessary base-256 exponents and their
multiplication factors.
The first bottleneck is addressed using a representation that uses an
array of individual digits. This avoids repeated conversion and
allocation overhead is also greatly reduced, as adding two big integers
can now be done in-place with virtually no memory allocations.
The second point is addressed by a memoized exponentiation pool to
optimize the multiplication of a base-256 exponent.
As an additional optimization are the two 32-bit words now converted to
decimal per word, instead of going through an intermediate byte buffer
and doing the decimal conversion per byte.
The results of these optimizations depend a lot on the number of i18n
messages for which a message should be computed. Benchmarks have shown
that computing message IDs is now ~6x faster for 1,000 messages, ~14x
faster for 10,000 messages, and ~24x faster for 100,000 messages.
PR Close#39694
`ViewRef` and `ApplicationRef` had a circular reference. This change
introduces `ViewRefTracker` which is a subset of `ApplicationRef` for
this purpose.
PR Close#39710
JIT needs to identify which type is `ChangeDetectorRef`. It was doing so
by importing `ChangeDetectorRef` and than comparing the types. This creates
circular dependency as well as prevents tree shaking. The new solution is
to brand the class with `__ChangeDetectorRef__` so that it can be identified
without creating circular dependency.
PR Close#39710
`LContainer` stores `ViewRef`s this is not quite right as it creates
circular dependency between the two types. Also `LContainer` should not
be aware of `ViewRef` which iv ViewEngine specific construct.
PR Close#39710
Due to historical reasons `Injector.__NG_ELEMENT_ID__` was set to `-1`.
This changes it to be consistent with other `*Ref.__NG_ELEMENT_ID__`
constructs.
PR Close#39710
`Renderer2` is declared in ViewEngine but it sub-classed in Ivy. This creates a circular
dependency between ViewEngine `Renderer2` which needs to declare `__NG_ELEMENT_ID__` and
ivy factory which needs to create it. The workaround used to be to pass the `Renderer2`
through stack but that created a very convoluted code. This refactoring simply bundles the
two files together and removes the stack workaround making the code simpler to follow.
PR Close#39710
`ChangeDetectorRef` is declared in ViewEngine but it sub-classed in Ivy. This creates a circular
dependency between ViewEngine `ChangeDetectorRef` which needs to declare `__NG_ELEMENT_ID__` and
ivy factory which needs to create it. The workaround used to be to pass the `ChangeDetectorRef`
through stack but that created a very convoluted code. This refactoring simply bundles the
two files together and removes the stack workaround making the code simpler to follow.
PR Close#39710
`ViewContainerRef` is declared in ViewEngine but it sub-classed in Ivy. This creates a circular
dependency between ViewEngine `ViewContainerRef` which needs to declare `__NG_ELEMENT_ID__` and
ivy factory which needs to create it. The workaround used to be to pass the `ViewContainerRef`
through stack but that created a very convoluted code. This refactoring simply bundles the
two files together and removes the stack workaround making the code simpler to follow.
PR Close#39710
`TemplateRef` is declared in ViewEngine but it sub-classed in Ivy. This creates a circular
dependency between ViewEngine `TemplateRef` which needs to declare `__NG_ELEMENT_ID__` and
ivy factory which needs to create it. The workaround used to be to pass the `TemplateRef`
through stack but that created a very convoluted code. This refactoring simply bundles the
two files together and removes the stack workaround making the code simpler to follow.
PR Close#39710
`ElementRef` is declared in ViewEngine but it sub-classed in Ivy. This creates a circular
dependency between ViewEngine `ElementRef` which needs to declare `__NG_ELEMENT_ID__` and
ivy factory which needs to create it. The workaround used to be to pass the `ElementRef`
through stack but that created a very convoluted code. This refactoring simply bundles the
two files together and removes the stack workaround making the code simpler to follow.
PR Close#39710
Currently when we encounter an implicit method call (e.g. `{{ foo(1) }}`) and we manage to resolve
its receiver to something within the template, we assume that the method is on the receiver itself
so we generate a type checking code to reflect it. This assumption is true in most cases, but it
breaks down if the call is on an implicit receiver and the receiver itself is being invoked. E.g.
```
<div *ngFor="let fn of functions">{{ fn(1) }}</div>
```
These changes resolve the issue by generating a regular function call if the method call's receiver
is pointing to `$implicit`.
Fixes#39634.
PR Close#39686
This commit removes the TODO comment that proposed
that we use the built-in RxJS `isObservable()` function.
This is not a viable approach since the built-in function
requires that the `obj` contains additional methods that
our "observable" types (such as `EventEmitter`) do not
necessarily have.
See #39643 for more information.
PR Close#39669
`setComponentScope` was previously undocumented. This commit adds a short
explanation of what the function does, and adds a link to a doc which
explains issues with cycles in more detail.
PR Close#39662
ngtsc will avoid emitting generated imports that would create an import
cycle in the user's program. The main way such imports can arise is when
a component would ordinarily reference its dependencies in its component
definition `directiveDefs` and `pipeDefs`. This requires adding imports,
which run the risk of creating a cycle.
When ngtsc detects that adding such an import would cause this to occur, it
instead falls back on a strategy called "remote scoping", where a side-
effectful call to `setComponentScope` in the component's NgModule file is
used to patch `directiveDefs` and `pipeDefs` onto the component. Since the
NgModule file already imports all of the component's dependencies (to
declare them in the NgModule), this approach does not risk adding a cycle.
It has several large downsides, however:
1. it breaks under `sideEffects: false` logic in bundlers including the CLI
2. it breaks tree-shaking for the given component and its dependencies
See this doc for further details: https://hackmd.io/Odw80D0pR6yfsOjg_7XCJg?view
In particular, the impact on tree-shaking was exacerbated by the naive logic
ngtsc used to employ here. When this feature was implemented, at the time of
generating the side-effectful `setComponentScope` call, the compiler did not
know which of the component's declared dependencies were actually used in
its template. This meant that unlike the generation of `directiveDefs` in
the component definition itself, `setComponentScope` calls had to list the
_entire_ compilation scope of the component's NgModule, including directives
and pipes which were not actually used in the template. This made the tree-
shaking impact much worse, since if the component's NgModule made use of any
shared NgModules (e.g. `CommonModule`), every declaration therein would
become un-treeshakable.
Today, ngtsc does have the information on which directives/pipes are
actually used in the template, but this was not being used during the remote
scoping operation. This commit modifies remote scoping to take advantage of
the extra context and only list used dependencies in `setComponentScope`
calls, which should ameliorate the tree-shaking impact somewhat.
PR Close#39662
The resource loader uses TypeScript's module resolution system to
determine at which locations it needs to look for a resource file. A
marker string is used to force the module resolution to fail, such that
all failed lookup locations can then be considered for actual resource
resolution. Any filesystem requests targeting files/directories that
contain the marker are known not to exist, so no filesystem request
needs to be done at all.
PR Close#39604
The type alias allows for this pattern to be more easily used in other
areas of the compiler code. The current usages of this pattern have been
updated to use the type alias.
PR Close#39604
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
Currently when an instance of the `FormControlName` directive is destroyed, the Forms package invokes
the `cleanUpControl` to clear all directive-specific logic (such as validators, onChange handlers,
etc) from a bound control. The logic of the `cleanUpControl` function should revert all setup
performed by the `setUpControl` function. However the `cleanUpControl` is too aggressive and removes
all callbacks related to the onChange and disabled state handling. This is causing problems when
a form control is bound to multiple FormControlName` directives, causing other instances of that
directive to stop working correctly when the first one is destroyed.
This commit updates the cleanup logic to only remove callbacks added while setting up a control
for a given directive instance.
The fix is needed to allow adding `cleanUpControl` function to other places where cleanup is needed
(missing this function calls in some other places causes memory leak issues).
PR Close#39623
* 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
TCB generation occasionally transforms binding expressions twice, which can
result in a `BindingPipe` operation being `resolve()`'d multiple times. When
the pipe does not exist, this caused multiple OOB diagnostics to be recorded
about the missing pipe.
This commit fixes the problem by making the OOB recorder track which pipe
expressions have had diagnostics produced already, and only producing them
once per expression.
PR Close#39517
`ComponentNgElementStrategy` is supposed to call `ngOnChanges()` on the
underlying component instance if available, but not fail if the
component does not have an `ngOnChanges()` method. This works as
expected. However, the test used to verify that was invalid; i.e. the
test would pass even if `ComponentNgElementStrategy` would try to call
`ngOnChanges()` on a component without such a method.
This commit replaces the invalid test with a new one that correctly
verifies that `ComponentNgElementStrategy` does not try to call
`ngOnChanges()`.
PR Close#39452
Previously, the `componentRef` property of `FakeComponentFactory` used
in `elements` tests was initialy set to a spy object with all mock
properties defined as spied methods. Later, the properties where
overwritten to the actual mock values.
This commit simplifies the creation of `componentRef` by correctly using
the arguments of [jasmine.createSpyObj()][1] to specify the desired
shape of the spy object (separating spied properties from methods and
directly providing the mock values).
[1]: https://jasmine.github.io/api/3.5/jasmine.html#.createSpyObj
PR Close#39452
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
There is a compiler transform that downlevels Angular class decorators
to static properties so that metadata is available for JIT compilation.
The transform was supposed to ignore non-Angular decorators but it was
actually completely dropping decorators that did not conform to a very
specific syntactic shape (i.e. the decorator was a simple identifier, or
a namespaced identifier).
This commit ensures that all non-Angular decorators are kepts as-is
even if they are built using a syntax that the Angular compiler does not
understand.
Fixes#39574
PR Close#39577
Tokenized text node may have leading whitespace skipped from their
source-span. But the source-span is used to compute where there are
interpolated blocks, resulting in placeholder nodes whose source-spans
are offset by the amount of skipped characters.
This fix uses the `fullStart` location of text source-spans for computing
the source-span of placeholders, so that they are accurate.
Fixes#39195
PR Close#39486
This commit ensures that when leading whitespace is skipped by
the tokenizer, the original start location (before skipping) is captured
in the `fullStart` property of the token's source-span.
PR Close#39486
The lexer is able to skip leading trivia in the `start` location of tokens.
This makes the source-span more friendly since things like elements
appear to begin at the start of the opening tag, rather than at the
start of any leading whitespace, which could include newlines.
But some tooling requires the full source-span to be available, such
as when tokenizing a text span into an Angular expression.
This commit simply adds the `fullStart` location to the `ParseSourceSpan`
class, and ensures that places where such spans are cloned, this
property flows through too.
PR Close#39486
In an i18n message, two placeholders next to each other must have
an "empty" message-part to separate them. Previously, the source-span
for this message-part was pointing to the wrong original location.
This caused problems in the generated source-maps and lead to extracted
i18n messages from being rendered incorrectly.
PR Close#39486
Prior to this commit, the `cleanUpControl` function (responsible for cleaning up control instance)
was not taking validators into account. As a result, these validators remain registered on a detached
form control instance, thus causing memory leaks. This commit updates the `cleanUpControl` function
logic to also run validators cleanup.
As a part of this change, the logic to setup and cleanup validators was refactored and moved to
separate functions (with completely opposite behavior), so that they can be reused in the future.
This commit doesn't add the `cleanUpControl` calls to all possible places, it just fixes the cases
where this function is being called, but doesn't fully perform a cleanup. The `cleanUpControl`
function calls will be added to other parts of code (to avoid more memory leaks) in a followup PR.
PR Close#39234
When a class with a custom decorator is transpiled to ES5, it looks something like this:
```
var SomeClass = (function() {
function SomeClass() {...};
var SomeClass_1 = __decorate([Decorator()], SomeClass);
SomeClass = SomeClass_1;
return SomeClass;
})();
```
The problem is that if the class also has an Angular decorator that refers to the class itself
(e.g. `{provide: someToken, useClass: SomeClass}`), the generated `setClassMetadata` code will
be emitted after the IIFE, but will still refer to the intermediate `SomeClass_1` variable from
inside the IIFE. This happens, because we generate the `setClassMetadata` call directly from
the source AST which contains identifiers that TS will rename when it emits the ES5 code.
These changes resolve the issue by looking through the metadata AST and cloning any `Identifier`
that is referring to the class. Since TS doesn't have references to the clone, it won't rename
it when transpiling to ES5.
Fixes#39509.
PR Close#39527
When registering an NgModule based on its id, all transitively imported
NgModules are also registered. This commit introduces a visited set to
avoid traversing into NgModules that are reachable from multiple import
paths multiple times.
Fixes#39487
PR Close#39514
The variable declaration for a template context is only needed when it
is referenced from somewhere, so the TCB operation to generate the
declaration is marked as optional.
PR Close#39321
When there is a primary outlet present in the outlets map and the object is also prefixed
with some other commands, the current logic only uses the primary outlet and ignores
the others. This change ensures that all outlets are respected at the
segment level when prefixed with other commands.
PR Close#39456
This commit has a small refactor of some methods in create_url_tree.ts
and adds some test cases, including two that will fail at the moment but
should pass. A follow-up commit will make use of the refactorings to fix
the test with minimal changes.
PR Close#39456
Kristiyan Kostadinov