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
Typescript 3.7 now emits d.ts files for getters differently than prior versions,
and there seems to be a bug in how it strips private types without replacing them
with explicit 'any' type. This then leads to compilation failures in projects compiled
against our packages that don't have skipLibCheck turned on but do have strict or
noImplicitAny check on.
I'm working around this by marking the affected getters as @internal and
adding a test to prevent future regressions.
I believe this is a TypeScript bug, and I filed a bug report:
https://github.com/microsoft/TypeScript/issues/36216
PR Close#34798
The major one that affects the angular repo is the removal of the bootstrap attribute in nodejs_binary, nodejs_test and jasmine_node_test in favor of using templated_args --node_options=--require=/path/to/script. The side-effect of this is that the bootstrap script does not get the require.resolve patches with explicitly loading the targets _loader.js file.
PR Close#34736
The major one that affects the angular repo is the removal of the bootstrap attribute in nodejs_binary, nodejs_test and jasmine_node_test in favor of using templated_args --node_options=--require=/path/to/script. The side-effect of this is that the bootstrap script does not get the require.resolve patches with explicitly loading the targets _loader.js file.
PR Close#34589
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
In the past, only the starting index of an expression Token has been
recorded, so a parser could demarkate the span of a token only by the
start locations of two tokens. This may lead to trailing whitespace
being included in the token span:
```html
{{ token1 + token2 }}
^^^^^^^^^ recorded span of `token1`
```
It's also not enough for a parser to determine the end of a token by
adding the length of the token value to the token's start location,
because lexed expression values may not exactly reflect the source code.
For example, `"d\\"e"` is lexed as a string token whose value is `d"e`.
Instead, this commit adds a `end` field to expression tokens. `end`
is one past the last index of the token source code. This will enable a
parser to determine the span of a token just by looking at that token.
This is a breaking change because the contructor interface of `Token`
has changed.
Part of #33477.
PR Close#33549
Previously, the type checker would compute an absolute source span by
combining an expression AST node's `ParseSpan` (relative to the start of
the expression) together with the absolute offset of the expression as
represented in a `ParseSourceSpan`, to arrive at a span relative to the
start of the file. This information is now directly available on an
expression AST node in the `AST.sourceSpan` property, which can be used
instead.
PR Close#34417
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
Expressions in an inline template binding are improperly recorded as
spaning an offset calculated from the start of the template binding
attribute key, whereas they should be calculated from the start of the
attribute value, which contains the actual binding AST.
PR Close#31813
Avoids the usage of array destructuring, as it introduces calls to
a `__values` helper function in ES5 that has a relatively high
performance impact. This shaves off roughly 130ms of CPU time for a
large compilation with big templates that uses i18n.
PR Close#34332
The template parser has a certain interpolation config associated with
it and builds a regular expression each time it needs to extract the
interpolations from an input string. Since the interpolation config is
typically the default of `{{` and `}}`, the regular expression doesn't
have to be recreated each time. Therefore, this commit creates only a
single regular expression instance that is used for the default
configuration.
In a large compilation unit with big templates, computing the regular
expression took circa 275ms. This change reduces this to effectively
zero.
PR Close#34332
On a large compilation unit with big templates, the total time spent in
the `PlainCharacterCursor` constructor was 470ms. This commit applies
two optimizations to reduce this time:
1. Avoid the object spread operator within the constructor, as the
generated `__assign` helper in the emitted UMD bundle (ES5) does not
optimize well compared to a hardcoded object literal. This results in a
significant performance improvement. Because of the straight-forward
object literal, the VM is now much better able to optimize the memory
allocations which makes a significant difference as the
`PlainCharacterCursor` constructor is called in tight loops.
2. Reduce the number of `CharacterCursor` clones. Although cloning
itself is now much faster because of the optimization above, several
clone operations were not necessary.
Combined, these changes reduce the total time spent in the
`PlainCharacterCursor` constructor to just 10ms.
PR Close#34332
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
The `ModuleWithProviders` type has an optional type parameter that
should be specified to indicate what NgModule class will be provided.
This enables the Ivy compiler to statically determine the NgModule type
from the declaration files. This type parameter will become required in
the future, however to aid in the migration the compiler will detect
code patterns where using `ModuleWithProviders` as return type is
appropriate, in which case it transforms the emitted .d.ts files to
include the generic type argument.
This should reduce the number of occurrences where `ModuleWithProviders`
is referenced without its generic type argument.
Resolves FW-389
PR Close#34235
Occasionally a factory function needs to be generated for an "invalid"
constructor (one with parameters types which aren't injectable). Typically
this happens in JIT mode where understanding of parameters cannot be done in
the same "up-front" way that the AOT compiler can.
This commit changes the JIT compiler to generate a new `invalidFactoryDep`
call for each invalid parameter. This instruction will error at runtime if
called, indicating both the index of the invalid parameter as well as (via
the stack trace) the factory function which was generated for the type being
constructed.
Fixes#33637
PR Close#33739
Previously the JIT evaluated code for ivy localized strings included
backtick tagged template strings, which are not compatible with ES5
in legacy browsers such as IE 11.
Now the generated code is ES5 compatible.
Fixes#34246
PR Close#34265
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
We should only generate the `providedIn` property in injectable
defs if it has a non-null value. `null` does not communicate
any information to the runtime that isn't communicated already
by the absence of the property.
This should give us some modest code size savings.
PR Close#34116
For injectables, we currently generate a factory function in the
injectable def (prov) that delegates to the factory function in
the factory def (fac). It looks something like this:
```
factory: function(t) { return Svc.fac(t); }
```
The extra wrapper function is unnecessary since the args for
the factory functions are the same. This commit changes the
compiler to generate this instead:
```
factory: Svc.fac
```
Because we are generating less code for each injectable, we
should see some modest code size savings. AIO's main bundle
is about 1 KB smaller.
PR Close#34076
Prior to this commit, all styles extracted from Component's template (defined using <style> tags) were ignored by JIT compiler, so only `styles` array values defined in @Component decorator were used. This change updates JIT compiler to take styles extracted from the template into account. It also ensures correct order where `styles` array values are applied first and template styles are applied second.
PR Close#34017
This commit transforms the setClassMetadata calls generated by ngtsc from:
```typescript
/*@__PURE__*/ setClassMetadata(...);
```
to:
```typescript
/*@__PURE__*/ (function() {
setClassMetadata(...);
})();
```
Without the IIFE, terser won't remove these function calls because the
function calls have arguments that themselves are function calls or other
impure expressions. In order to make the whole block be DCE-ed by terser,
we wrap it into IIFE and mark the IIFE as pure.
It should be noted that this change doesn't have any impact on CLI* with
build-optimizer, which removes the whole setClassMetadata block within
the webpack loader, so terser or webpack itself don't get to see it at
all. This is done to prevent cross-chunk retention issues caused by
webpack's internal module registry.
* actually we do expect a short-term size regression while
https://github.com/angular/angular-cli/pull/16228
is merged and released in the next rc of the CLI. But long term this
change does nothing to CLI + build-optimizer configuration and is done
primarly to correct the seemingly correct but non-function PURE annotation
that builds not using build-optimizer could rely on.
PR Close#33337
NgModules in Ivy have a definition which contains various different bits
of metadata about the module. In particular, this metadata falls into two
categories:
* metadata required to use the module at runtime (for bootstrapping, etc)
in AOT-only applications.
* metadata required to depend on the module from a JIT-compiled app.
The latter metadata consists of the module's declarations, imports, and
exports. To support JIT usage, this metadata must be included in the
generated code, especially if that code is shipped to NPM. However, because
this metadata preserves the entire NgModule graph (references to all
directives and components in the app), it needs to be removed during
optimization for AOT-only builds.
Previously, this was done with a clever design:
1. The extra metadata was added by a function called `setNgModuleScope`.
A call to this function was generated after each NgModule.
2. This function call was marked as "pure" with a comment and used
`noSideEffects` internally, which causes optimizers to remove it.
The effect was that in dev mode or test mode (which use JIT), no optimizer
runs and the full NgModule metadata was available at runtime. But in
production (presumably AOT) builds, the optimizer runs and removes the JIT-
specific metadata.
However, there are cases where apps that want to use JIT in production, and
still make an optimized build. In this case, the JIT-specific metadata would
be erroneously removed. This commit solves that problem by adding an
`ngJitMode` global variable which guards all `setNgModuleScope` calls. An
optimizer can be configured to statically define this global to be `false`
for AOT-only builds, causing the extra metadata to be stripped.
A configuration for Terser used by the CLI is provided in `tooling.ts` which
sets `ngJitMode` to `false` when building AOT apps.
PR Close#33671
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
This is a breaking change in nodejs rules 0.40.0 as part of the API review & cleanup for the 1.0 release. Their APIs are identical as ts_web_test was just karma_web_test without the config_file attribute.
PR Close#33802
Prior to this change, ComponentFactory.create function invocation in Ivy retained the content of the host element (in case host element reference or CSS seelctor is provided as an argument). This behavior is different in View Engine, where the content of the host element was cleared, except for the case when ShadowDom encapsulation is used (to make sure native slot projection works). This commit aligns Ivy and View Engine and makes sure the host element is cleared before component content insertion.
PR Close#33487
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
Previously the compiler would ignore everything in the attribute
name after the first dot. For example
<div [attr.someAttr.attrSuffix]="var"></div>
is turned into <div someAttr="varValue"></div>.
This commit ensures that whole attribute name is captured.
Now <div [attr.someAttr.attrSuffix]="var"></div>
is turned into <div someAttr.attrSuffix="varValue"></div>
PR Close#32256
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 compiler would crash if a pipe was encountered which did not
match any pipe in the scope of a template.
This commit introduces a new diagnostic error for unknown pipes instead.
PR Close#33454
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
The parser was accidentally reading the `target` tag
below the `alt-trans` target and overriding the correct
`target` tag.
(This already worked in `$localize` but a test has been
added to confirm.)
Fixes#33161
PR Close#33450
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
In Angular View Engine, there are two kinds of decorator inheritance:
1) both the parent and child classes have decorators
This case is supported by InheritDefinitionFeature, which merges some fields
of the definitions (such as the inputs or queries).
2) only the parent class has a decorator
If the child class is missing a decorator, the compiler effectively behaves
as if the parent class' decorator is applied to the child class as well.
This is the "undecorated child" scenario, and this commit adds a migration
to ngcc to support this pattern in Ivy.
This migration has 2 phases. First, the NgModules of the application are
scanned for classes in 'declarations' which are missing decorators, but
whose base classes do have decorators. These classes are the undecorated
children. This scan is performed recursively, so even if a declared class
has a base class that itself inherits a decorator, this case is handled.
Next, a synthetic decorator (either @Component or @Directive) is created
on the child class. This decorator copies some critical information such
as 'selector' and 'exportAs', as well as supports any decorated fields
(@Input, etc). A flag is passed to the decorator compiler which causes a
special feature `CopyDefinitionFeature` to be included on the compiled
definition. This feature copies at runtime the remaining aspects of the
parent definition which `InheritDefinitionFeature` does not handle,
completing the "full" inheritance of the child class' decorator from its
parent class.
PR Close#33362
This commit adds CopyDefinitionFeature, which supports the case where an
entire decorator (@Component or @Directive) is inherited from parent to
child.
The existing inheritance feature, InheritDefinitionFeature, supports merging
of parent and child definitions when both were originally present. This
merges things like inputs, outputs, host bindings, etc.
CopyDefinitionFeature, on the other hand, compensates for a definition that
was missing entirely on the child class, by copying fields that aren't
ordinarily inherited (like the template function itself).
This feature is intended to only be used as part of ngcc code generation.
PR Close#33362
This patch ensures that the `[style]` and `[class]` based bindings
are directly applied to an element's style and className attributes.
This patch optimizes the algorithm so that it...
- Doesn't construct an update an instance of `StylingMapArray` for
`[style]` and `[class]` bindings
- Doesn't apply `[style]` and `[class]` based entries using
`classList` and `style` (direct attributes are used instead)
- Doesn't split or iterate over all string-based tokens in a
string value obtained from a `[class]` binding.
This patch speeds up the following cases:
- `<div [class]>` and `<div class="..." [class]>`
- `<div [style]>` and `<div style="..." [style]>`
The overall speec increase is by over 5x.
PR Close#33336
Recently it was made possible to have a directive without selector,
which are referred to as abstract directives. Such directives should not
be registered in an NgModule, but can still contain decorators for
inputs, outputs, queries, etc. The information from these decorators and
the `@Directive()` decorator itself needs to be registered with the
central `MetadataRegistry` so that other areas of the compiler can
request information about a given directive, an example of which is the
template type checker that needs to know about the inputs and outputs of
directives.
Prior to this change, however, abstract directives would only register
themselves with the `MetadataRegistry` as being an abstract directive,
without all of its other metadata like inputs and outputs. This meant
that the template type checker was unable to resolve the inputs and
outputs of these abstract directives, therefore failing to check them
correctly. The typical error would be that some property does not exist
on a DOM element, whereas said property should have been bound to the
abstract directive's input.
This commit fixes the problem by always registering the metadata of a
directive or component with the `MetadataRegistry`. Tests have been
added to ensure abstract directives are handled correctly in the
template type checker, together with tests to verify the form of
abstract directives in declaration files.
Fixes#30080
PR Close#33131
Previously, we had tested that expressions parsed in a Render3 AST
had correctly-defined absolute spans (spans relative to the entire
template, not the local expression). Sometimes we use Template ASTs
rather than Render3 ASTs, and it's desirable to test for correct
expression spans in the template parser as well.
Adding these tests resolved one bug, similar to the one fixed in
fd4fed14d8, where expressions in the value
of a template attribute were not given an absolute span corresponding to
the start of the attribute name rather than the start of the attribute
value.
The diff on this commit is large, partially because it involves some
structural changes of the template parser testing layout. In particular,
the following is done:
1. Move `createMeta*`-like functions from `template_parser_spec.ts` to
be exported from a new test utility file.
2. Create an `ExpressionSourceHumanizer`, similar to the one created in
b04488d692, to allow convenient testing
of expressions' locations.
3. Create `template_parser_absolute_span_spec.ts`, testing the spans of
expressions parsed by the template parser. This is very similar to
the `r3_ast_absolute_span_spec`.
PR Close#33253
Libraries can expose directive/component base classes that will be
used by consumer applications. Using such a base class from another
compilation unit works fine with "ngtsc", but when using "ngc", the
compiler will thrown an error saying that the base class is not
part of a NgModule. e.g.
```
Cannot determine the module for class X in Y! Add X to the NgModule to fix it.
```
This seems to be because the logic for distinguishing directives from
abstract directives is scoped to the current compilation unit within
ngc. This causes abstract directives from other compilation units to
be considered as actual directives (causing the exception).
PR Close#33347
Prior to this commit, we always invoked second i18n pass (in case whitespace removal is on, which is a default), even if a given template doesn't contain i18n information. Now we store a flag (that indicates presence of i18n information in a template) during first i18n pass and use it to check whether second pass is needed.
PR Close#33284
When computing i18n messages for templates there are two passes.
This is because messages must be computed before any whitespace
is removed. Then on a second pass, the messages must be recreated
but reusing the message ids from the first pass.
Previously ICUs were losing their legacy ids that had been computed
via the first pass. This commit fixes that by keeping track of the
message from the first pass (`previousMessage`) for ICU placeholder
nodes.
// FW-1637
PR Close#33318
Previously the parameter was `id` which is ambigous because it
could be a computed value rather than a developer specified custom
value.
PR Close#33318
This commit cleans up the I18MetaVisitor code by moving all the
state of the visitor into a `context` object that gets passed along
as the nodes are being visited. This is in keeping with how visitors
are designed but also makes it easy to remove the
[definite assignment assertions](https://mariusschulz.com/blog/strict-property-initialization-in-typescript#solution-4-definite-assignment-assertion)
from the class properties.
Also, a `I18nMessageFactory` named type is exported to make it
clearer to consumers of the `createI18nMessageFactory()` function.
PR Close#33318
This is a potential fix for https://github.com/angular/vscode-ng-language-service/issues/235
suggested by @andrius-pra in
47696136e3.
Currently, CRLF line endings are converted to LFs and this causes the
diagnostics span to be off in templates that use CRLF. The line endings
must be preserved in order to maintain correct span offset. The solution
is to add an option to the Tokenizer to indicate such preservation.
PR Close#33241
Prior to this commit, the absolute spans (relative to template source
file rather than the start of an expression) of expressions in a
template attribute like `*ngIf` were generated incorrectly, equating to
the relative spans.
This fixes the bug by passing an `absoluteOffset` parameter when parsing
template bindings.
Through some levels of indirection, this is required for the Language
Service to support text replacement in
https://github.com/angular/angular/pull/33091.
PR Close#33189
Prior to this commit, metadata defined on ICU container element was not inherited by the ICU if the whole message is a single ICU (for example: `<ng-container i18n="meaning|description@@id">{count, select, ...}</ng-container>). This commit updates the logic to use parent container i18n meta information for the cases when a message consists of a single ICU.
Fixes#33171
PR Close#33191
Prior to this commit, all `className` inputs were not set because the runtime code assumed that the `classMap` instruction is only generated for `[class]` bindings. However the `[className]` binding also produces the same `classMap`, thus the code needs to distinguish between `class` and `className`. This commit adds extra logic to select the right input name and also throws an error in case `[class]` and `[className]` bindings are used on the same element simultaneously.
PR Close#33188
Injectable defs are not considered public API, so the property
that contains them should be prefixed with Angular's marker
for "private" ('ɵ') to discourage apps from relying on def
APIs directly.
This commit adds the prefix and shortens the name from
ngInjectableDef to "prov" (for "provider", since injector defs
are known as "inj"). This is because property names cannot
be minified by Uglify without turning on property mangling
(which most apps have turned off) and are thus size-sensitive.
PR Close#33151
Injector defs are not considered public API, so the property
that contains them should be prefixed with Angular's marker
for "private" ('ɵ') to discourage apps from relying on def
APIs directly.
This commit adds the prefix and shortens the name from
ngInjectorDef to inj. This is because property names
cannot be minified by Uglify without turning on property
mangling (which most apps have turned off) and are thus
size-sensitive.
PR Close#33151
The `legacyMessageIdFormat` is taken from the `i18nInFormat` property but we were only considering
`xmb`, `xlf` and `xlf2` values.
The CLI also supports `xliff` and `xliff2` values for the
`i18nInFormat`.
This commit adds support for those aliases.
PR Close#33160
Module defs are not considered public API, so the property
that contains them should be prefixed with Angular's marker
for "private" ('ɵ') to discourage apps from relying on def
APIs directly.
This commit adds the prefix and shortens the name from
ngModuleDef to mod. This is because property names
cannot be minified by Uglify without turning on property
mangling (which most apps have turned off) and are thus
size-sensitive.
PR Close#33142
Pipe defs are not considered public API, so the property
that contains them should be prefixed with Angular's marker
for "private" ('ɵ') to discourage apps from relying on def
APIs directly.
This commit adds the prefix and shortens the name from
ngPipeDef to pipe. This is because property names
cannot be minified by Uglify without turning on property
mangling (which most apps have turned off) and are thus
size-sensitive.
PR Close#33142
Factory defs are not considered public API, so the property
that contains them should be prefixed with Angular's marker
for "private" ('ɵ') to discourage apps from relying on def
APIs directly.
This commit adds the prefix and shortens the name from
ngFactoryDef to fac. This is because property names
cannot be minified by Uglify without turning on property
mangling (which most apps have turned off) and are thus
size-sensitive.
Note that the other "defs" (ngPipeDef, etc) will be
prefixed and shortened in follow-up PRs, in an attempt to
limit how large and conflict-y this change is.
PR Close#33116
Currently, the spans of expressions are recorded only relative to the
template node that they reside in, not their source file.
Introduce a `sourceSpan` property on expression ASTs that records the
location of an expression relative to the entire source code file that
it is in. This may allow for reducing duplication of effort in
ngtsc/typecheck/src/diagnostics later on as well.
Child of #31898
PR Close#31897
BREAKING CHANGE:
We no longer directly have a direct depedency on `tslib`. Instead it is now listed a `peerDependency`.
Users not using the CLI will need to manually install `tslib` via;
```
yarn add tslib
```
or
```
npm install tslib --save
```
PR Close#32167
Directive defs are not considered public API, so the property
that contains them should be prefixed with Angular's marker
for "private" ('ɵ') to discourage apps from relying on def
APIs directly.
This commit adds the prefix and shortens the name from
ngDirectiveDef to dir. This is because property names
cannot be minified by Uglify without turning on property
mangling (which most apps have turned off) and are thus
size-sensitive.
Note that the other "defs" (ngFactoryDef, etc) will be
prefixed and shortened in follow-up PRs, in an attempt to
limit how large and conflict-y this change is.
PR Close#33110
Component defs are not considered public API, so the property
that contains them should be prefixed with Angular's marker
for "private" ('ɵ') to discourage apps from relying on def
APIs directly.
This commit adds the prefix and shortens the name from
`ngComponentDef` to `cmp`. This is because property names
cannot be minified by Uglify without turning on property
mangling (which most apps have turned off) and are thus
size-sensitive.
Note that the other "defs" (ngDirectiveDef, etc) will be
prefixed and shortened in follow-up PRs, in an attempt to
limit how large and conflict-y this change is.
PR Close#33088
For v9 we want the migration to the new i18n to be as
simple as possible.
Previously the developer had to positively choose to use
legacy messsage id support in the case that their translation
files had not been migrated to the new format by setting the
`legacyMessageIdFormat` option in tsconfig.json to the format
of their translation files.
Now this setting has been changed to `enableI18nLegacyMessageFormat`
as is a boolean that defaults to `true`. The format is then read from
the `i18nInFormat` option, which was previously used to trigger translations
in the pre-ivy angular compiler.
PR Close#33053
This commit implements a tool that will inline translations and generate
a translated copy of a set of application files from a set of translation
files.
PR Close#32881
Currently Ivy stores the element attributes into an array above the component def and passes it into the relevant instructions, however the problem is that upon minification the array will get a unique name which won't compress very well. These changes move the attributes array into the component def and pass in the index into the instructions instead.
Before:
```
const _c0 = ['foo', 'bar'];
SomeComp.ngComponentDef = defineComponent({
template: function() {
element(0, 'div', _c0);
}
});
```
After:
```
SomeComp.ngComponentDef = defineComponent({
consts: [['foo', 'bar']],
template: function() {
element(0, 'div', 0);
}
});
```
A couple of cases that this PR doesn't handle:
* Template references are still in a separate array.
* i18n attributes are still in a separate array.
PR Close#32798
Re-enables the dynamic queries migration, now that we have all of the necessary framework changes in place.
Also moves the logic that identifies static queries out of the compiler and into the static queries migration, because that's the only place left that's using it.
PR Close#32992
The creation of StaticReflector in createMetadataResolver() is a very expensive operation because it involves numerous module resolutions.
To make matter worse, since the API of the Reflector does not provide the ability to invalidate its internal caches, it has to be destroyed and recreated on *every* program change.
This has a HUGE impact on performance.
This PR fixes this problem by carefully invalidating all StaticSymbols in a file that has changed, thereby reducing the overhead of recomputation on program change.
PR Close#32543
This is a re-submit of #32686.
Switches back to having the static flag be optional on ViewChild and ContentChild queries, in preparation for changing its default value.
PR Close#32986
These changes switch to defaulting the `static` flag on `ViewChild` and `ContentChild` queries to `false`, in addition to removing the logic that statically determines whether a query is dynamic.
PR Close#32720
The `$localize` library uses a new message digest function for
computing message ids. This means that translations in legacy
translation files will no longer match the message ids in the code
and so will not be translated.
This commit adds the ability to specify the format of your legacy
translation files, so that the appropriate message id can be rendered
in the `$localize` tagged strings. This results in larger code size
and requires that all translations are in the legacy format.
Going forward the developer should migrate their translation files
to use the new message id format.
PR Close#32937
In an attempt to be compatible with previous translation files
the Angular compiler was generating instructions that always
included the message id. This was because it was not possible
to accurately re-generate the id from the calls to `$localize()` alone.
In line with https://hackmd.io/EQF4_-atSXK4XWg8eAha2g this
commit changes the compiler so that it only renders ids if they are
"custom" ones provided by the template author.
NOTE:
When translating messages generated by the Angular compiler
from i18n tags in templates, the `$localize.translate()` function
will compute message ids, if no custom id is provided, using a
common digest function that only relies upon the information
available in the `$localize()` calls.
This computed message id will not be the same as the message
ids stored in legacy translation files. Such files will need to be
migrated to use the new common digest function.
This only affects developers who have been trialling `$localize`, have
been calling `loadTranslations()`, and are not exclusively using custom
ids in their templates.
PR Close#32867
Metadata blocks are delimited by colons. Previously the code naively just
looked for the next colon in the string as the end marker.
This commit supports escaping colons within the metadata content.
The Angular compiler has been updated to add escaping as required.
PR Close#32867
Previously the metadata and placeholder blocks were serialized in
a variety of places. Moreover the code for creating the `LocalizedString`
AST node was doing serialization, which break the separation of concerns.
Now this is all done by the code that renders the AST and is refactored into
helper functions to avoid repeating the behaviour.
PR Close#32867
With #31953 we moved the factories for components, directives and pipes into a new field called `ngFactoryDef`, however I decided not to do it for injectables, because they needed some extra logic. These changes set up the `ngFactoryDef` for injectables as well.
For reference, the extra logic mentioned above is that for injectables we have two code paths:
1. For injectables that don't configure how they should be instantiated, we create a `factory` that proxies to `ngFactoryDef`:
```
// Source
@Injectable()
class Service {}
// Output
class Service {
static ngInjectableDef = defineInjectable({
factory: () => Service.ngFactoryFn(),
});
static ngFactoryFn: (t) => new (t || Service)();
}
```
2. For injectables that do configure how they're created, we keep the `ngFactoryDef` and generate the factory based on the metadata:
```
// Source
@Injectable({
useValue: DEFAULT_IMPL,
})
class Service {}
// Output
export class Service {
static ngInjectableDef = defineInjectable({
factory: () => DEFAULT_IMPL,
});
static ngFactoryFn: (t) => new (t || Service)();
}
```
PR Close#32433
Switches back to having the `static` flag be optional on `ViewChild` and `ContentChild` queries, in preparation for changing its default value.
PR Close#32686
These changes switch to defaulting the `static` flag on `ViewChild` and `ContentChild` queries to `false`, in addition to removing the logic that statically determines whether a query is dynamic.
PR Close#32720
Prior to this commit, the `ngProjectAs` attribute was only included with a special flag and in a parsed format. As a result, projected node was missing `ngProjectAs` attribute as well as other attributes added after `ngProjectAs` one. This is problematic since app code might rely on the presence of `ngProjectAs` attribute (for example in CSS). This commit fixes the problem by including `ngProjectAs` into attributes array as a regular attribute and also makes sure that the parsed version of the `ngProjectAs` attribute with a special marker is added after regular attributes (thus we set them correctly at runtime). This change also aligns View Engine and Ivy behavior.
PR Close#32784
Now that the `$localize` translations are `MessageId` based the
compiler must render `MessageId`s in its generated `$localize` code.
This is because the `MessageId` used by the compiler is computed
from information that does not get passed through to the `$localize`
tagged string.
For example, the generated code for the following template
```html
<div id="static" i18n-title="m|d" title="introduction"></div>
```
will contain these localization statements
```ts
if (ngI18nClosureMode) {
/**
* @desc d
* @meaning m
*/
const MSG_EXTERNAL_8809028065680254561$$APP_SPEC_TS_1 = goog.getMsg("introduction");
I18N_1 = MSG_EXTERNAL_8809028065680254561$$APP_SPEC_TS_1;
}
else {
I18N_1 = $localize \`:m|d@@8809028065680254561:introduction\`;
}
```
Since `$localize` is not able to accurately regenerate the source-message
(and so the `MessageId`) from the generated code, it must rely upon the
`MessageId` being provided explicitly in the generated code.
The compiler now prepends all localized messages with a "metadata block"
containing the id (and the meaning and description if defined).
Note that this metadata block will also allow translation file extraction
from the compiled code - rather than relying on the legacy ViewEngine
extraction code. (This will be implemented post-v9).
Although these metadata blocks add to the initial code size, compile-time
inlining will completely remove these strings and so will not impact on
production bundle size.
PR Close#32594
As discussed in https://hackmd.io/33M5Wb-JT7-0fneA0JuHPA `SourceMessage`
strings are not sufficient for matching translations.
This commit updates `@angular/localize` to use `MessageId`s for translation
matching instead.
Also the run-time translation will now log a warning to the console if a
translation is missing.
BREAKING CHANGE:
Translations (loaded via the `loadTranslations()` function) must now use
`MessageId` for the translation key rather than the previous `SourceMessage`
string.
PR Close#32594
This patch is a final major refactor in styling Angular.
This PR includes three main fixes:
All temporary state taht is persisted between template style/class application
and style/class application in host bindings is now removed.
Removes the styling() and stylingApply() instructions.
Introduces a "direct apply" mode that is used apply prop-based
style/class in the event that there are no map-based bindings as
well as property collisions.
PR Close#32259
PR Close#32591
If an <ng-template> contains a structural directive (for example *ngIf), Ngtsc generates extra template function with 1 template instruction call. When <ng-template> tag also contains i18n attribute on it, we generate i18nStart and i18nEnd instructions around it, which is unnecessary and breaking runtime. This commit adds a logic to make sure we do not generate i18n instructions in case only `template` is present.
PR Close#32623
This patch is a final major refactor in styling Angular.
This PR includes three main fixes:
All temporary state taht is persisted between template style/class application
and style/class application in host bindings is now removed.
Removes the styling() and stylingApply() instructions.
Introduces a "direct apply" mode that is used apply prop-based
style/class in the event that there are no map-based bindings as
well as property collisions.
PR Close#32259
PR Close#32596
This patch is a final major refactor in styling Angular.
This PR includes three main fixes:
All temporary state taht is persisted between template style/class application
and style/class application in host bindings is now removed.
Removes the styling() and stylingApply() instructions.
Introduces a "direct apply" mode that is used apply prop-based
style/class in the event that there are no map-based bindings as
well as property collisions.
PR Close#32259
Replaces the `select` instruction with a new one called `advance`. Instead of the jumping to a specific index, the new instruction goes forward X amount of elements. The advantage of doing this is that it should generate code the compresses better.
PR Close#32516
The `goog.getMsg()` function requires placeholder names to be camelCased.
This is not the case for `$localize`. Here placeholder names need
match what is serialized to translation files.
Specifically such placeholder names keep their casing but have all characters
that are not in `a-z`, `A-Z`, `0-9` and `_` converted to `_`.
PR Close#32509
Prior to this commit, complex expressions (that require additional statements to be generated) were handled incorrectly in case they were used in attributes annotated with i18n flags. The problem was caused by the fact that extra statements were not appended to the temporary vars block, so they were missing in generated code. This commit updated the logic to use the `convertPropertyBinding`, which contains the necessary code to append extra statements. The `convertExpressionBinding` function was removed as it duplicates the `convertPropertyBinding` one (for the most part) and is no longer used.
PR Close#32309
Previously the template compiler would generate the same jsdoc comment
block for `$localize` as for `goog.getMsg()`. But it turns out that
the closure compiler will complain if the `@desc` and `@meaning`
tags are used for non-`getMsg()` calls.
For now we do not generate the comments for `$localize` calls. They are
not being used at the moment.
In the future it would be good to be able to extract the descriptions and
meanings from the `$localize` calls rather than relying upon the `getMsg()`
calls, which we do now. So we need to find a workaround for this constraint.
PR Close#32473
This commit changes the Angular compiler (ivy-only) to generate `$localize`
tagged strings for component templates that use `i18n` attributes.
BREAKING CHANGE
Since `$localize` is a global function, it must be included in any applications
that use i18n. This is achieved by importing the `@angular/localize` package
into an appropriate bundle, where it will be executed before the renderer
needs to call `$localize`. For CLI based projects, this is best done in
the `polyfills.ts` file.
```ts
import '@angular/localize';
```
For non-CLI applications this could be added as a script to the index.html
file or another suitable script file.
PR Close#31609
Reworks the compiler to output the factories for directives, components and pipes under a new static field called `ngFactoryFn`, instead of the usual `factory` property in their respective defs. This should eventually allow us to inject any kind of decorated class (e.g. a pipe).
**Note:** these changes are the first part of the refactor and they don't include injectables. I decided to leave injectables for a follow-up PR, because there's some more cases we need to handle when it comes to their factories. Furthermore, directives, components and pipes make up most of the compiler output tests that need to be refactored and it'll make follow-up PRs easier to review if the tests are cleaned up now.
This is part of the larger refactor for FW-1468.
PR Close#31953
Previously, ngtsc attempted to use the .d.ts schema for HTML elements to
check bindings to DOM properties. However, the TypeScript lib.dom.d.ts
schema does not perfectly align with the Angular DomElementSchemaRegistry,
and these inconsistencies would cause issues in apps. There is also the
concern of supporting both CUSTOM_ELEMENTS_SCHEMA and NO_ERRORS_SCHEMA which
would have been very difficult to do in the existing system.
With this commit, the DomElementSchemaRegistry is employed in ngtsc to check
bindings to the DOM. Previous work on producing template diagnostics is used
to support generation of this different kind of error with the same high
quality of error message.
PR Close#32171
This option makes ngc behave as tsc, and was originally implemented before
ngtsc existed. It was designed so we could build JIT-only versions of
Angular packages to begin testing Ivy early, and is not used at all in our
current setup.
PR Close#32219
In Angular today, the following pattern works:
```typescript
export class BaseDir {
constructor(@Inject(ViewContainerRef) protected vcr: ViewContainerRef) {}
}
@Directive({
selector: '[child]',
})
export class ChildDir extends BaseDir {
// constructor inherited from BaseDir
}
```
A decorated child class can inherit a constructor from an undecorated base
class, so long as the base class has metadata of its own (for JIT mode).
This pattern works regardless of metadata in AOT.
In Angular Ivy, this pattern does not work: without the @Directive
annotation identifying the base class as a directive, information about its
constructor parameters will not be captured by the Ivy compiler. This is a
result of Ivy's locality principle, which is the basis behind a number of
compilation optimizations.
As a solution, @Directive() without a selector will be interpreted as a
"directive base class" annotation. Such a directive cannot be declared in an
NgModule, but can be inherited from. To implement this, a few changes are
made to the ngc compiler:
* the error for a selector-less directive is now generated when an NgModule
declaring it is processed, not when the directive itself is processed.
* selector-less directives are not tracked along with other directives in
the compiler, preventing other errors (like their absence in an NgModule)
from being generated from them.
PR Close#31379
Miško Hevery
ayazhafiz
Sonu Kapoor
Kristiyan Kostadinov
Matias Niemelä
Paul Gschwendtner
Andrew Kushnir
Igor Minar
Greg Magolan
atscott
Andrew Scott
JoostK
Pete Bacon Darwin
Alex Rickabaugh
arturovt
Kara Erickson
mohax
Joey Perrott
Keen Yee Liau
Sergey Nikitin
Alan Agius
Nikita Potapenko
Danny Skoog
Alex Eagle
Keen Yee Liau
Andrius
atscott
Pawel Kozlowski