Previously localized strings were not mapped to their original
source location, so it was not possible to back-trace them
in tools like the i18n message extractor.
PR Close#32912
Currently Angular internally already handles `InjectionToken` as
predicates for queries. This commit exposes this as public API as
developers already relied on this functionality but currently use
workarounds to satisfy the type constraints (e.g. `as any`).
We intend to make this public as it's low-effort to support, and
it's a significant key part for the use of light-weight tokens as
described in the upcoming guide: https://github.com/angular/angular/pull/36144.
In concrete, applications might use injection tokens over classes
for both optional DI and queries, because otherwise such references
cause classes to be always retained. This was also an issue in View
Engine, but now with Ivy, this pattern became worse, as factories are
directly attached to retained classes (ultimately ending up in the
production bundle, while being unused).
More details in the light-weight token guide and in: https://github.com/angular/angular-cli/issues/16866.
Closes#21152. Related to #36144.
PR Close#37506
In 420b9be1c14ec6900692b641a3d89b99eb9c6e5b 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
ASTs for property read and method calls contain information about
the entire span of the expression, including its receiver. Use cases
like a language service and compile error messages may be more
interested in the span of the direct identifier for which the
expression is constructed (i.e. an accessed property). To support this,
this commit adds a `nameSpan` property on
- `PropertyRead`s
- `SafePropertyRead`s
- `PropertyWrite`s
- `MethodCall`s
- `SafeMethodCall`s
The `nameSpan` property already existed for `BindingPipe`s.
This commit also updates usages of these expressions' `sourceSpan`s in
Ngtsc and the langauge service to use `nameSpan`s where appropriate.
PR Close#36826
The html parser already normalizes line endings (converting `\r\n` to `\n`)
for most text in templates but it was missing the expressions of ICU expansions.
In ViewEngine backticked literal strings, used to define inline templates,
were already normalized by the TypeScript parser.
In Ivy we are parsing the raw text of the source file directly so the line
endings need to be manually normalized.
This change ensures that inline templates have the line endings of ICU
expression normalized correctly, which matches the ViewEngine.
In ViewEngine external templates, defined in HTML files, the behavior was
different, since TypeScript was not normalizing the line endings.
Specifically, ICU expansion "expressions" are not being normalized.
This is a problem because it means that i18n message ids can be different on
different machines that are setup with different line ending handling,
or if the developer moves a template from inline to external or vice versa.
The goal is always to normalize line endings, whether inline or external.
But this would be a breaking change since it would change i18n message ids
that have been previously computed. Therefore this commit aligns the ivy
template parsing to have the same "buggy" behavior for external templates.
There is now a compiler option `i18nNormalizeLineEndingsInICUs`, which
if set to `true` will ensure the correct non-buggy behavior. For the time
being this option defaults to `false` to ensure backward compatibility while
allowing opt-in to the desired behavior. This option's default will be
flipped in a future breaking change release.
Further, when this option is set to `false`, any ICU expression tokens,
which have not been normalized, are added to the `ParseResult` from the
`HtmlParser.parse()` method. In the future, this collection of tokens could
be used to diagnose and encourage developers to migrate their i18n message
ids. See FW-2106.
Closes#36725
PR Close#36741
Prior to this change, there was a problem while matching template attributes, which mistakenly took i18n attributes (that might be present in attrs array after template ones) into account. This commit updates the logic to avoid template attribute matching logic from entering the i18n section and as a result this also allows generating proper i18n attributes sections instead of keeping these attribute in plain form (with their values) in attribute arrays.
PR Close#36422
Previously we had a singleton `ROOT_SCOPE` object, from
which all `BindingScope`s derived. But this caused ngcc to
produce non-deterministic output when running multiple workers
in parallel, since each process had its own `ROOT_SCOPE`.
In reality there is no need for `BindingScope` reference names
to be unique across an entire application (or in the case of ngcc
across all the libraries). Instead we just need uniqueness within
a template.
This commit changes the compiler to create a new root `BindingScope`
each time it compiles a component's template.
Resolves#35180
PR Close#36362
This commit augments the `FactoryDef` declaration of Angular decorated
classes to contain information about the parameter decorators used in
the constructor. If no constructor is present, or none of the parameters
have any Angular decorators, then this will be represented using the
`null` type. Otherwise, a tuple type is used where the entry at index `i`
corresponds with parameter `i`. Each tuple entry can be one of two types:
1. If the associated parameter does not have any Angular decorators,
the tuple entry will be the `null` type.
2. Otherwise, a type literal is used that may declare at least one of
the following properties:
- "attribute": if `@Attribute` is present. The injected attribute's
name is used as string literal type, or the `unknown` type if the
attribute name is not a string literal.
- "self": if `@Self` is present, always of type `true`.
- "skipSelf": if `@SkipSelf` is present, always of type `true`.
- "host": if `@Host` is present, always of type `true`.
- "optional": if `@Optional` is present, always of type `true`.
A property is only present if the corresponding decorator is used.
Note that the `@Inject` decorator is currently not included, as it's
non-trivial to properly convert the token's value expression to a
type that is valid in a declaration file.
Additionally, the `ComponentDefWithMeta` declaration that is created for
Angular components has been extended to include all selectors on
`ng-content` elements within the component's template.
This additional metadata is useful for tooling such as the Angular
Language Service, as it provides the ability to offer suggestions for
directives/components defined in libraries. At the moment, such
tooling extracts the necessary information from the _metadata.json_
manifest file as generated by ngc, however this metadata representation
is being replaced by the information emitted into the declaration files.
Resolves FW-1870
PR Close#35695
Prior to this commit, Ivy compiler didn't handle directive inputs with interpolations located on `<ng-template>` elements (e.g. `<ng-template dir="{{ field }}">`). That was the case for regular inputs as well as inputs that should be processed via i18n subsystem (e.g. `<ng-template i18n-dir dir="Hello {{ name }}">`). This commit adds support for such expressions for explicit `<ng-template>`s as well as a number of tests to confirm the behavior.
Fixes#35752.
PR Close#35984
`ɵɵ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
Before this change `[class]` and `[className]` were both converted into `ɵɵclassMap`. The implication of this is that at runtime we could not differentiate between the two and as a result we treated `@Input('class')` and `@Input('className)` as equivalent.
This change makes `[class]` and `[className]` distinct. The implication of this is that `[class]` becomes `ɵɵclassMap` instruction but `[className]` becomes `ɵɵproperty' instruction. This means that `[className]` will no longer participate in styling and will overwrite the DOM `class` value.
Fix#35577
PR Close#35668
Prior to this commit, i18n attributes defined on `<ng-template>` tags were not processed by the compiler. This commit adds the necessary logic to handle i18n attributes in the same way how these attrs are processed for regular elements.
PR Close#35681
This commit removes the `NullAstVisitor` and `visitAstChildren` exported
from `packages/compiler/src/expression_parser/ast.ts` because they
contain duplicate and buggy implementation, and their use cases could be
sufficiently covered by `RecursiveAstVisitor` if the latter implements the
`visit` method. This use case is only needed in the language service.
With this change, any visitor that extends `RecursiveAstVisitor` could
just define their own `visit` function and the parent class will behave
correctly.
A bit of historical context:
In language service, we need a way to tranverse the expression AST in a
selective manner based on where the user's cursor is. This means we need a
"filtering" function to decide which node to visit and which node to not
visit. Instead of refactoring `RecursiveAstVisitor` to support this,
`visitAstChildren` was created. `visitAstChildren` duplicates the
implementation of `RecursiveAstVisitor`, but introduced some bugs along
the way. For example, in `visitKeyedWrite`, it visits
```
obj -> key -> obj
```
instead of
```
obj -> key -> value
```
Moreover, because of the following line
```
visitor.visit && visitor.visit(ast, context) || ast.visit(visitor, context);
```
`visitAstChildren` visits every node *twice*.
PR Close#35619
Currently Ivy always generates the `$event` function argument, even if it isn't being used by the listener expressions. This can lead to unnecessary bytes being generated, because optimizers won't remove unused arguments by default. These changes add some logic to avoid adding the argument when it isn't required.
PR Close#35097
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
Compiler keeps track of number of slots (`vars`) which are needed for binding instructions. Normally each binding instructions allocates a single slot in the `LView` but styling instructions need to allocate two slots.
PR Close#34616
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 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
Pipes in host binding expressions are not supported in View Engine and Ivy, but in some more complex cases (like `(value | pipe) === true`) compiler was not reporting errors. This commit extends Ivy logic to detect pipes in host binding expressions and throw in cases bindings are present. View Engine behavior remains the same.
PR Close#34655
The `getProjectAsAttrValue` in `node_selector_matcher` finds the
ProjectAs marker and then additionally checks that the marker appears in
an even index of the node attributes because "attribute names are stored
at even indexes". This is true for "regular" attribute bindings but
classes, styles, bindings, templates, and i18n do not necessarily follow
this rule because there can be an uneven number of them, causing the
next "special" attribute "name" to appear at an odd index. To address
this issue, ensure ngProjectAs is placed right after "regular"
attributes.
PR Close#34617
Prior to this commit, there were no `advance` instructions generated before `i18nExp` instructions and as a result, lifecycle hooks for components used inside i18n blocks were flushed too late. This commit adds the logic to generate `advance` instructions in front of `i18nExp` ones (similar to what we have in other places like interpolations, property bindings, etc), so that the necessary lifecycle hooks are flushed before expression value is captured.
PR Close#34436
Previously the identifiers used in the typings files were the same as
those used in the source files.
When the typings files and the source files do not match exactly, e.g.
when one of them is flattened, while the other is a deep tree, it is
possible for identifiers to be renamed.
This commit ensures that the correct identifier is used in typings files
when the typings file does not export the same name as the source file.
Fixes https://github.com/angular/ngcc-validation/pull/608
PR Close#34254
Previously, bound events were incorrectly bound to directives with
inputs matching the bound event attribute. This fixes that so bound
events can only be bound to directives with matching outputs.
Adds tests for all kinds of directive matching on bound attributes.
PR Close#31938
To create a binding parser, an instance of `ElementSchemaRegistry` is
required. Prior to this change, each time a new binding parser was
created a new instance of `DomElementSchemaRegistry` would be
instantiated. This is an expensive operation that takes roughly 1ms per
instantiation, so it is key that multiple allocations are avoided.
By sharing a single `DomElementSchemaRegistry`, we avoid two such
allocations, i.e. save ~2ms, per component template.
PR Close#34332
If the `ngI18nClosureMode` global check actually makes it
through to the runtime, then checks for its existence should
be guarded to prevent `Reference undefined` errors in strict
mode.
(Normally, it is stripped out by dead code elimination during
build optimization.)
This comment ensures that generated template code guards
this global check.
PR Close#34211
Prior to this commit, if a template (for example, generated using structural directive such as *ngIf) contains `ngProjectAs` attribute, it was not included into attributes array in generated code and as a result, these templates were not matched at runtime during content projection. This commit adds the logic to append `ngProjectAs` values into corresponding element's attribute arrays, so content projection works as expected.
PR Close#34200
It is possible for HTML formatters to add whitespace
around the content of `i18n` attribute values. This can
make the meaning and custom ids brittle to simple
whitespace formatting.
This commit ensures that the metadata string extracted
from HTML `i18n` attributes is trimmed before being parsed
into meaning, description and custom id.
PR Close#34154
Now that `@angular/localize` can interpret multiple legacy message ids in the
metablock of a `$localize` tagged template string, this commit adds those
ids to each i18n message extracted from component templates, but only if
the `enableI18nLegacyMessageIdFormat` is not `false`.
PR Close#34135
Adds support for chaining of `styleProp`, `classProp` and `stylePropInterpolateX` instructions whenever possible which should help generate less code. Note that one complication here is for `stylePropInterpolateX` instructions where we have to break into multiple chains if there are other styling instructions inbetween the interpolations which helps maintain the execution order.
PR Close#33837
Since i18n messages are mapped to `$localize` tagged template strings,
the "raw" version must be properly escaped. Otherwise TS will throw an
error such as:
```
Error: Debug Failure. False expression: Expected argument 'text' to be the normalized (i.e. 'cooked') version of argument 'rawText'.
```
This commit ensures that we properly escape these raw strings before creating
TS AST nodes from them.
PR Close#33820
The `:` char is used as a metadata marker in `$localize` messages.
If this char appears in the metadata it must be escaped, as `\:`.
Previously, although the `:` char was being escaped, the TS AST
being generated was not correct and so it was being output double
escaped, which meant that it appeared in the rendered message.
As of TS 3.6.2 the "raw" string can be specified when creating tagged
template AST nodes, so it is possible to correct this.
PR Close#33820
Currently if a consumer does something like the following, the object literal will be shared across the two elements and any instances of the component template. The same applies to array literals:
```
<div [someDirective]="{}"></div>
<div [someDirective]="{}"></div>
```
These changes make it so that we generate a pure function even if an object is constant so that each instance gets its own object.
Note that the original design for this fix included moving the pure function factories into the `consts` array. In the process of doing so I realized that pure function are also used inside of directive host bindings which means that we don't have access to the `consts`.
These changes also:
* Fix an issue that meant that the `pureFunction0` instruction could only be run during creation mode.
* Make the `getConstant` utility slightly more convenient to use. This isn't strictly required for these changes to work, but I had made it as a part of a larger refactor that I ended up reverting.
PR Close#33705
Chains multiple listener instructions on a particular element into a single call which results in less generated code. Also handles listeners on templates, host listeners and synthetic host listeners.
PR Close#33720
Prior to this change, namespaced elements such as SVG elements would not
participate correctly in directive matching as their namespace was not
ignored, which was the case with the View Engine compiler. This led to
incorrect behavior at runtime and template type checking.
This commit resolved the issue by ignoring the namespace of elements and
attributes like they were in View Engine.
Fixes#32061
PR Close#33555
When compiling an Angular decorator (e.g. Directive), @angular/compiler
generates an 'expression' to be added as a static definition field
on the class, a 'type' which will be added for that field to the .d.ts
file, and a statement adjacent to the class that calls `setClassMetadata()`.
Previously, the same WrappedNodeExpr of the class' ts.Identifier was used
within each of this situations.
In the ngtsc case, this is proper. In the ngcc case, if the class being
compiled is within an ES5 IIFE, the outer name of the class may have
changed. Thus, the class has both an inner and outer name. The outer name
should continue to be used elsewhere in the compiler and in 'type'.
The 'expression' will live within the IIFE, the `internalType` should be used.
The adjacent statement will also live within the IIFE, the `adjacentType` should be used.
This commit introduces `ReflectionHost.getInternalNameOfClass()` and
`ReflectionHost.getAdjacentNameOfClass()`, which the compiler can use to
query for the correct name to use.
PR Close#33533
This commit moves nested i18n section detection to an earlier stage where we convert HTML AST to Ivy AST. This also gives a chance to produce better diagnistic message for nested i18n sections, that also includes a file name and location.
PR Close#33583
Previously the template binder would crash when encountering an unknown
localref (# reference) such as `<div #ref="foo">` when no directive has
`exportAs: "foo"`.
With this commit, the compiler instead generates a template diagnostic error
informing the user about the invalid reference.
PR Close#33454
Now that we've replaced `ngBaseDef` with an abstract directive definition, there are a lot more cases where we generate a directive definition without a selector. These changes make it so that we don't generate the `selectors` array if it's going to be empty.
PR Close#33431
Removes `ngBaseDef` from the compiler and any runtime code that was still referring to it. In the cases where we'd previously generate a base def we now generate a definition for an abstract directive.
PR Close#33264
For abstract directives, i.e. directives without a selector, it may
happen that their constructor is called explicitly from a subclass,
hence its parameters are not required to be valid for Angular's DI
purposes. Prior to this commit however, having an abstract directive
with a constructor that has parameters that are not eligible for
Angular's DI would produce a compilation error.
A similar scenario may occur for `@Injectable`s, where an explicit
`use*` definition allows for the constructor to be irrelevant. For
example, the situation where `useFactory` is specified allows for the
constructor to be called explicitly with any value, so its constructor
parameters are not required to be valid. For `@Injectable`s this is
handled by generating a DI factory function that throws.
This commit implements the same solution for abstract directives, such
that a compilation error is avoided while still producing an error at
runtime if the type is instantiated implicitly by Angular's DI
mechanism.
Fixes#32981
PR Close#32987
