Escape the content of the strings so that it can be safely inserted into a comment node.
The issue is that HTML does not specify any way to escape comment end text inside the comment.
`<!-- The way you close a comment is with "-->". -->`. Above the `"-->"` is meant to be text
not an end to the comment. This can be created programmatically through DOM APIs.
```
div.innerHTML = div.innerHTML
```
One would expect that the above code would be safe to do, but it turns out that because comment
text is not escaped, the comment may contain text which will prematurely close the comment
opening up the application for XSS attack. (In SSR we programmatically create comment nodes which
may contain such text and expect them to be safe.)
This function escapes the comment text by looking for the closing char sequence `-->` and replace
it with `-_-_>` where the `_` is a zero width space `\u200B`. The result is that if a comment
contains `-->` text it will render normally but it will not cause the HTML parser to close the
comment.
PR Close#40152
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#39589
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
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
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
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
When reading globals such as `ngDevMode` the read should be guarded by `typeof ngDevMode` otherwise it will throw if not
defined in `"use strict"` mode.
PR Close#36055
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 updates micro benchmarks to use relative path to Ivy runtime code. Keeping absolute
locations caused issues with build optimizer that retained certain symbols and they appeared in the
output twice.
PR Close#39142
This commit adds micro benchmarks to run micro benchmarks for i18n-related logic in the
following scenarios:
- i18n static attributes
- i18n attributes with interpolations
- i18n blocks of static text
- i18n blocks of text + interpolations
- simple ICUs
- nested ICUs
First 4 scenarios also have baseline scenarios (non-i18n) so that we can compare i18n perf with
non-i18n logic.
PR Close#39142
Updates to rules_nodejs 2.2.0. This is the first major release in 7 months and includes a number of features as well
as breaking changes.
Release notes: https://github.com/bazelbuild/rules_nodejs/releases/tag/2.0.0
Features of note for angular/angular:
* stdout/stderr/exit code capture; this could be potentially be useful
* TypeScript (ts_project); a simpler tsc rule that ts_library that can be used in the repo where ts_library is too
heavy weight
Breaking changes of note for angular/angular:
* loading custom rules from npm packages: `ts_library` is no longer loaded from `@npm_bazel_typescript//:index.bzl`
(which no longer exists) but is now loaded from `@npm//@bazel/typescript:index.bzl`
* with the loading changes above, `load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")` is
no longer needed in the WORKSPACE which also means that yarn_install does not need to run unless building/testing
a target that depends on @npm. In angular/angular this is a minor improvement as almost everything depends on @npm.
* @angular/bazel package is also updated in this PR to support the new load location; Angular + Bazel users that
require it for ng_package (ng_module is no longer needed in OSS with Angular 10) will need to load from
`@npm//@angular/bazel:index.bzl`. I investigated if it was possible to maintain backward compatability for the old
load location `@npm_angular_bazel` but it is not since the package itself needs to be updated to load from
`@npm//@bazel/typescript:index.bzl` instead of `@npm_bazel_typescript//:index.bzl` as it depends on ts_library
internals for ng_module.
* runfiles.resolve will now throw instead of returning undefined to match behavior of node require
Other changes in angular/angular:
* integration/bazel has been updated to use both ng_module and ts_libary with use_angular_plugin=true.
The latter is the recommended way for rules_nodejs users to compile Angular 10 with Ivy. Bazel + Angular ViewEngine is
supported with @angular/bazel <= 9.0.5 and Angular <= 8. There is still Angular ViewEngine example on rules_nodejs
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_view_engine on these older versions but users
that want to update to Angular 10 and are on Bazel must switch to Ivy and at that point ts_library with
use_angular_plugin=true is more performant that ng_module. Angular example in rules_nodejs is configured this way
as well: https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular. As an aside, we also have an
example of building Angular 10 with architect() rule directly instead of using ts_library with angular plugin:
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_bazel_architect.
NB: ng_module is still required for angular/angular repository as it still builds ViewEngine & @angular/bazel
also provides the ng_package rule. ng_module can be removed in the future if ViewEngine is no longer needed in
angular repo.
* JSModuleInfo provider added to ng_module. this is for forward compat for future rules_nodejs versions.
PR Close#39182
Updates to rules_nodejs 2.2.0. This is the first major release in 7 months and includes a number of features as well
as breaking changes.
Release notes: https://github.com/bazelbuild/rules_nodejs/releases/tag/2.0.0
Features of note for angular/angular:
* stdout/stderr/exit code capture; this could be potentially be useful
* TypeScript (ts_project); a simpler tsc rule that ts_library that can be used in the repo where ts_library is too
heavy weight
Breaking changes of note for angular/angular:
* loading custom rules from npm packages: `ts_library` is no longer loaded from `@npm_bazel_typescript//:index.bzl`
(which no longer exists) but is now loaded from `@npm//@bazel/typescript:index.bzl`
* with the loading changes above, `load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")` is
no longer needed in the WORKSPACE which also means that yarn_install does not need to run unless building/testing
a target that depends on @npm. In angular/angular this is a minor improvement as almost everything depends on @npm.
* @angular/bazel package is also updated in this PR to support the new load location; Angular + Bazel users that
require it for ng_package (ng_module is no longer needed in OSS with Angular 10) will need to load from
`@npm//@angular/bazel:index.bzl`. I investigated if it was possible to maintain backward compatability for the old
load location `@npm_angular_bazel` but it is not since the package itself needs to be updated to load from
`@npm//@bazel/typescript:index.bzl` instead of `@npm_bazel_typescript//:index.bzl` as it depends on ts_library
internals for ng_module.
* runfiles.resolve will now throw instead of returning undefined to match behavior of node require
Other changes in angular/angular:
* integration/bazel has been updated to use both ng_module and ts_libary with use_angular_plugin=true.
The latter is the recommended way for rules_nodejs users to compile Angular 10 with Ivy. Bazel + Angular ViewEngine is
supported with @angular/bazel <= 9.0.5 and Angular <= 8. There is still Angular ViewEngine example on rules_nodejs
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_view_engine on these older versions but users
that want to update to Angular 10 and are on Bazel must switch to Ivy and at that point ts_library with
use_angular_plugin=true is more performant that ng_module. Angular example in rules_nodejs is configured this way
as well: https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular. As an aside, we also have an
example of building Angular 10 with architect() rule directly instead of using ts_library with angular plugin:
https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_bazel_architect.
NB: ng_module is still required for angular/angular repository as it still builds ViewEngine & @angular/bazel
also provides the ng_package rule. ng_module can be removed in the future if ViewEngine is no longer needed in
angular repo.
* JSModuleInfo provider added to ng_module. this is for forward compat for future rules_nodejs versions.
@josephperrott, this touches `packages/bazel/src/external.bzl` which will make the sync to g3 non-trivial.
PR Close#37727
`NodeInjector` is store in expando as a list of values in an array. The
offset constant into the array have been brought together into a single
`NodeInjectorOffset` enum with better documentation explaining their usage.
PR Close#38707
This change makes `getPreviousOrParentTNode` return `TNode|null` (rather
than just `TNode`) which is more reflective of the reality. The
`getPreviousOrParentTNode` can be `null` upon entering the `LView`.
PR Close#38707
`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
Host `TNode` was passed into `getOrCreateTNode` just so that we can
compute weather or not we are a root node. This was needed because
`previousOrParentTNode` could have `TNode` from `TView` other then
current `TView`. This is confusing mental model. Previous change
ensured that `previousOrParentTNode` must always be part of `TView`,
which enabled this change to remove the unneeded argument.
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 updates several import statements in the core package to decrease the number of
cycles detected by the dependency checker tool.
PR Close#38805
This commit creates a sample router test application to introduce the
symbol tests. It serves as a guard to ensure that any future work on the
router package does not unintentionally increase the payload size.
PR Close#38714
The `RefreshTransplantedView` flag is used to indicate that the view or one of its children
is transplanted and dirty, so it should still be refreshed as part of change detection.
This flag is set on the transplanted view itself as well setting a
counter on as its parents.
When a transplanted view is detached and still has this flag, it means
it got detached before it was refreshed. This can happen for "backwards
references" or transplanted views that are inserted at a location that
was already checked. In this case, we should decrement the parent
counters _and_ clear the flag on the detached view so it's not seen as
"transplanted" anymore (it is detached and has no parent counters to
adjust).
fixes#38619
PR Close#38768
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
`tView` that is stored on a component def contains information about directives and pipes
that are available in the scope of this component. Patching component scope causes `tView` to be
updated. Prior to this commit, the `tView` information was not restored/reset in case component
class is not declared in the `declarations` field while calling `TestBed.configureTestingModule`,
thus causing `tView` to be reused between tests (thus preserving scopes information between tests).
This commit updates TestBed logic to preserve `tView` value before applying scope changes and
reset it back to the previous state between tests.
Closes#38600.
PR Close#38659
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
Fix a bug in the HTML sanitizer where an unclosed iframe tag would
result in an escaped closing body tag as the output:
_sanitizeHtml(document, '<iframe>') => '</body>'
This closing body tag comes from the DOMParserHelper where the HTML to be
sanitized is wrapped with surrounding body tags. When an opening iframe
tag is parsed by DOMParser, which DOMParserHelper uses, everything up
until its matching closing tag is consumed as a text node. In the above
example this includes the appended closing body tag.
By removing the explicit closing body tag from the DOMParserHelper and
relying on the body tag being closed implicitly at the end, the above
example is sanitized as expected:
_sanitizeHtml(document, '<iframe>') => ''
PR Close#38454
Similarly to the change we landed in the `@angular/core` reflection
capabilities, we need to make sure that ngcc can detect pass-through
delegate constructors for classes using downleveled ES2015 output.
More details can be found in the preceding commit, and in the issue
outlining the problem: #38453.
Fixes#38453.
PR Close#38463
In the Angular Package Format, we always shipped UMD bundles and previously even ES5 module output.
With V10, we removed the ES5 module output but kept the UMD ES5 output.
For this, we were able to remove our second TypeScript transpilation. Instead we started only
building ES2015 output and then downleveled it to ES5 UMD for the NPM packages. This worked
as expected but unveiled an issue in the `@angular/core` reflection capabilities.
In JIT mode, Angular determines constructor parameters (for DI) using the `ReflectionCapabilities`. The
reflection capabilities basically read runtime metadata of classes to determine the DI parameters. Such
metadata can be either stored in static class properties like `ctorParameters` or within TypeScript's `design:params`.
If Angular comes across a class that does not have any parameter metadata, it tries to detect if the
given class is actually delegating to an inherited class. It does this naively in JIT by checking if the
stringified class (function in ES5) matches a certain pattern. e.g.
```js
function MatTable() {
var _this = _super.apply(this, arguments) || this;
```
These patterns are reluctant to changes of the class output. If a class is not recognized properly, the
DI parameters will be assumed empty and the class is **incorrectly** constructed without arguments.
This actually happened as part of v10 now. Since we downlevel ES2015 to ES5 (instead of previously
compiling sources directly to ES5), the class output changed slightly so that Angular no longer detects
it. e.g.
```js
var _this = _super.apply(this, __spread(arguments)) || this;
```
This happens because the ES2015 output will receive an auto-generated constructor if the class
defines class properties. This constructor is then already containing an explicit `super` call.
```js
export class MatTable extends CdkTable {
constructor() {
super(...arguments);
this.disabled = true;
}
}
```
If we then downlevel this file to ES5 with `--downlevelIteration`, TypeScript adjusts the `super` call so that
the spread operator is no longer used (not supported in ES5). The resulting super call is different to the
super call that would have been emitted if we would directly transpile to ES5. Ultimately, Angular no
longer detects such classes as having an delegate constructor -> and DI breaks.
We fix this by expanding the rather naive RegExp patterns used for the reflection capabilities
so that downleveled pass-through/delegate constructors are properly detected. There is a risk
of a false-positive as we cannot detect whether `__spread` is actually the TypeScript spread
helper, but given the reflection patterns already make lots of assumptions (e.g. that `super` is
actually the superclass, we should be fine making this assumption too. The false-positive would
not result in a broken app, but rather in unnecessary providers being injected (as a noop).
Fixes#38453
PR Close#38463
This commit updates the code to move generated i18n statements into the `consts` field of
ComponentDef to avoid invoking `$localize` function before component initialization (to better
support runtime translations) and also avoid problems with lazy-loading when i18n defs may not
be present in a chunk where it's referenced.
Prior to this change the i18n statements were generated at the top leve:
```
var I18N_0;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
var MSG_X = goog.getMsg(“…”);
I18N_0 = MSG_X;
} else {
I18N_0 = $localize('...');
}
defineComponent({
// ...
template: function App_Template(rf, ctx) {
i0.ɵɵi18n(2, I18N_0);
}
});
```
This commit updates the logic to generate the following code instead:
```
defineComponent({
// ...
consts: function() {
var I18N_0;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
var MSG_X = goog.getMsg(“…”);
I18N_0 = MSG_X;
} else {
I18N_0 = $localize('...');
}
return [
I18N_0
];
},
template: function App_Template(rf, ctx) {
i0.ɵɵi18n(2, 0);
}
});
```
Also note that i18n template instructions now refer to the `consts` array using an index
(similar to other template instructions).
PR Close#38404
When removal of one view causes removal of another one from the same
ViewContainerRef it triggers an error with views length calculation. This commit
fixes this bug by removing a view from the list of available views before invoking
actual view removal (which might be recursive and relies on the length of the list
of available views).
Fixes#38201.
PR Close#38317
Fixes an error if a CSS custom property, used inside a host binding, has a
number in its name. The error is thrown because the styling parser only
expects characters from A to Z,dashes, underscores and a handful of other
characters.
Fixes#37292.
PR Close#38432
Misko Hevery