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
`ViewRef` and `ApplicationRef` had a circular reference. This change
introduces `ViewRefTracker` which is a subset of `ApplicationRef` for
this purpose.
PR Close#39621
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#39621
`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
`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#39621
`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#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
`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#39621
Close#39348
Now `NgZone` has an option `shouldCoalesceEventChangeDetection` to coalesce
multiple event handler's change detections to one async change detection.
And there are some cases other than `event handler` have the same issues.
In #39348, the case like this.
```
// This code results in one change detection occurring per
// ngZone.run() call. This is entirely feasible, and can be a serious
// performance issue.
for (let i = 0; i < 100; i++) {
this.ngZone.run(() => {
// do something
});
}
```
So such kind of case will trigger multiple change detections.
And now with Ivy, we have a new `markDirty()` API will schedule
a requestAnimationFrame to trigger change detection and also coalesce
the change detections in the same event loop, `markDirty()` API doesn't
only take care `event handler` but also all other cases `sync/macroTask/..`
So this PR add a new option to coalesce change detections for all cases.
test(core): add test case for shouldCoalesceEventChangeDetection option
Add new test cases for current `shouldCoalesceEventChangeDetection` in `ng_zone.spec`, since
currently we only have integration test for this one.
PR Close#39422
`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
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
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
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
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
For consistency with other generated code, the partial declaration
functions are renamed to use the `ɵɵ` prefix which indicates that it is
generated API.
This commit also removes the declaration from the public API golden
file, as it's not yet considered stable at this point. Once the linker
is finalized will these declaration function be included into the golden
file.
PR Close#39518
This commit implements partial code generation for directives, which
will be transformed by the linker plugin to fully AOT compiled code in
follow-up work.
PR Close#39518
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
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
Currently expressions `$event.foo()` and `this.$event.foo()`, as well as `$any(foo)` and
`this.$any(foo)`, are treated as the same expression by the compiler, because `this` is considered
the same implicit receiver as when the receiver is omitted. This introduces the following issues:
1. Any time something called `$any` is used, it'll be stripped away, leaving only the first parameter.
2. If something called `$event` is used anywhere in a template, it'll be preserved as `$event`,
rather than being rewritten to `ctx.$event`, causing the value to undefined at runtime. This
applies to listener, property and text bindings.
These changes resolve the first issue and part of the second one by preserving anything that
is accessed through `this`, even if it's one of the "special" ones like `$any` or `$event`.
Furthermore, these changes only expose the `$event` global variable inside event listeners,
whereas previously it was available everywhere.
Fixes#30278.
PR Close#39323
This commit updates the docs for the `tView.preOrderHooks` and `tView.preOrderCheckHooks` TView
fields. Current docs are not up-to-date as it was pointed out in #39439.
Closes#39439.
PR Close#39497
Close#39296
Fix an issue that `markDirty()` will not trigger change detection.
The case is for example we have the following component.
```
export class AppComponent implements OnInit {
constructor(private router: Router) {}
ngOnInit() {
this.router.events
.pipe(filter((e) => e instanceof NavigationEnd))
.subscribe(() => ɵmarkDirty(this));
}
}
export class CounterComponent implements OnInit, OnDestroy {
ngOnInit() {
this.countSubject.pipe(takeUntil(this.destroy)).subscribe((count) => {
this.count = count;
ɵmarkDirty(this);
});
}
```
Then the app navigate from `AppComponent` to `CounterComponent`,
so there are 2 `markDirty()` call at in a row.
The `1st` call is from `AppComponent` when router changed, the
`2nd` call is from `CounterComponent.ngOnInit()`.
And the `markDirty()->scheduleTick()` code look like this
```
function scheduleTick(rootContext, flags) {
const nothingScheduled = rootContext.flags === 0 /* Empty */;
rootContext.flags |= flags;
if (nothingScheduled && rootContext.clean == _CLEAN_PROMISE) {
rootContext.schedule(() => {
...
if (rootContext.flags & RootContextFlags.DetectChanges)
rootContext.flags &= ~RootContextFlags.DetectChanges;
tickContext();
rootContext.clean = _CLEAN_PROMISE;
...
});
```
So in this case, the `1st` markDirty() will
1. set rootContext.flags = 1
2. before `tickContext()`, reset rootContext.flags = 0
3. inside `tickContext()`, it will call `CounterComponent.ngOnint()`,
so the `2nd` markDirty() is called.
4. and the `2nd` scheduleTick is called, `nothingScheduled` is true,
but rootContext.clean is not `_CLEAN_PROMISE` yet, since the `1st` markDirty tick
is still running.
5. So nowhere will reset the `rootContext.flags`.
6. then in the future, any other `markDirty()` call will not trigger the tick, since
`nothingScheduled` is always false.
So `nothingScheduled` means no tick is scheduled, `rootContext.clean === _CLEAN_PROMISE`
means no tick is running.
So we should set the flags to `rootContext` only when `no tick is scheudled or running`.
PR Close#39316
Angular-internal type definitions for Trusted Types were added in #39211.
When compiled using the Closure compiler with certain optimization
flags, identifiers from these type definitions (such as createPolicy)
are currently uglified and renamed to shorter strings. This causes
Angular applications compiled in this way to fail to create a Trusted
Types policy, and fall bock to using strings.
To fix this, mark the internal Trusted Types definitions as declarations
using the "declare" keyword. Also convert types to interfaces, for
the reasons explained in https://ncjamieson.com/prefer-interfaces/
PR Close#39471
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
Currently `i18n` attributes are treated the same no matter if they have data bindings or not. This
both generates more code since they have to go through the `ɵɵi18nAttributes` instruction and
prevents the translated attributes from being injected using the `@Attribute` decorator.
These changes makes it so that static translated attributes are treated in the same way as regular
static attributes and all other `i18n` attributes go through the old code path.
Fixes#38231.
PR Close#39408
This commit removes a workaround to calculate the `expandoStartIndex` value. That workaround was needed
because the `expandoStartIndex` was updated previously, so it pointed at the wrong location. The problem
was fixed in PR #39301 and the workaround is no longer needed.
PR Close#39416
In production mode, the `ngDevMode` global may not have been declared.
This is typically not a problem, as optimizers should have removed all
usages of the `ngDevMode` variables. This does however require the
bundler/optimizer to have been configured in a certain way, as to allow
for `ngDevMode` guarded code to be removed.
As an example, Terser can be configured to remove the `ngDevMode`
guarded code using the following configuration:
```js
const terserOptions = {
// ...
compress: {
// ...
global_defs: require('@angular/compiler-cli').GLOBAL_DEFS_FOR_TERSER,
}
}
```
(Taken from https://github.com/angular/angular/issues/31595#issuecomment-519129090)
If this is not done, however, the bundle should still work (albeit with
larger code size due to missed tree-shaking opportunities). This commit
adds a check for whether `ngDevMode` has been declared, as it is a
top-level statement that executes before `ngDevMode` has been initialized.
Fixes#31595
PR Close#39415
Runtime i18n logic doesn't distinguish `<ng-content>` tag placeholders and regular element tag
placeholders in i18n messages, so there is no need to have a special marker for projection-based
placeholders and element markers can be used instead.
PR Close#39172
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
`TNode.insertBeforeIndex` is only populated when i18n is present. This
change puts all code which reads `insertBeforeIndex` behind a
dynamically loaded functions which are set only when i18n code executes.
PR Close#39301
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
`expandoInstructions` uses negative numbers by `-x`. This has lead to
issues in the paste as `-0` is processed as float rather than integer
leading to de-optimization.
PR Close#39233
IMPORTANT: `HEADER_OFFSET` should only be refereed to the in the `ɵɵ*` instructions to translate
instruction index into `LView` index. All other indexes should be in the `LView` index space and
there should be no need to refer to `HEADER_OFFSET` anywhere else.
PR Close#39233
- Made `*OpCodes` array branded for safer type checking.
- Simplify `I18NRemoveOpCodes` encoding.
- Broke out `IcuCreateOpCodes` from `I18nMutableOpCodes`.
PR Close#39233
`COMMENT_MARKER` is a generic name which does not make it obvious that
it is used for ICU use case. `ICU_MARKER` is more explicit as it is used
exclusively with ICUs.
PR Close#39233
Misko Hevery
Egor
ngdevelop-tech
Pete Bacon Darwin
JiaLiPassion
Alex Rickabaugh
Kristiyan Kostadinov
Andrew Kushnir
Jessica Janiuk
JoostK
Andrew Scott
Igor Minar
Bjarki
Alan Agius
twerske
AleksanderBodurri