.substr() is deprecated so we replace it with functions which work similarily but aren't deprecated
Signed-off-by: Tobias Speicher <rootcommander@gmail.com>
PR Close#45397
When we have an event listener inside an embedded view, we generate a `restoreView` call which saves the view inside of the LFrame. The problem is that we don't clear it until it gets overwritten which can lead to memory leaks.
These changes rework the generated code in order to generate a `resetView` call which will clear the view from the LFrame.
Fixes#42848.
PR Close#43075
In early versions of Angular, it was sometimes necessary to provide a
`moduleId` to `@Component` metadata, and the common pattern for doing this
was to set `moduleId: module.id`. This relied on the bundler to fill in a
value for `module.id`.
However, due to the superficial similarity between `Component.moduleId` and
`NgModule.id`, many users ended up setting `id: module.id` in their
NgModules. This is an anti-pattern that has a few negative effects,
including preventing the NgModule from tree-shaking properly.
This commit changes the compiler to ignore `id: module.id` in NgModules, and
instead provide a warning which suggests removing the line entirely.
PR Close#45024
Angular contains an NgModule registry, which allows a user to declare
NgModules with string ids and retrieve them via those ids, using the
`getNgModuleById` API.
Previously, we attempted to structure this registration in a clever fashion
to allow for tree-shaking of registered NgModules (that is, those with ids).
This sort of worked due to the accidental alignment of behaviors from the
different tree-shakers involved. However, this trick relies on the
generation of `.ngfactory` files and how they're specifically processed in
various bundling scenarios. We intend to remove `.ngfactory` files, hence
we can no longer rely on them in this way.
The correct solution here is to recognize that `@NgModule({id})` is
inherently declaring a global side-effect, and such classes should not
really be eligible for tree-shaking in the first place. This commit removes
all the old registration machinery, and standardizes on generating a side-
effectful call to `registerNgModuleType` for NgModules that have ids.
There is some risk here that NgModules with unnecessary `id`s may not
tree-shake as a result of this change, whereas they would have in previous
circumstances. The fix here should be to remove the `id` if it's not needed.
Specifying an `id` is a request that the NgModule be retained regardless of
any other references, in case it is later looked up by string id.
PR Close#45024
Jasmine logs a warning when there's a `describe` with no tests. These changes fix one such case in the compiler that happens when the tests are run against Windows.
PR Close#45285
The `@angular/localize` package depends on a version of Babel that is two years
old, so this commit updates to the latest version.
Some changes were made to the linker and compliance tests to account for slight
changes in source maps, along with a few code updates because of changes to
the typings of Babel.
PR Close#44931
In templates with several levels of nested nodes, it's common for several `elementStart`/`elementEnd` instructions to show up in a row which can be optimized away.
These changes add chaining support for `elementStart`, `elementEnd`, `elementContainerStart` and `elementContainerEnd` to shave off some bytes when possible.
PR Close#44994
This commit implements the first phase of standalone components in the Angular
compiler. This mainly includes the scoping rules for standalone components
(`@Component({imports})`).
Significant functionality from the design is _not_ implemented by this PR,
including:
* imports of standalone components into NgModules.
* the provider aspect of standalone components
Future commits will address these issues, as we proceed with the design of
this feature.
PR Close#44812
Refs http://b/214103351.
This happens if a user writes `<span i18n>Message</span>`. This is accepted as an internationalized message, but without a description. JSCompiler will throw an error in this situation because descriptions are generally required. Now, the Angular compiler will generate a suppression annotation so JSCompiler allows the syntax. This will ease an internal migration to JSCompiler-based i18n.
PR Close#44787
Attempting to run as is fails because we have `"type": "module"`. `shelljs` is a CommonJS module however, so we need to do a default import and destructure.
```
$ node packages/compiler-cli/test/compliance/update_all_goldens.js
const {exec} = require('shelljs');
^
ReferenceError: require is not defined in ES module scope, you can use import instead
This file is being treated as an ES module because it has a '.js' file extension and '/home/douglasparker/Source/ng/packages/compiler-cli/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
at file:///home/douglasparker/Source/ng/packages/compiler-cli/test/compliance/update_all_goldens.js:11:16
at ModuleJob.run (node:internal/modules/esm/module_job:183:25)
at async Loader.import (node:internal/modules/esm/loader:178:24)
at async Object.loadESM (node:internal/process/esm_loader:68:5)
at async handleMainPromise (node:internal/modules/run_main:63:12)
```
PR Close#44916
An `ng-template` with an inline template (i.e. has a structural
directive) would previously not get an `undefined` `tagName` because the
logic assumed the element would be `t.Element` or `t.Content` and read
the tag name from the `name` property. For a `t.Template`, this exists
instead on the `t.tagName`. The final result would be an `tagName` of `undefined`
for the parent `t.Template`, causing failures in the indexer downstream.
This `undefined` value is actually expected in the renderer code, even
though the type does not specify this possibility. This change updates
the type of `tagName` to be `string|null` and explicitly handles the
case where there is a structural directive on an `ng-template`. You can
see how the two are differentiated in the compliance code that was
modified in this commit.
PR Close#44788
Previously, if a bad extended diagnostic category was given, it would fail with the expected error as well as an unexpected assertion error:
```
$ ng build -c development
✔ Browser application bundle generation complete.
./src/main.ts - Error: Module build failed (from ./node_modules/@ngtools/webpack/src/ivy/index.js):
Error: Unexpected call to 'assertNever()' with value:
test
at /home/douglasparker/Source/ng-new/node_modules/@ngtools/webpack/src/ivy/loader.js:77:18
at processTicksAndRejections (internal/process/task_queues.js:95:5)
./src/polyfills.ts - Error: Module build failed (from ./node_modules/@ngtools/webpack/src/ivy/index.js):
Error: Unexpected call to 'assertNever()' with value:
test
at /home/douglasparker/Source/ng-new/node_modules/@ngtools/webpack/src/ivy/loader.js:77:18
at processTicksAndRejections (internal/process/task_queues.js:95:5)
Error: error NG4004: Angular compiler option "extendedDiagnostics.checks['invalidBananaInBox']" has an unknown diagnostic category: "test".
Allowed diagnostic categories are:
warning
error
suppress
```
The assertion comes from `ExtendedTemplateCheckerImpl`, which expects a well-formed configuration, yet the compiler would construct it even when errors were found. This commit skips constructing and running extended diagnostics if the configuration had errors, which should avoid triggering these assertion errors.
I'm unfortunately not able to actually test this change. The test passes even before the fix because the `ngc` binary and end-to-end tests [don't request diagnostics unless the configuration is considered valid](ed21f5c753/packages/compiler-cli/src/perform_compile.ts (L292-L293)). See [Slack](https://angular-team.slack.com/archives/C4WHZQMRA/p1642641305003800) for more details.
PR Close#44778
Refs #42966.
Extended diagnostics provide additional analysis about Angular templates by emitting warnings for specific patterns known to be error prone or cause developer confusion. Currently, there are two such diagnostics which are enabled by default:
* `invalidBananaInBox` emits a warning if a user writes a two-way binding backwards like `([foo])="bar"`, when they actually wanted `[(foo)]="bar"`.
* `nullishCoalescingNotNullable` emits a warning if a binding attempts to perform nullish coalescing (`??`) on a type which does not include `null` or `undefined`, such as `{{ foo ?? 'bar' }}` where `foo` is defined as `string` instead of `string | null`.
These diagnostics are enabled as warnings by default, but this can be configured in the `tsconfig.json` like so:
```jsonc
{
"angularCompilerOptions": {
"extendedDiagnostics": {
// The categories to use for specific diagnostics.
"checks": {
// Maps check name to its category.
"invalidBananaInBox": "suppress"
},
// The category to use for any diagnostics not listed in `checks` above.
"defaultCategory": "error"
}
}
}
```
Allowed categories for a diagnostic are `warning` (default), `error`, or `suppress`. `warning` emits the diagnostic but allows the compilation to succeed, `error` *will* fail the compilation, while `suppress` will ignore the diagnostic altogether.
The initial release has two diagnostics, and we are hoping to expand this longer term to add more diagnostics and provide additional insight into Angular templates to detect and surface developer mistakes *before* hours of debugging are wasted.
PR Close#44712
Refs #42966.
This validates the `tsconfig.json` options for extended template diagnostics. It verifies:
* `strictTemplates` must be enabled if `extendedDiagnostics` have any explicit configuration.
* `extendedDiagnostics.defaultCategory` must be a valid `DiagnosticCategoryLabel`.
* `extendedDiagnostics.checks` keys must all be real diagnostics.
* `extendedDiagnostics.checks` values must all be valid `DiagnosticCategoryLabel`s.
These include new error codes, each of which prints out the exact property that was the issue and what the available options are to fix them.
It does disallow the config:
```json
{
"angularCompilerOptions": {
"strictTemplates": false,
"extendedDiagnostics": {
"defaultCategory": "suppress"
}
}
}
```
Such a configuration is technically valid and could be executed, but will be rejected by this verification logic. There isn't much reason to ever do this, since users could just drop `extendedDiagnostics` altogether and get the intended effect. This is unlikely to be a significant issue for users, so it is considered invalid for now to keep the implementation simple.
PR Close#44391
Refs #42966.
The `defaultCategory` option is used for any extended template diagnostics which do not have any specific category specified for them. It defaults to `warning`, since that is the most common behavior expected for users. This provides an easy way for users to promote all diagnostics to errors or suppress all diagnostics.
PR Close#44391
Refs #42966.
This updates `TemplateContext` to include a new `makeTemplateDiagnostic()` function which automatically uses the correct diagnostic category for that check. This makes sure that each diagnostic is emitted with the correct category. It also implicitly passes some known values like `component` and `code` to make the extended template diagnostics a little simpler. Diagnostics which are suppressed are never instantiated at all, which acts as a slight performance optimization since any emitted diagnostics would be ignored anyways.
Unfortunately, diagnostics still have access to `ctx.templateTypeChecker.makeTemplateDiagnostic()` to manually create diagnostics with a different category. Both banana in box and nullish coalescing checks include tests to make sure they respect a manually configured category. This convention should hopefully give a reasonable certainty that new diagnostics will use the correct reporting function, even if that is not strictly enforced.
PR Close#44391
Refs #42966.
This moves extended template check factory invocations into the checker itself, where it can provide a more consistent API contract. Factories are called with compiler options and may return a `TemplateCheck` or `undefined` if the current options do not support that check. This allows `nullishCoalescingNotNullable` to disable itself when `strictNullChecks` is disabled without throwing errors. This gives extended template diagnostics a stronger abstraction model to define their behavior with.
PR Close#44391
In certain scenarios, the compiler may have crashed with an
`Unable to write a reference` error which would be particularly hard
to diagnose. One of the primary reasons for this failure is when the
`rootDir` option is configured---typically the case for libraries---
and a source file is imported using a relative import from an external
entry-point. This would normally report TS6059 for the invalid relative
import, but the crash prevents this error from being surfaced.
This commit refactors the reference emit logic to result in an explicit
`Failure` state with a reason as to why the failure occurred. This state
is then used to report a `FatalDiagnosticException`, preventing a hard
crash.
Closes#44414
PR Close#44587
To make our test output i.e. devmode output more aligned
with what we produce in the NPM packages, or to be more
aligned with what Angular applications will usually consume,
the devmode output is switched from ES5 to ES2015.
Additionally various tsconfigs (outside of Bazel) have been
updated to match with the other parts of the build. The rules
are:
ES2015 for test configurations, ES2020 for actual code that will
end up being shipped (this includes the IDE-only tsconfigs).
PR Close#44505
This commit finishes the removal of View Engine from the codebase, deleting
those pieces of @angular/compiler which were only used for VE.
Co-Authored-By: JoostK <joost.koehoorn@gmail.com>
PR Close#44368
This commit does a first-pass removal of the View Engine infrastructure
in compiler-cli. A more in-depth cleanup is necessary and large parts
of the View Engine compiler infrastructure remain within
`@angular/compiler`, this is just a first cleanup step.
PR Close#44269
As a preparation for the removal of the ViewEngine parts in
`compiler-cli`, this commit moves the version number helper functions
up one level such that the whole `diagnostics` subfolder can be removed.
PR Close#44269
Adds support for TypeScript 4.5. Includes the following changes:
* Bumping the package versions.
* Fixing a few calls to `createExportSpecifier` and `createImportSpecifier` that require an extra parameter.
* Adding some missing methods to the TS compiler hosts.
* Fixing an issue in the TS mocks for the ngcc tests where a regex was too agressive and was trying to match a path like `/node_modules/@typescript/lib-es5`.
* Accounting for type-only import specifiers when reporting DI errors (see #43620).
Fixes#43620.
PR Close#44164
The downlevel decorator transform (commonly used in the CLI and other
tooling of the ecosystem for enabling JIT ES2015+), is currently
incorrectly dealing with nested classes.
The transform will accidentally visit nested classes (in a constructor)
multiple times and generate duplicated instances of the `ctorParameters`
fields. This does not sound like an issue at first, but the duplicated
`ctorParameters` fields will miss significant type information that has
been elided by the first visit, resulting in generated code like the
following:
```js
let MyClass = /* @__PURE__ */ __name(class MyClass extends UpgradeNg1ComponentAdapter {
constructor(scope, injector3, elementRef) {
}
}, "MyClass");
MyClass.ctorParameters = () => [
{ type: void 0, decorators: [{ type: Inject, args: [$SCOPE] }] },
{ type: Injector },
{ type: ElementRef }
];
MyClass.ctorParameters = () => [
{ type: void 0 }, // <---- NOTE!
{ type: Injector },
{ type: ElementRef }
];
```
PR Close#44281
Now that the core package has been cleaned up to no longer contain Ivy
switch code, the transform to switch the `PRE_R3` markers to become
`POST_R3` is deleted as well.
PR Close#43891
This commit removes most tests that were designated as only covering View
Engine code. It also removes tag filters from CI and local commands to run
tests.
In a few cases (such as with the packages/compiler tests), this tag was
improperly applied, and certain test cases have been added back running in
Ivy mode.
This commit also empties `@angular/compiler/testing` as it is no longer
necessary (this is safe since compiler packages are not public API). It can
be deleted in the future.
PR Close#43884
When a partially compiled component or directive is "linked" in JIT mode, the body
of its declaration is evaluated by the JavaScript runtime. If a class is referenced
in a query (e.g. `ViewQuery` or `ContentQuery`) but its definition is later in the
file, then the reference must be wrapped in a `forwardRef()` call.
Previously, query predicates were not wrapped correctly in partial declarations
causing the code to crash at runtime. In AOT mode, this code is never evaluated
but instead transformed as part of the build, so this bug did not become apparent
until Angular Material started running JIT mode tests on its distributable output.
This change fixes this problem by noting when queries are wrapped in `forwardRef()`
calls and ensuring that this gets passed through to partial compilation declarations
and then suitably stripped during linking.
See https://github.com/angular/components/pull/23882 and https://github.com/angular/components/issues/23907
PR Close#44113
Currently the TS version checking function interprets a version like `1.2.3-rc.5` as `1.2.NaN` which would allow it to bypass the version checking altogether.
These changes add a little bit more logic to ensure that such versions are handled correctly. There's also an error if we don't manage to parse the version string.
Also it seemed like we never actually ran the version check unit tests, because they didn't have a test target.
PR Close#44109
When a safe method call such as `person?.getName()` is used, the
compiler would generate invalid code if the argument list also contained
a safe method call. For example, the following code:
```
person?.getName(config?.get('title').enabled)
```
would generate
```
let tmp;
ctx.person == null ? null : ctx.person.getName((tmp = tmp) == null ?
null : tmp.enabled)
```
Notice how the call to `config.get('title')` has completely disappeared,
with `(tmp = tmp)` having taken its place.
The issue occurred due to how the argument list would be converted
from expression AST to output AST twice. First, the outer safe method
call would first convert its arguments list. This resulted in a
temporary being allocated for `config.get('title')`, which was stored in
the internal `_resultMap`. Only after the argument list has been
converted would the outer safe method call realize that it should be
guarded by a safe access of `person`, entering the `convertSafeAccess`
procedure to convert itself. This would convert the argument list once
again, but this time the `_resultMap` would already contain the
temporary `tmp` for `config?.get('title')`. Consequently, the safe
method in the argument list would be emitted as `tmp`.
This commit fixes the issue by ensuring that nodes are only converted
once.
Closes#44069
PR Close#44088
This commit re-enables the `perform_watch` test target and updates the
test to run with the Ivy compiler.
Additionally, this target was switched over to use Angular v12 packages
as input to the test, to allow the ViewEngine tests to continue working
with v13 packages which are Ivy-only. This commit reverts those changes
now that View Engine tests are disabled, as it's desirable to test
against local artifacts that are build within the monorepo instead of
depending on NPM packages.
PR Close#43893
Using the tag "view-engine-only" better describes the expected usage of bazel targets with the test. They can
only be run with view engine.
PR Close#43862
Since building with ViewEngine is not longer desired on CI, removing the ivy vs non-ivy testing yarn scripts
is done, informing developers to instead use `yarn test` as all tests should be run using the Ivy complier.
PR Close#43862
This reverts commit bba0a87055.
The reason for rollback: this change is breaking some targets in Google's codebase when there is no attribute value is displayed (attr.aria-label) when translated.
PR Close#43882
While fully dynamic bound properties (and attributes) cannot be marked for localization, properties that only contain interpolation can.
This commit ensure that attribute bindings that only contain interpolation can also be marked for localization.
Closes#43260
PR Close#43815
When extracting i18n messages from templates, ICU messages are split out from the
message that contains them. This can make it difficult in the translation files to match up
the two messages, especially if the ICU is reused in multiple placeholders.
This commit builds on top of the previous one to expose the message ID of ICU messages
from the ICU placeholders as additional metadata in the `$localize` tagged strings.
Now the metablock following any placeholder can also contain the associated ID
delimited from the placeholder name by `@@`.
Fixes#17506
PR Close#43534
Refs #42966.
Previously, a build when emitted one warning and no errors would fail with a non-zero exit code. This is not what users would expect, but had not been an issue before since the compiler did not actually emit any warnings. With upcoming extended template diagnostics and other warnings, this is now a case that needs to be supported. Warnings are printed to `stderr` as before, but `ngc` now exits with code `0` and the build is considered successful.
Implemented this by adding a new `expectedExitCode` parameter to `driveDiagnostics()` which asserts against the real exit code. Most importantly, it does not **require** the the build to pass since any exit code can be given, so it is up to the test to assert this as well as many messages printed to make sure they are acceptable. This is useful for testing warnings and ensuring the build still passes.
PR Close#43673
Similar to the other private entry-points we have added for localize,
bazel or the migrations, we should expose the tooling code through
a dedicated private export. This will make the compiler-cli exports
more consistent and it will become easier for the CLI to export
necessary code.
PR Close#43431
Removes the remaining usages of dynamic require statements in the
package output. Since we declare all shipped packages as strict ESM,
we cannot use dynamic require statements anymore. This commit switches
these usages to actual `import` statements.
Note: Tsickle continues to remain an optional dependency since bundling
does not work with its UMD package output. Also tsickle is rarely used by
consumers, if at all, so bundling does not really provide any significant
value. To continue keeping tsickle optional (since it's still needed by the
`annotateForClosureCompiler` option which is also respected in ngtsc), we
pass-through a tsickle instance as a parameter to `main`. This allows us to
keep the compile functions synchronous without having to refactor the majority
of the watch compilation code, and majority of tests for ngc, ngtsc.
Consumers (like the `ngc` bin entry-point) can then load tsickle based on their
module format. e.g. tsickle can be imported through `require` to keep everything
sync, but in ESM, the dynamic import can be used beforehand to pass `tsickle` to
the `main` function. We can revisit this in the future but for now this does the
trick without exceeding the scope of this commit..
PR Close#43431
As outlined in the previous commit which enabled the `esModuleInterop`
TypeScript compiler option, we need to update all namespace imports
for `typescript` to default imports. This is needed to allow for
TypeScript to be imported at runtime from an ES module.
Similar changes are needed for modules like `semver` where the types incorrectly
suggest named exports that will not exist at runtime when imported from ESM.
This commit refactors all imports to match with the lint rule we have
configured in the previous commit. See the previous commit for more
details on why certain imports have been changed.
A special case are the imports to `@babel/core` and `@babel/types`. For
these a special interop is needed as both default imports, or named
imports break the other module format. e.g default imports would work
well for ESM, but it breaks for CJS. For CJS, the named imports would
only work, but in ESM, only the default export exist. We work around
this for now until the devmode is using ESM as well (which would be
consistent with prodmode and gives us more valuable test results). More
details on the interop can be found in the `babel_core.ts` files (two
interops are needed for both localize/or the compiler-cli).
PR Close#43431
The view engine language-service tests currently rely on the `npm_package` output
that is built locally. They rely on the package output mostly for
compiling test scenarios (with dependencies on e.g. forms), and for
the testing the metadata extraction (testing proper suggestions for VE).
The reliance on these packages becomes problematic with the new Angular
Package Format v13 where no metadata files are shipped. To continue
being able to test View Engine language-service compatibility, we will
use the v12.x framework packages for some of the test scenarios.
PR Close#43431
The View Engine ngc tests currently rely on the `npm_package` output
that is built locally. This becomes problematic with the new Angular
Package Format v13 where no metadata files are shipped. To continue
being able to test View Engine compilation, we will use the v12.x
framework packages for running the View Engine test.
Note: This means that we no longer test metadata extraction directly
for our framework packages, but given that any change to View Engine
will still land in patch, where the VE packaging still occurs, we should
be covered here.
PR Close#43431
Now that `Route.loadChildren` no longer accepts a string, there is no
need for tooling to find all string-based `loadChildren` to setup lazy
imports for them. As a result, the `listLazyRoutes` operation that
enumerates all string-based `loadChildren` occurrences is no longer
needed and is therefore removed from the compiler.
The `listLazyRoutes` API remains on the `Program` interface to avoid
breaking external tools that may be using this method, but those tools
should ultimately move away from using this API.
PR Close#43591
This commit removes the ability to configure lazy routes using a string
for `loadChildren`, together with the supporting classes to load an
`NgModuleFactory` at runtime.
BREAKING CHANGE:
It is no longer possible to use `Route.loadChildren` using a string
value. The following supporting classes were removed from
`@angular/core`:
- `NgModuleFactoryLoader`
- `SystemJsNgModuleFactoryLoader`
The `@angular/router` package no longer exports these symbols:
- `SpyNgModuleFactoryLoader`
- `DeprecatedLoadChildren`
The signature of the `setupTestingRouter` function from
`@angular/core/testing` has been changed to drop its `NgModuleFactoryLoader`
parameter, as an argument for that parameter can no longer be created.
PR Close#43591
When specifying the `deps` array in the `@Injectable` decorator to
inject dependencies into the injectable's factory function, it should
be possible to use an array literal to configure how the dependency
should be resolved by the DI system.
For example, the following example is allowed:
```ts
@Injectable({
providedIn: 'root',
useFactory: a => new AppService(a),
deps: [[new Optional(), 'a']],
})
export class AppService {
constructor(a) {}
}
```
Here, the `'a'` string token should be injected as optional. However,
the AOT compiler incorrectly used the array literal itself as injection
token, resulting in a failure at runtime. Only if the token were to be
provided using `[new Optional(), new Inject('a')]` would it work
correctly.
This commit fixes the issue by using the last non-decorator in the
array literal as the token value, instead of the array literal itself.
Note that this is a loose interpretation of array literals: if a token
is omitted from the array literal then the array literal itself is used
as token, but any decorator such as `new Optional()` would still have
been applied. When there's multiple tokens in the list then only the
last one will be used as actual token, any prior tokens are silently
ignored. This behavior mirrors the JIT interpretation so is kept as is
for now, but may benefit from some stricter checking and better error
reporting in the future.
Fixes#42987
PR Close#43226
Adds support for TypeScript 4.4. High-level overview of the changes made in this PR:
* Bumps the various packages to `typescript@4.4.2` and `tslib@2.3.0`.
* The `useUnknownInCatchVariables` compiler option has been disabled so that we don't have to cast error objects explicitly everywhere.
* TS now passes in a third argument to the `__spreadArray` call inside child class constructors. I had to update a couple of places in the runtime and ngcc to be able to pick up the calls correctly.
* TS now generates code like `(0, foo)(arg1, arg2)` for imported function calls. I had to update a few of our tests to account for it. See https://github.com/microsoft/TypeScript/pull/44624.
* Our `ngtsc` test setup calls the private `matchFiles` function from TS. I had to update our usage, because a new parameter was added.
* There was one place where we were setting the readonly `hasTrailingComma` property. I updated the usage to pass in the value when constructing the object instead.
* Some browser types were updated which meant that I had to resolve some trivial type errors.
* The downlevel decorators tranform was running into an issue where the Closure synthetic comments were being emitted twice. I've worked around it by recreating the class declaration node instead of cloning it.
PR Close#43281
Currently the compiler has three different classes to represent a "call to something":
1. `MethodCall` - `foo.bar()`
2. `SafeMethodCall` - `foo?.bar()`.
3. `FunctionCall` - Any calls that don't fit into the first two classes. E.g. `foo.bar()()`.
There are a few problems with this approach:
1. It is inconistent with the TypeScript AST which only has one node: `CallExpression`.
2. It means that we have to maintain more code, because the various parts of the compiler need to know about three node types.
3. It doesn't allow us to easily implement some new JS features like safe calls (e.g. `foo.bar?.())`).
These changes rework the compiler so that it produces only one node: `Call`. The new node behaves similarly to the TypeScript `CallExpression` whose `receiver` can be any expression.
There was a similar situation in the output AST where we had an `InvokeMethodExpression` and `InvokeFunctionExpression`. I've combined both of them into `InvokeFunctionExpression`.
PR Close#42882
Previously, the way templates were tokenized meant that we lost information
about the location of interpolations if the template contained encoded HTML
entities. This meant that the mapping back to the source interpolated strings
could be offset incorrectly.
Also, the source-span assigned to an i18n message did not include leading
whitespace. This confused the output source-mappings so that the first text
nodes of the message stopped at the first non-whitespace character.
This commit makes use of the previous refactorings, where more fine grain
information was provided in text tokens, to enable the parser to identify
the location of the interpolations in the original source more accurately.
Fixes#41034
PR Close#43132
The lexer now splits encoded entity tokens out from text and attribute value tokens.
Previously encoded entities would be decoded and the decoded value would be
included as part of the text token of the surrounding text. Now the entities
have their own tokens. There are two scenarios: text and attribute values.
Previously the contents of `<div>Hello & goodbye</div>` would be a single
TEXT token. Now it will be three tokens:
```
TEXT: "Hello "
ENCODED_ENTITY: "&", "&"
TEXT: " goodbye"
```
Previously the attribute value in `<div title="Hello & goodbye">` would be
a single text token. Now it will be three tokens:
```
ATTR_VALUE_TEXT: "Hello "
ENCODED_ENTITY: "&", "&"
ATTR_VALUE_TEXT: " goodbye"
```
- ENCODED_ENTITY tokens have two parts: "decoded" and "encoded".
- ENCODED_ENTITY tokens are always preceded and followed by either TEXT tokens
or ATTR_VALUE_TEXT tokens, depending upon the context, even if they represent
an empty string.
The HTML parser has been modified to recombine these tokens to allow this
refactoring to have limited effect in this commit. Further refactorings
to use these new tokens will follow in subsequent commits.
PR Close#43132
Previously, the decorator transformer was annotating the synthesized properties with TS type annotations. However, because it ran after the JSDoc transformer, the TS types were just dropped from the emitted JS. Attempting to move the decorator transformer before the JSDoc transformer causes tsickle crashes because synthetic AST fragments are not attached to a SourceFile node.
PR Close#43021
Return `TemplateDiagnostic` instead of `ts.Diagnostic` when getting the
extended template diagnostics. This makes the integration with the
language service easier. This also fixes the error code and now uses the
`ngErrorCode` for extended template diagnostics.
Refs #42966
PR Close#43134
This commit adds extended template diagnostics end-to-end tests, to make
sure the diagnostics are generated correctly. Template checks are
already tested with unit tests.
Refs #42966
PR Close#43107
Previously with View Engine output, the `enableResourceInlining` option
could be set to inline external templates and styles (also for the
resulting `.metadata.json` files). We want to do the same for the Ivy
compilation pipeline (regardless of the compilation mode). The full
compilation definitions, and partial declarations currently already
inline resources in a way that no external requests need to be made.
Although there is one exception currently. These are the calls for
setting class metadata (for testbed overrides). This commit updates
the set class metadata calls (for both partial and full compilation)
to always inline resources. This means that libraries do not need
to start shipping external styles/templates just for the
`setClassMetadata` calls.
Note: Only doing this for partial compilation has been considered, but
it seems like it would be simpler implementation-wise to do this for
full compilation as well. Given the external resources are already
inlined (through their `ecmp` definitions), it seems acceptable (or
even more aligned) to do the same for the set class metadata calls.
PR Close#43178
The compliance tests can check source-map segments against expectations
encoded into the expectation files. Previously, the encoding of the expected
segment was only delimited by whitespace, but this made it difficult to identify
segments that started or ended with whitespace.
Now these segment expectations are wrapped in double-quotes which makes
it easier to read and understand the expectation files.
PR Close#43129
Export `getSourceCodeForDiagnostic` from `ngtsc/testing` to make it
available for other packages. This will help confirm that the source
code is correct in other tests.
Refs #42966
PR Close#42984
Previously, the way templates were tokenized meant that we lost information
about the location of interpolations if the template contained encoded HTML
entities. This meant that the mapping back to the source interpolated strings
could be offset incorrectly.
Also, the source-span assigned to an i18n message did not include leading
whitespace. This confused the output source-mappings so that the first text
nodes of the message stopped at the first non-whitespace character.
This commit makes use of the previous refactorings, where more fine grain
information was provided in text tokens, to enable the parser to identify
the location of the interpolations in the original source more accurately.
Fixes#41034
PR Close#42062
The compliance tests can check source-map segments against expectations
encoded into the expectation files. Previously, the encoding of the expected
segment was only delimited by whitespace, but this made it difficult to identify
segments that started or ended with whitespace.
Now these segment expectations are wrapped in double-quotes which makes
it easier to read and understand the expectation files.
PR Close#42062
The static interpreter assumed that a foreign function expression would
have to be imported from the absolute module specifier that was used for
the foreign function itself. This assumption does not hold for the
`forwardRef` foreign function resolver, as that extracts the resolved
expression from the function's argument, which is not behind the
absolute module import of the `forwardRef` function.
The prior behavior has worked for the typical usage of `forwardRef`,
when it is contained within the same source file as where the static
evaluation started. In that case, the resulting reference would
incorrectly have an absolute module guess of `@angular/core`, but the
local identifier emit strategy was capable of emitting the reference
without generating an import using the absolute module guess.
In the scenario where the static interpreter would first have to follow
a reference to a different source that contained the `forwardRef` would
the compilation fail. In that case, there is no local identifier
available such that the absolute module emitter would try to locate the
imported symbol from `@angular/core`. which fails as the symbol is not
exported from there.
This commit fixes the issue by checking whether a foreign expression
occurs in the same source file as the call expression. If it does, then
the absolute module specifier that was used to resolve the call
expression is ignored.
Fixes#42865
PR Close#42887
In #41995 the type of `TrackByFunction` was changed such that the
declaration of a `trackBy` function did not cause the item type to be
widened to the `trackBy`'s item type, which may be a supertype of the
iterated type. This has introduced situations where the template type
checker is now reporting errors for cases where a `trackBy` function is
no longer assignable to `TrackByFunction`.
This commit fixes the error by also including the item type `T` in
addition to the constrained type parameter `U`, allowing TypeScript to
infer an appropriate `T`.
Fixes#42609
PR Close#42692
Currently unless a listener inside of an embedded view tries to reference something from the parent view, or if the reference is a local ref, we don't generate the view restoration instructions and we allow for the value to be picked up from the context object in the function parameters. The problem is that the listener is only run during creation mode and the context object may have been swapped out afterwards.
These changes fix the issue by always generating the view restoration instructions for listeners inside templates.
Fixes#42698.
PR Close#42755
Updates the Bazel NodeJS rules to v4.0.0-beta.0. This is necessary
so that the Angular components repo can update, and it's generally
good to stay as up-to-date as possible with the Bazel rules as it's
easy to fall behind, and updating early allows us to discover issues
affecting our tooling earlier (where they are easier to address due to
e.g. potential breaking change policy).
PR Close#42760
Source files that contain directives or components that need an inline
type constructor or inline template type-check block would always be
considered as affected in incremental rebuilds. The inline operations
cause the source file to be updated in the TypeScript program that is
created for template type-checking, which becomes the reuse program
in a subsequent incremental rebuild.
In an incremental rebuild, the source files from the new user program
are compared to those from the reuse program. The updated source files
are not the same as the original source file from the user program, so
the incremental engine would mark the file which needed inline
operations as affected. This prevents incremental reuse for these files,
causing sub-optimal rebuild performance.
This commit attaches the original source file for source files that have
been updated with inline operations, such that the incremental engine
is able to compare source files using the original source file.
Fixes#42543
PR Close#42759
As of ES2021, JavaScript allows using underscores as separators inside numbers, in order to make them more readable (e.g. `1_000_000` vs `1000000`). TypeScript has had support for separators for a while so these changes expand the template parser to handle them as well.
PR Close#42672
Adds support for shorthand property declarations inside Angular templates. E.g. doing `{foo, bar}` instead of `{foo: foo, bar: bar}`.
Fixes#10277.
PR Close#42421
If an implcit receiver is accessed in a listener inside of an `ng-template`, we generate some extra code in order to ensure that we're assigning to the correct object. The problem is that the logic wasn't covering keyed writes which caused it to write to the wrong object and throw an assertion error at runtime.
These changes expand the logic to cover keyed writes.
Fixes#41267.
PR Close#42603
xi18n is the operation of extracting i18n messages from templates in the
compilation. Previously, only View Engine was able to perform xi18n. This
commit implements xi18n in the Ivy compiler, and a copy of the View Engine
test for Ivy verifies that the results are identical.
PR Close#42485
This is based on a discussion we had a few weeks ago. Currently if a component uses `ViewEncapsulation.ShadowDom` and its selector doesn't meet the requirements for a custom element tag name, a vague error will be thrown at runtime saying something like "Element does not support attachShadowRoot".
These changes add a new diagnostic to the compiler that validates the component selector and gives a better error message during compilation.
PR Close#42245
Updates the compiler-cli compliance goldens. The golden updates are
required due to a regression in TypeScript 4.3 that causes the emitter
to not incorrectly preserve lines when emitting a node list.
This can be reverted once https://github.com/microsoft/TypeScript/pull/44070
is available.
PR Close#42022
Switches the repository to TypeScript 4.3 and the latest
version of tslib. This involves updating the peer dependency
ranges on `typescript` for the compiler CLI and for the Bazel
package. Tests for new TypeScript features have been added to
ensure compatibility with Angular's ngtsc compiler.
PR Close#42022
Currently we support safe property (`a?.b`) and method (`a?.b()`) accesses, but we don't handle safe keyed reads (`a?.[0]`) which is inconsistent. These changes expand the compiler in order to support safe key read expressions as well.
PR Close#41911
The `ModuleWithProviders` type has required a generic type since Angular 10,
so it is no longer necessary for the compiler to transform usages of the
`ModuleWithProviders` type without the generic type, as that should have
been reported as a compile error. This commit removes the detection logic
from ngtsc.
PR Close#41996
When a `trackBy` function is used that accepts a supertype of the iterated
array's type, the loop variable would undesirably be inferred as the supertype
instead of the array's item type. This commit adds an inferred type parameter
to `TrackByFunction` to allow an extra degree of freedom, enabling the
loop value to be inferred as the most narrow type.
Fixes#40125
PR Close#41995
The ngtsc test targets have fake declarations files for `@angular/core`
and `@angular/common` and the template type checking tests can leverage
the fake common declarations instead of declaring its own types.
PR Close#41995
Type-only imports are known to be elided by TypeScript, so the compiler
can be certain that such imports do not contribute to potential import
cycles. As such, type-only imports are no longer considered during cycle
analysis.
Regular import statements that would eventually be fully elided by
TypeScript during emit if none of the imported symbols are used in a
value position continue to be included in the cycle analysis, as the
cycle analyzer is unaware of these elision opportunities. Only explicit
`import type` statements are excluded.
PR Close#42453
The compiler flag `compileNonExportedClasses` allows the Angular compiler to
process classes which are not exported at the top level of a source file.
This is often used to allow for AOT compilation of test classes inside
`it()` test blocks, for example.
Previously, the compiler would identify exported classes by looking for an
`export` modifier on the class declaration itself. This works for the
trivial case, but fails for indirectly exported classes:
```typescript
// Component is declared unexported.
@Component({...})
class FooCmp {...}
// Indirect export of FooCmp
export {FooCmp};
```
This is not an immediate problem for most application builds, since the
default value for `compileNonExportedClasses` is `true` and therefore such
classes get compiled regardless.
However, in the Angular Language Service now, `compileNonExportedClasses` is
forcibly overridden to `false`. That's because the tsconfig used by the IDE
and Language Service is often far broader than the application build's
configuration, and pulls in test files that can contain unexported classes
not designed with AOT compilation in mind.
Therefore, the Language Service has trouble working with such structures.
In this commit, the `ReflectionHost` gains a new API for detecting whether a
class is exported. The implementation of this method now not only considers
the `export` modifier, but also scans the `ts.SourceFile` for indirect
exports like the example above. This ensures the above case will be
processed directly in the Language Service.
This new operation is cached using an expando symbol on the `ts.SourceFile`,
ensuring good performance even when scanning large source files with lots of
exports (e.g. a FESM file under `ngcc`).
Fixes#42184.
PR Close#42207
When `checkTypeOfPipes` is set to `false`, our TCB currently generates
the a statement like the following when pipes appear in the template:
`(_pipe1 as any).transform(args)`
This did enable us to get _some_ information from the Language Service
about pipes in this case because we still had access to the pipe
instance. However, because it is immediately cast to `any`, we cannot
get type information about the transform access. That means actions like "go to
definition", "find references", "quick info", etc. will return
incomplete information or fail altogether.
Instead, this commit changes the TCB to generate `(_pipe1.transform as any)(args)`.
This gives us the ability to get complete information for the LS
operations listed above.
PR Close#40523
This commit changes the reference emitters in the Ivy compiler to prefer
non-aliased exports if they exist. This avoids selecting "private
exports" that may not be stable, e.g. the reexports that have been added
by the View Engine compiler. Such reexports are not stable and are
therefore not suitable to be emitted into partial compilations, as the
output of partial compilations should only reference stable symbols
from upstream libraries.
An alternative solution has been considered where ViewEngine-generated
exports would gain a certain prefix, such that the Ivy compiler could
just exclude those exports (see #41443). However, that solution would
be insufficient in case a library is built using partial compilation and
while depending itself on a VE-compiled library from earlier versions of
Angular, where the magic prefix would be missing. For such libraries,
ngcc would have generated reexports using the declared name if not already
present so this change does result in choosing the correct export.
Because ngcc always generates reexports using the declared name even if
an aliased export is present, this change causes those ngcc-generated
exports to be chosen in downstream libraries using partial compilation.
This is unfortunate as it means that the declared names become
effectively public even if the library author was intentionally
exporting it using an alias. This commit does not address this problem;
it is expected that this should not result in widespread issues across
the library ecosystem.
Fixes#41277
PR Close#41866
We have a check that determines whether to generate property binding instructions for an `ng-template`. The check looks at whether the tag name is exactly `ng-template`, but the problem is that if the tag is placed in a non-HTML namespace (e.g. `svg`), the tag name will actually be `:namespace:ng-template` and the check will fail.
These changes resolve the issue by looking at the tag name without the namespace.
Fixes#41308.
PR Close#41669
Currently if a component defines a template inline, but not through a
string literal, the partial compilation references the template expression
as is. This is problematic because the component declaration can no longer
be processed by the linker later as there is no static interpretation. e.g.
```js
const myTemplate = `...`;
TestCmp.ɵcmp = i0.ɵɵngDeclareComponent({
version: "0.0.0-PLACEHOLDER",
type: TestCmp,
selector: "test-cmp",
ngImport: i0,
template: myTemplate,
isInline: true
});
```
To fix this, we use the the resolved template in such cases so that
the linker can process the template/component declaration as expected.
PR Close#41583
This is follow-up from #41437 and it reduces the amount of code we generate for safe property accesses (`a?.b`) and nullish coalescing (`a ?? b`) by:
1. Reusing variables in nested nullish coalescing expressions.
2. Not initializing temporary variables to `null`. The way our code is generated means that the value will always be overwritten before we compare against it so the initializer didn't really matter.
Fixes#41491.
PR Close#41563
Previously, it was not possible to block a partial-linker from trying to
process a declaration that was defined in a newer version of Angular than
that of the partial-linker. For example, if a partial-linker was published as
part of version 12.0.0, there was no way for a partially-compiled declaration
compiled via version 13.0.0 to tell the 12.0.0 linker that it would be invalid
to attempt to process it.
This commit adds a new `minVersion` property to partial-declarations, which is
interpreted as the "minimum partial-linker version" that can process this
declaration. When selecting a partial-linker for such a declaration, the known
linker version ranges are checked to find the most recent linker whose version
range has an overlap with the interpreted declaration range.
This approach allows us to set a minimum version for a declaration, which
can inform an old partial-linker that will it not be able to accurately
process the declaration.
Note that any pre-release part to versions are ignored in this selection
process.
The file-linker can be configured, via the `unknownDeclarationVersionHandling`
property of `LinkerOptions`, to handle such a situation in one of three ways:
- `error` - the version mismatch is a fatal error
- `warn` - a warning is sent to the logger but the most recent partial-linker
will attempt to process the declaration anyway.
- `ignore` - the most recent partial-linker will, silently, attempt to process
the declaration.
The default is to throw an error.
Closes#41497
PR Close#41578
There were three options being made available to users of the linker:
- ` enableI18nLegacyMessageIdFormat`
- `i18nNormalizeLineEndingsInICUs`
- ` i18nUseExternalIds`
None of these should actually be configurable at linking time
because partially-linked libraries have tighter restrictions on
what i18n options can be used.
This commit removes those options from the `LinkerOptions` interface.
It was considered to add a check for backwards compatibilty to ensure
that if these options were being passed, and were different to the expected
defaults, we would throw an informative error. But from looking at the
Angular CLI (the only known client of the linker) it has never been setting
these options so they have already always been set to the defaults.
BREAKING CHANGE:
Linked libraries no longer generate legacy i18n message ids. Any downstream
application that provides translations for these messages, will need to
migrate their message ids using the `localize-migrate` command line tool.
Closes#40673
PR Close#41554
When an Ivy NgModule is imported into a View Engine build, it doesn't have
metadata.json files that describe it as an NgModule, so it appears to VE
builds as a plain, undecorated class. The error message shown in this
situation generic and confusing, since it recommends adding an @NgModule
annotation to a class from a library.
This commit adds special detection into the View Engine compiler to give a
more specific error message when an Ivy NgModule is imported.
PR Close#41534
