This commit fixes the issue of the ASSERTION ERROR issue when
a projected node(RNode) inside an array is checked against the types
of TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer,
TNodeType.IcuContainer, TNodeType.Projection. As it's inside an array,
it doesn't fall into any of those types, as a result, it throws
the ASSERTION ERROR.
PR Close#37120
PR Close#37167
The codebase currently contains several `EMPTY_ARRAY` constants,
and they can end up in the bundle of an application.
A recent commit 6fbe219 tipped us off
as it introduced several `noop` occurrences in the golden symbol files.
After investigating with @petebacondarwin,
we decided to remove the duplicated symbols.
This probably shaves only a few bytes,
but this commit removes the duplicated functions,
by always using the one in `core/src/utils/empty`.
PR Close#40587
This commit updates the logic that calculates `useFactory` function arguments to avoid relying on `instanceof`
checks (thus always retaining symbols) and relies on flags that DI decorators contain (as a monkey-patched property).
Another perf benefit is having less megamorphic reads while calculating args for the `useFactory` call: we used to
check whether a token has `ngMetadataName` property 4 times (in worst case), now we have just 1 megamorphic read in
all cases.
Closes#40143.
PR Close#40145
Prior to this commit, removing `FormControlDirective` and `FormGroupName` directive instances didn't clear
the callbacks previously registered on FromControl/FormGroup class instances. As a result, these callbacks
were executed even after `FormControlDirective` and `FormGroupName` directive instances were destroyed. That was
also causing memory leaks since these callbacks also retained references to DOM elements.
This commit updates the cleanup logic to take care of properly detaching FormControl/FormGroup/FormArray instances
from the view by removing view-specific callback at destroy time.
Closes#20007, #37431, #39590.
PR Close#39235
DI providers can be defined via `useFactory` function, which may have arguments configured via `deps` array.
The `deps` array may contain DI flags represented by DI decorators (such as `@Self`, `@SkipSelf`, etc). Prior to this
commit, having the `@Host` decorator in `deps` array resulted in runtime error in Ivy. The problem was that the `@Host`
decorator was not taken into account while `useFactory` argument list was constructed, the `@Host` decorator was
treated as a token that should be looked up.
This commit updates the logic which prepares `useFactory` arguments to recognize the `@Host` decorator.
PR Close#40122
PR #39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.
Fix#40105
PR Close#40120
Differs tries to inject parent differ in order to support extending.
This does not work in the 'root' injector as the provider overrides the
default injector. The fix is to just assume standard set of providers
and extend those instead.
PR close#25015
Issue close#11309 `Can't extend IterableDiffers`
Issue close#18554 `IterableDiffers.extend is not AOT compatible`
(This is fixed because we no longer have an arrow function in the
factory but a proper function which can be imported.)
PR Close#39981
The codebase currently contains several `noop` functions,
and they can end up in the bundle of an application.
A recent commit 6fbe21941d tipped us off
as it introduced several `noop` occurrences in the golden symbol files.
After investigating with @petebacondarwin,
we decided to remove the duplicated functions.
This probably shaves only a few bytes,
but this commit removes the duplicated functions,
by always using the one in `core/src/utils/noop`.
PR Close#39761
`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#39621
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#39621
`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#39621
`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#39621
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
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
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
adds RuntimeError and code enum to improve debugging experience
refactor ExpressionChangedAfterItHasBeenCheckedError to code NG0100
refactor CyclicDependency to code NG0200
refactor No Provider to code NG0201
refactor MultipleComponentsMatch to code NG0300
refactor ExportNotFound to code NG0301
refactor PipeNotFound to code NG0302
refactor BindingNotKnown to code NG0303
refactor NotKnownElement to code NG0304
PR Close#39188
group together similar error messages as part of error code efforts
ProviderNotFound & NodeInjector grouped into throwProviderNotFoundError
Cyclic dependency errors grouped into throwCyclicDependencyError
PR Close#39251
The `ExpandoInstructions` was unnecessarily convoluted way to solve the
problem of calling the `HostBindingFunction`s on components and
directives. The code was complicated and hard to fallow.
The replacement is a simplified way to achieve the same thing, which
is also more efficient in space and speed.
PR Close#39301
Before this refactoring/fix the ICU would store the current selected
index in `TView`. This is incorrect, since if ICU is in `ngFor` it will
cause issues in some circumstances. This refactoring properly moves the
state to `LView`.
closes#37021closes#38144closes#38073
PR Close#39233
getCheckNoChangesMode was discovered to be unclear as to the purpose of
it. This refactor is a simple renaming to make it much clearer what that
method and property does.
PR Close#39277
This commit removes a workaround previously used for IE 9 and 10 to identify whether InjectableDef
was defined on a given class instance. Since support for IE 9 and 10 is removed, this fallback is
no longer needed.
PR Close#39090
`TNodeType.View` was created to support inline views. That feature did
not materialize and we have since removed the instructions for it, leave
an unneeded `TNodeType.View` which was still used in a very
inconsistent way. This change no longer created `TNodeType.View` (and
there will be a follow up chang to completely remove it.)
Also simplified the mental model so that `LView[HOST]`/`LView[T_HOST]`
always point to the insertion location of the `LView`.
PR Close#38707
`previousOrParentTNode` stores current `TNode`. Due to inconsistent
implementation the value stored would sometimes belong to the current
`TView` and sometimes to the parent. We have extra logic which accounts
for it. A better solution is to just ensure that `previousOrParentTNode`
always belongs to current `TNode`. This simplifies the mental model
and cleans up some code.
PR Close#38707
This commit adds `ngDevMode` guard to throw some errors only in dev mode
(similar to how things work in other parts of Ivy runtime code). The
`ngDevMode` flag helps to tree-shake these error messages from production
builds (in dev mode everything will work as it works right now) to decrease
production bundle size.
PR Close#38612
This commit adds a guard before throwing any forms errors. This will tree-shake
error messages which cannot be minified. It should also help to reduce the
bundle size of the `forms` package in production by ~20%.
Closes#37697
PR Close#37821
This commit performs minor refactoring in Forms package to get rid of duplicate functions.
It looks like the functions were duplicated due to a slightly different type signatures, but
their logic is completely identical. The logic in retained functions remains the same and now
these function also accept a generic type to achieve the same level of type safety.
PR Close#38371
This commit creates a sample forms test application to introduce the symbol
tests. It serves as a guard to ensure that any future work on the
forms package does not unintentionally increase the payload size.
PR Close#38044
stsogoo
Misko Hevery
cexbrayat
Andrew Kushnir
Egor
Kristiyan Kostadinov
Jessica Janiuk
twerske
Sonu Kapoor