This commit updates the symbols in the TemplateTypeCheck API and methods
for retrieving them:
* Include `isComponent` and `selector` for directives so callers can determine which
attributes on an element map to the matched directives.
* Add a new `TextAttributeSymbol` and return this when requesting a symbol for a `TextAttribute`.
* When requesting a symbol for `PropertyWrite` and `MethodCall`, use the
`nameSpan` to retrieve symbols.
* Add fix to retrieve generic directives attached to elements/templates.
PR Close#38844
Now that we have `keySpan` for `BoundAttribute` (implemented in
https://github.com/angular/angular/pull/38898) we could do the same
for `Variable`.
This would allow us to distinguish the LHS and RHS from the whole source
span.
PR Close#38965
Though we currently have the knowledge of where the `key` for an
attribute binding appears during parsing, we do not propagate this
information to the output AST. This means that once we produce the
template AST, we have no way of mapping a template position to the key
span alone. The best we can currently do is map back to the
`sourceSpan`. This presents problems downstream, specifically for the
language service, where we cannot provide correct information about a
position in a template because the AST is not granular enough.
PR Close#38898
Common AST formats such as TS and Babel do not use a separate
node for comments, but instead attach comments to other AST nodes.
Previously this was worked around in TS by creating a `NotEmittedStatement`
AST node to attach the comment to. But Babel does not have this facility,
so it will not be a viable approach for the linker.
This commit refactors the output AST, to remove the `CommentStmt` and
`JSDocCommentStmt` nodes. Instead statements have a collection of
`leadingComments` that are rendered/attached to the final AST nodes
when being translated or printed.
PR Close#38811
Recent work on compiler internals in #38539 led to an unexpected failure,
where a pipe used exclusively inside of an ICU would no longer be
emitted into the compilation output. This caused runtime errors due to
missing pipes.
The issue occurred because the change in #38539 would determine the set
of used pipes up-front, independent from the template compilation using
the `R3TargetBinder`. However, `R3TargetBinder` did not consider
expressions within ICUs, so any pipe usages within those expressions
would not be detected. This fix unblocks #38539 and also concerns
upcoming linker work, given that prelink compilations would not go
through full template compilation but only `R3TargetBinder`.
PR Close#38810
Previously, localized strings had very limited or incorrect source-mapping
information available.
Now the i18n AST nodes and related output AST nodes include source-span
information about message-parts and placeholders - including closing tag
placeholders.
This information is then used when generating the final localized string
ASTs to ensure that the correct source-mapping is rendered.
See #38588 (comment)
PR Close#38645
The `MessagePiece` and derived classes, `LiteralPiece` and `PlaceholderPiece`
need to be referenced in the `LocalizedString` output AST class, so that we
can render the source-spans of each piece.
PR Close#38645
Previously this interface was mostly stored in compiler-cli, but it
contains some properties that would be useful for compiling the
"declare component" prelink code.
This commit moves some of the interface over to the compiler
package so that it can be referenced there without creating a
circular dependency between the compiler and compiler-cli.
PR Close#38594
The `R3TargetBinder` accepts an interface for directive metadata which
declares types for `input` and `output` objects. These types convey the
mapping between the property names for an input or output and the
corresponding property name on the component class. Due to
`R3TargetBinder`'s requirements, this mapping was specified with property
names as keys and field names as values.
However, because of duck typing, this interface was accidentally satisifed
by the opposite mapping, of field names to property names, that was produced
in other parts of the compiler. This form more naturally represents the data
model for inputs.
Rather than accept the field -> property mapping and invert it, this commit
introduces a new abstraction for such mappings which is bidirectional,
eliminating the ambiguous plain object type. This mapping uses new,
unambiguous terminology ("class property name" and "binding property name")
and can be used to satisfy both the needs of the binder as well as those of
the template type-checker (field -> property).
A new test ensures that the input/output metadata produced by the compiler
during analysis is directly compatible with the binder via this unambiguous
new interface.
PR Close#38685
Previously, the compiler was not able to display template parsing errors as
true `ts.Diagnostic`s that point inside the template. Instead, it would
throw an actual `Error`, and "crash" with a stack trace containing the
template errors.
Not only is this a poor user experience, but it causes the Language Service
to also crash as the user is editing a template (in actuality the LS has to
work around this bug).
With this commit, such parsing errors are converted to true template
diagnostics with appropriate span information to be displayed contextually
along with all other diagnostics. This majorly improves the user experience
and unblocks the Language Service from having to deal with the compiler
"crashing" to report errors.
PR Close#38576
Previously, the `sourceSpan` and `startSourceSpan` were the same
object, which meant that you had the following situation:
```
element = <div>some content</div>
sourceSpan = <div>
startSourceSpan = <div>
endSourceSpan = </div>
```
This made `sourceSpan` redundant and meant that if you
wanted a span for the whole element including its content
and closing tag, it had to be computed.
Now `sourceSpan` is separated from `startSourceSpan`
resulting in:
```
element = <div>some content</div>
sourceSpan = <div>some content</div>
startSourceSpan = <div>
endSourceSpan = </div>
```
PR Close#38581
Previously, the `startSourceSpan` property could be null
but in reality it is always well defined - except for a legacy
case in the old i18n extraction/merging code, where the
typings for source-spans are already being undermined.
Making this property non-null, simplifies code elsewhere
in the project.
PR Close#38581
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 using the safe navigation operator in a binding expression, a temporary
variable may be used for storing the result of a side-effectful call.
For example, the following template uses a pipe and a safe property access:
```html
<app-person-view [enabled]="enabled" [firstName]="(person$ | async)?.name"></app-person-view>
```
The result of the pipe evaluation is stored in a temporary to be able to check
whether it is present. The temporary variable needs to be declared in a separate
statement and this would also cause the full expression itself to be pulled out
into a separate statement. This would compile into the following
pseudo-code instructions:
```js
var temp = null;
var firstName = (temp = pipe('async', ctx.person$)) == null ? null : temp.name;
property('enabled', ctx.enabled)('firstName', firstName);
```
Notice that the pipe evaluation happens before evaluating the `enabled` binding,
such that the runtime's internal binding index would correspond with `enabled`,
not `firstName`. This introduces a problem when the pipe uses `WrappedValue` to
force a change to be detected, as the runtime would then mark the binding slot
corresponding with `enabled` as dirty, instead of `firstName`. This results
in the `enabled` binding to be updated, triggering setters and affecting how
`OnChanges` is called.
In the pseudo-code above, the intermediate `firstName` variable is not strictly
necessary---it only improved readability a bit---and emitting it inline with
the binding itself avoids the out-of-order execution of the pipe:
```js
var temp = null;
property('enabled', ctx.enabled)
('firstName', (temp = pipe('async', ctx.person$)) == null ? null : temp.name);
```
This commit introduces a new `BindingForm` that results in the above code to be
generated and adds compiler and acceptance tests to verify the proper behavior.
Fixes#37194
PR Close#37911
Currently the `getInheritedFactory` function is implemented to allow
closure to remove the call if the base factory is unused. However, this
method does not work with terser. By adding the PURE annotation,
terser will also be able to remove the call when unused.
PR Close#38291
Prior to this commit, duplicated styles defined in multiple components in the same file were not
shared between components, thus causing extra payload size. This commit updates compiler logic to
use `ConstantPool` for the styles (while generating the `styles` array on component def), which
enables styles sharing when needed (when duplicates styles are present).
Resolves#38204.
PR Close#38213
This commit updates synthetic host property and listener instruction names to better align with other instructions.
The `ɵɵupdateSyntheticHostBinding` instruction was renamed to `ɵɵsyntheticHostProperty` (to match the `ɵɵhostProperty`
instruction name) and `ɵɵcomponentHostSyntheticListener` was renamed to `ɵɵsyntheticHostListener` since this
instruction is generated for both Components and Directives (so 'component' is removed from the name).
This PR is a followup after PR #35568.
PR Close#37145
Fixes the following issues related to how we validate properties during JIT:
- The invalid property warning was printing `null` as the node name
for `ng-content`. The problem is that when generating a template from
`ng-content` we weren't capturing the node name.
- We weren't running property validation on `ng-container` at all.
This used to be supported on ViewEngine and seems like an oversight.
In the process of making these changes, I found and cleaned up a
few places where we were passing in `LView` unnecessarily.
PR Close#37773
Currently when the `plural` or `select` keywords in an ICU contain trailing spaces (e.g. `{count, select , ...}`), these spaces are also included into the key names in ICU vars (e.g. "VAR_SELECT "). These trailing spaces are not desirable, since they will later be converted into `_` symbols while normalizing placeholder names, thus causing mismatches at runtime (i.e. placeholder will not be replaced with the correct value). This commit updates the code to trim these spaces while generating an object with placeholders, to make sure the runtime logic can replace these placeholders with the right values.
PR Close#37866
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 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
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
This commit propagates the `sourceSpan` and `valueSpan` of a `VariableBinding`
in a microsyntax expression to `ParsedVariable`, and subsequently to
View Engine Variable AST and Ivy Variable AST.
Note that this commit does not propagate the `keySpan`, because it involves
significant changes to the template AST.
PR Close#36047
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
Currently, would-be binding attributes that are missing binding names
are not parsed as bindings, and fall through as regular attributes. In
some cases, this can lead to a runtime error; trying to assign `#` as a
DOM attribute in an element like in `<div #></div>` fails because `#` is
not a valid attribute name.
Attributes composed of binding prefixes but not defining a binding
should be considered invalid, as this almost certainly indicates an
unintentional elision of a binding by the developer. This commit
introduces error reporting for attributes with a binding name prefix but
no actual binding name.
Closes https://github.com/angular/vscode-ng-language-service/issues/293.
PR Close#34595
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
Andrew Scott