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
This commit contains no changes to code. It only breaks `i18n.ts` file
into `i18n.ts` + `i18n_apply.ts` + `i18n_parse.ts` +
`i18n_postprocess.ts` for easier maintenance.
PR Close#38368
The currently selected ICU was incorrectly being stored it `TNode`
rather than in `LView`.
Remove: `TIcuContainerNode.activeCaseIndex`
Add: `LView[TIcu.currentCaseIndex]`
PR Close#38345
This change provides better typing for the `LView.debug` property which
is intended to be used by humans while debugging the application with
`ngDevMode` turned on.
In addition this chang also adds jasmine matchers for better asserting
that `LView` is in the correct state.
PR Close#38359
I18n code breaks up internationalization into opCodes which are then stored
in arrays. To make it easier to debug the codebase this PR adds `debug`
property to the arrays which presents the data in human readable format.
PR Close#38154
Refactors the `ng_rollup_bundle` rule to a macro that relies on
the `@bazel/rollup` package. This means that the rule no longer
deals with custom ESM5 flavour output, but rather only builds
prodmode ES2015 output. This matches the common build output
in Angular projects, and optimizations done in CLI where
ES2015 is the default optimization input.
The motiviation for this change is:
* Not duplicating rollup Bazel rules. Instead leveraging the official
rollup rule.
* Not dealing with a third TS output flavor in Bazel.The ESM5 flavour has the
potential of slowing down local development (as it requires compilation replaying)
* Updating the rule to be aligned with current CLI optimizations.
This also _fixes_ a bug that surfaced in the old rollup bundle rule.
Code that is unused, is not removed properly. The new rule fixes this by
setting the `toplevel` flag. This instructs terser to remove unused
definitions at top-level. This matches the optimization applied in CLI
projects. Notably the CLI doesn't need this flag, as code is always
wrapped by Webpack. Hence, the unused code eliding runs by default.
PR Close#37623
If we detect that an injectable class is inheriting from another injectable, we generate code that looks something like this:
```
const baseFactory = ɵɵgetInheritedFactory(Child);
@Injectable()
class Parent {}
@Injectable()
class Child extends Parent {
static ɵfac = (t) => baseFactory(t || Child)
}
```
This usually works fine, because the `ɵɵgetInheritedFactory` resolves to the factory of `Parent`, but the logic can break down if the `Child` class has a custom decorator. Custom decorators can return a new class that extends the original once, which means that the `ɵɵgetInheritedFactory` call will now resolve to the factory of the `Child`, causing an infinite loop.
These changes fix the issue by changing the inherited factory resolution logic so that it walks up the prototype chain class-by-class, while skipping classes that have the same factory as the class that was passed in.
Fixes#35733.
PR Close#37022
Update docs in the micro benchmarks to include:
* How to run with no turbo inlining
* Where to find the profiles in the DevTools
* Best way to debug benchmarks (using the profile_in_browser rather than --inspect-brk)
PR Close#37140
Previously all static styling information (including the ones from component/directive host bindings) would get merged into a single value before it would be written into the `@Input('class'/'style')`. The new behavior specifically excludes host values from the `@Input` bindings.
Fix#35383
PR Close#35889
In 420b9be1c1 all style-based sanitization code was
disabled because modern browsers no longer allow for javascript expressions within
CSS. This patch is a follow-up patch which removes all traces of style sanitization
code (both instructions and runtime logic) for the `[style]` and `[style.prop]` bindings.
PR Close#36965
This patch is the first of many commits to disable sanitization for
[stlye.prop] and [style] bindings in Angular.
Historically, style-based sanitization has only been required for old
IE browsers (IE6 and IE7). Since Angular does not support these old
browsers at all, there is no reason for the framework to support
style-based sanitization.
PR Close#35621
1. update jasmine to 3.5
2. update @types/jasmine to 3.5
3. update @types/jasminewd2 to 2.0.8
Also fix several cases, the new jasmine 3 will help to create test cases correctly,
such as in the `jasmine 2.x` version, the following case will pass
```
expect(1 == 2);
```
But in jsamine 3, the case will need to be
```
expect(1 == 2).toBeTrue();
```
PR Close#34625
Switches our tslint setup to the standard `tslint.json` linter excludes.
The set of files that need to be linted is specified through a Yarn script.
For IDEs, open files are linted with the closest tslint configuration, if the
tslint IDE extension is set up, and the source file is not excluded.
We cannot use the language service plugin for tslint as we have multiple nested
tsconfig files, and we don't want to add the plugin to each tsconfig. We
could reduce that bloat by just extending from a top-level tsconfig that
defines the language service plugin, but unfortunately the tslint plugin does
not allow the use of tslint configs which are not part of the tsconfig project.
This is problematic since the tslint configuration is at the project root, and we
don't want to copy tslint configurations next to each tsconfig file.
Additionally, linting of `d.ts` files has been re-enabled. This has been
disabled in the past and a TODO has been left. This commit fixes the
lint issues and re-enables linting.
PR Close#35800
`ɵɵNgOnChangesFeature()` would set `ngInherit`, which is a side effect and also not necessary. This was pulled out to module scope so the function itself can be pure. Since it only curries another function, the call is entirely unnecessary. Updated the compiler to only generate a reference to this function, rather than a call to it, and removed the extra curry indirection.
PR Close#35769
This commit updates the host bindings micro benchmark to run tests with mutliple directives (where each directive contains host bindings). The number of directives is configurable as a constant in the micro benchmark file. This change is needed to have an ability to measure/compare perf in different scenarios.
PR Close#35736
This commit adds micro benchmark for host bindings, so that we can assess the impact of changes related to host bindings (for example PR #35568).
PR Close#35705
When a pipe inherits its constructor, and as a result its factory, from an injectable in AOT mode, it can end up throwing an error, because the inject implementation hasn't been set yet. These changes ensure that the implementation is set before the pipe's factory is invoked.
Note that this isn't a problem in JIT mode, because the factory inheritance works slightly differently, hence why this test isn't going through `TestBed`.
Fixes#35277.
PR Close#35468
In View Engine, host element of dynamically created component received attributes and classes extracted from component's selector. For example, if component selector is `[attr] .class`, the `attr` attribute and `.class` class will be add to host element. This commit adds similar logic to Ivy, to make sure this behavior is aligned with View Engine.
PR Close#34481
Before this change content queries with the `descendants: false` option, as implemented in ivy,
would not descendinto `<ng-container>` elements. This behaviour was different from the way the
View Engine worked. This change alligns ngIvy and VE behaviours when it comes to queries and the
`<ng-container>` elements and fixes a common bugs where a query target was placed inside the
`<ng-container>` element with a * directive on it.
Before:
```html
<needs-target>
<ng-container *ngIf="condition">
<div #target>...</div> <!-- this node would NOT match -->
</ng-container>
</needs-target>
```
After:
```html
<needs-target>
<ng-container *ngIf="condition">
<div #target>...</div> <!-- this node WILL match -->
</ng-container>
</needs-target>
```
Fixes#34768
PR Close#35384
Currently the logic that handles ICUs located outside of i18n blocks may throw exceptions at runtime. The problem is caused by the fact that we store incorrect TNode index for previous TNode (index that includes HEADER_OFFSET) and do not store a flag whether this TNode is a parent or a sibling node. As a result, the logic that assembles the final output uses incorrect TNodes and in some cases throws exceptions (when incompatible structure is extracted from tView.data due to the incorrect index). This commit adjusts the index and captures whether TNode is a parent to make sure underlying logic manipulates correct TNode.
PR Close#35347
- Adds `TView` into `LFrame`, read the `TView` from `LView` on `enterView`.
- Before this change the `TView` was ofter looked up from `LView` as `lView[TVIEW]`. This is suboptimal since reading from an Array, requires that the read checks array size before the read. This means that such a read has a much higher cost than reading from the property directly. By passing in the `TView` explicitly it makes the code more explicit and faster.
- Some rearrangements of arguments so that `TView` would come before `LView` for consistency.
PR Close#35069
`TNode.directives` was introduced in https://github.com/angular/angular/pull/34938. Turns out that it is unnecessary because the information is already present it `TData` when combining with `TNode.directiveStart` and `TNode.directiveEnd`
Mainly this is true (conceptually):
```
expect(tNode.directives).toEqual(
tData.slice(
tNode.directivesStart,
tNode.directivesEnd - tNode.DirectivesStart -1
)
);
```
The refactoring removes `TNode.directives` and adds `TNode.directiveStyling` as we still need to keep location in the directive in `TNode`
PR Close#35050
These tests are used for perf testing and don't run as part of CI, as a result they bit-rotted. This fixes that. Long term these tests should be run as part of CI.
PR Close#35071
This change changes the priority order of static styling.
Current priority:
```
(least priority)
- Static
- Component
- Directives
- Template
- Dynamic Binding
- Component
- Map/Interpolation
- Property
- Directives
- Map/Interpolation
- Property
- Template
- Map/Interpolation
- Property
(highest priority)
```
The issue with the above priority is this use case:
```
<div style="color: red;" directive-which-sets-color-blue>
```
In the above case the directive will win and the resulting color will be `blue`. However a small change of adding interpolation to the example like so. (Style interpolation is coming in https://github.com/angular/angular/pull/34202)
```
<div style="color: red; width: {{exp}}px" directive-which-sets-color-blue>
```
Changes the priority from static binding to interpolated binding which means now the resulting color is `red`. It is very surprising that adding an unrelated interpolation and style can change the `color` which was not changed. To fix that we need to make sure that the static values are associated with priority of the source (directive or template) where they were declared. The new resulting priority is:
```
(least priority)
- Component
- Static
- Map/Interpolation
- Property
- Directives
- Static
- Map/Interpolation
- Property
- Template
- Static
- Map/Interpolation
- Property
(highest priority)
```
PR Close#34938
We had some logic for generating and passing in the `elIndex` parameter into the `hostBindings` function, but it wasn't actually being used for anything. The only place left that had a reference to it was the `StylingBuilder` and it only stored it without referencing it again.
PR Close#34969
Previously we would write to class/style as strings `element.className` and `element.style.cssText`. Turns out that approach is good for initial render but not good for updates. Updates using this approach are problematic because we have to check to see if there was an out of bound write to style and than perform reconciliation. This also requires the browser to bring up CSS parser which is expensive.
Another problem with old approach is that we had to queue the DOM writes and flush them twice. Once on element advance instruction and once in `hostBindings`. The double flushing is expensive but it also means that a directive can observe that styles are not yet written (they are written after directive executes.)
The new approach uses `element.classList.add/remove` and `element.style.setProperty/removeProperty` API for updates only (it continues to use `element.className` and `element.style.cssText` for initial render as it is cheaper.) The other change is that the styling changes are applied immediately (no queueing). This means that it is the instruction which computes priority. In some circumstances it may result in intermediate writes which are than overwritten with new value. (This should be rare)
Overall this change deletes most of the previous code and replaces it with new simplified implement. The simplification results in code savings.
PR Close#34804
NOTE: This change must be reverted with previous deletes so that it code remains in build-able state.
This change deletes old styling code and replaces it with a simplified styling algorithm.
The mental model for the new algorithm is:
- Create a linked list of styling bindings in the order of priority. All styling bindings ere executed in compiled order and than a linked list of bindings is created in priority order.
- Flush the style bindings at the end of `advance()` instruction. This implies that there are two flush events. One at the end of template `advance` instruction in the template. Second one at the end of `hostBindings` `advance` instruction when processing host bindings (if any).
- Each binding instructions effectively updates the string to represent the string at that location. Because most of the bindings are additive, this is a cheap strategy in most cases. In rare cases the strategy requires removing tokens from the styling up to this point. (We expect that to be rare case)S Because, the bindings are presorted in the order of priority, it is safe to resume the processing of the concatenated string from the last change binding.
PR Close#34616
NOTE: This change deletes code and creates a BROKEN SHA. If reverting this SHA needs to be reverted with the next SHA to get back into a valid state.
PR Close#34616
The `computeStaticStyling` will be used for computing static styling value during `firstCreatePass`.
The function takes into account static styling from the template as well as from the host bindings. The host bindings need to be merged in front of the template so that they have the correct priority.
PR Closes#34418
Parsing styling is now simplified to be used like so:
```
for (let i = parseStyle(text); i <= 0; i = parseStyleNext(text, i)) {
const key = getLastParsedKey();
const value = getLastParsedValue();
...
}
```
This change makes it easier to invoke the parser from other locations in the system without paying the cost of creating and iterating over `Map` of styles.
PR Closes#34418
This change moves information from instructions to declarative position:
- `ɵɵallocHostVars(vars)` => `DirectiveDef.hostVars`
- `ɵɵelementHostAttrs(attrs)` => `DirectiveDef.hostAttrs`
When merging directives it is necessary to know about `hostVars` and `hostAttrs`. Before this change the information was stored in the `hostBindings` function. This was problematic, because in order to get to the information the `hostBindings` would have to be executed. In order for `hostBindings` to be executed the directives would have to be instantiated. This means that the directive instantiation would happen before we had knowledge about the `hostAttrs` and as a result the directive could observe in the constructor that not all of the `hostAttrs` have been applied. This further complicates the runtime as we have to apply `hostAttrs` in parts over many invocations.
`ɵɵallocHostVars` was unnecessarily complicated because it would have to update the `LView` (and Blueprint) while existing directives are already executing. By moving it out of `hostBindings` function we can access it statically and we can create correct `LView` (and Blueprint) in a single pass.
This change only changes how the instructions are generated, but does not change the runtime much. (We cheat by emulating the old behavior by calling `ɵɵallocHostVars` and `ɵɵelementHostAttrs`) Subsequent change will refactor the runtime to take advantage of the static information.
PR Close#34683
This adds `insertTStyleValue` but does not hook it up to anything yet.
The purpose of this function is to create a linked-list of styling
related bindings. The bindings can be traversed during flush.
The linked list also keeps track of duplicates. This is important
for binding to know if it needs to check other styles for reconciliation.
PR Close#34004
This change introduces class/style reconciliation algorithm for DOM elements.
NOTE: The code is not yet hooked up, it will be used by future style algorithm.
Background:
Styling algorithm currently has [two paths](https://hackmd.io/@5zDGNGArSxiHhgvxRGrg-g/rycZk3N5S)
when computing how the style should be rendered.
1. A direct path which concatenates styling and uses `elemnent.className`/`element.style.cssText` and
2. A merge path which uses internal data structures and uses `element.classList.add/remove`/`element.style[property]`.
The situation is confusing and hard to follow/maintain. So a future PR will remove the merge-path and do everything with
direct-path. This however breaks when some other code adds class or style to the element without Angular's knowledge.
If this happens instead of switching from direct-path to merge-path algorithm, this change provides a different mental model
whereby we always do `direct-path` but the code which writes to the DOM detects the situation and reconciles the out of bound write.
The reconciliation process is as follows:
1. Detect that no one has modified `className`/`cssText` and if so just write directly (fast path).
2. If out of bounds write did occur, switch from writing using `className`/`cssText` to `element.classList.add/remove`/`element.style[property]`.
This does require that the write function computes the difference between the previous Angular expected state and current Angular state.
(This requires a parser. The advantage of having a parser is that we can support `style="width: {{exp}}px" kind of bindings.`)
Compute the diff and apply it in non destructive way using `element.classList.add/remove`/`element.style[property]`
Properties of approach:
- If no out of bounds style modification:
- Very fast code path: Just concatenate string in right order and write them to DOM.
- Class list order is preserved
- If out of bounds style modification detected:
- Penalty for parsing
- Switch to non destructive modification: `element.classList.add/remove`/`element.style[property]`
- Switch to alphabetical way of setting classes.
PR Close#34004
This patch removes the need for the styleSanitizer() instruction in
favor of passing the sanitizer into directly into the styleProp
instruction.
This patch also increases the binding index size for all style/class bindings in preparation for #34418
PR Close#34480
Andrew Kushnir
Misko Hevery
Paul Gschwendtner
crisbeto
Joey Perrott
Andrew Scott
Matias Niemelä
JiaLiPassion
Doug Parker
Pawel Kozlowski
Greg Magolan