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
Similar to interpolation, we do not want to completely remove whitespace
nodes that are siblings of an expansion.
For example, the following template
```html
<div>
<strong>items left<strong> {count, plural, =1 {item} other {items}}
</div>
```
was being collapsed to
```html
<div><strong>items left<strong>{count, plural, =1 {item} other {items}}</div>
```
which results in the text looking like
```
items left4
```
instead it should be collapsed to
```html
<div><strong>items left<strong> {count, plural, =1 {item} other {items}}</div>
```
which results in the text looking like
```
items left 4
```
---
**Analysis of the code and manual testing has shown that this does not cause
the generated ids to change, so there is no breaking change here.**
PR Close#31962
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
Template AST nodes for (bound) attributes, variables and references will
now retain a reference to the source span of their value, which allows
for more accurate type check diagnostics.
PR Close#30181
Currently we always generate the `read` parameter for the view and content query instructions, however since most of the time the `read` parameter won't be set, we'll end up generating `null` which adds 5 bytes for each query when minified. These changes make it so that the `read` parameter only gets generated if it has a value.
PR Close#31667
This commit updates the `_clone` function of the `_ApplySourceSpanTransformer` class, where the for-in loop was used, resulting in copying from prototype to own properties, thus consuming more memory. Prior to NodeJS 12 (V8 versions before 7.4) there was an optimization that was improving the situation and since that logic was removed in favor of other optimizations, the situation with memory consumption caused by the for-in loop got worse. This commit adds a check to make sure we copy only own properties over to cloned object.
Closes#31627.
PR Close#31638
When injecting a `ChangeDetectorRef` into a pipe, the expected result is that the ref will be tied to the component in which the pipe is being used. This works for most cases, however when a pipe is used inside a property binding of a component (see test case as an example), the current `TNode` is pointing to component's host so we end up injecting the inner component's view. These changes fix the issue by only looking up the component view of the `TNode` if the `TNode` is a parent.
This PR resolves FW-1419.
PR Close#31438
Currently, template expressions and statements have their location
recorded relative to the HTML element they are in, with no handle to
absolute location in a source file except for a line/column location.
However, the line/column location is also not entirely accurate, as it
points an entire semantic expression, and not necessarily the start of
an expression recorded by the expression parser.
To support record of the source code expressions originate from, add a
new `sourceSpan` field to `ASTWithSource` that records the absolute byte
offset of an expression within a source code.
Implement part 2 of [refactoring template parsing for
stability](https://hackmd.io/@X3ECPVy-RCuVfba-pnvIpw/BkDUxaW84/%2FMA1oxh6jRXqSmZBcLfYdyw?type=book).
PR Close#31391
This commit is the final patch of the ivy styling algorithm refactor.
This patch swaps functionality from the old styling mechanism to the
new refactored code by changing the instruction code the compiler
generates and by pointing the runtime instruction code to the new
styling algorithm.
PR Close#30742
Fixes all TypeScript failures caused by enabling the `--strict`
flag for test source files. We also want to enable the strict
options for tests as the strictness enforcement improves the
overall codehealth, unveiled common issues and additionally it
allows us to enable `strict` in the `tsconfig.json` that is picked
up by IDE's.
PR Close#30993
As part of FW-1265, the `@angular/compiler` package is made compatible
with the TypeScript `--strict` flag. This already unveiled a few bugs,
so the strictness flag seems to help with increasing the overall code health.
Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)
PR Close#30993
Fixes Ivy matching directives against attribute bindings (e.g. `[attr.some-directive]="foo"`). Works by excluding attribute bindings from the attributes array during compilation. This has the added benefit of generating less code.
**Note:** My initial approach to implementing this was to have a different marker for attribute bindings so that they can be ignored when matching directives, however as I was implementing it I realized that the attributes in that array were only used for directive matching (as far as I could tell). I decided to drop the attribute bindings completely, because it results in less generated code.
PR Close#31541
Currently we reuse the same instruction both for regular property bindings and property bindings on the `host`. The only difference between the two is that when it's on the host we shouldn't support inputs. We have an optional parameter called `nativeOnly` which is used to differentiate the two, however since `nativeOnly` is preceeded by another optional parameter (`sanitizer`), we have to generate two extra parameters for each host property bindings every time (e.g. `property('someProp', 'someValue', null, true)`).
These changes add a new instruction called `hostProperty` which avoids the need for the two parameters by removing `nativeOnly` which is always set and it allows us to omit `sanitizer` when it isn't being used.
These changes also remove the `nativeOnly` parameter from the `updateSyntheticHostBinding` instruction, because it's only generated for host elements which means that we can assume that its value will always be `true`.
PR Close#31550
Prior to this fix, the logic to set the right placeholder format for ICUs was a bit incorrect: if there was a nested ICU in one of the root ICU cases, that led to a problem where placeholders in subsequent branches used the wrong ({$placeholder}) format instead of {PLACEHOLDER} one. This commit updates the logic to make sure we properly transform all placeholders even if nested ICUs are present.
PR Close#31516
Since `goog.getMsg` does not process ICUs (post-processing is required via goog.i18n.MessageFormat, https://google.github.io/closure-library/api/goog.i18n.MessageFormat.html) and placeholder format used for ICUs and regular messages inside `goog.getMsg` are different, the current implementation (that assumed the same placeholder format) needs to be updated. This commit updates placeholder format used inside ICUs from `{$placeholder}` to `{PLACEHOLDER}` to better align with Closure. ICU placeholders (that were left as is prior to this commit) are now replaced with actual values in post-processing step (inside `i18nPostprocess`).
PR Close#31459
crisbeto
atscott
Andrew Kushnir
Pete Bacon Darwin
Matias Niemelä
Kara Erickson
Misko Hevery
Pawel Kozlowski
Alex Rickabaugh
atscott
JoostK
Ayaz Hafiz
Paul Gschwendtner