So-called "Quote expressions" were added in b6ec2387b3
to support foreign syntax to be used in Angular templates, requiring a custom
template transform to convert them somehow during compilation. Support for template
transforms was originally implemented in a43ed79ee7 but
has since been dropped. Since the compiler is not public API the quote expressions
should not have any usages anymore. Removing support for them can improve error
reporting for expressions that contain a `:`, e.g. binding to a URL without quotes:
```html
<a [href]="http://google.com">Click me</a>
```
Here, `http` would be parsed as foreign "http" quote expression with `//google.com` as
value, later reporting the error "Quotes are not supported for evaluation!" because
there was no template transform to convert that code.
Closes#40398
PR Close#44915
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
TypeScript configures `strictNullChecks` to be disabled by default, so the nullish
coalescing check should follow the same default. The rule actively depends on
`strictNullChecks`, as TypeScript doesn't include `null`/`undefined` in its types
otherwise so the check wouldn't have a way to differentiate between them.
This commit also takes the `strict` flag into account when `strictNullChecks` itself
is not configured.
PR Close#44862
The initial commit e9124b42d5 stored the errors rather than
throwing but did not store them in a place that was accessible to consumers. Instead,
the errors should be added to the IndexedComponent so they can be surfaced where the
index results are consumed
PR Close#44884
When the indexer encounters a location where the source span doesn't
match up with the expected identifier, the current visitor code throws
an error. Instead, this change creates an error and moves on to the next
template item. This allows the indexer to continue analysis even when
there are errors in the source mapping. In addition, it still allows callers
to surface those errors in their own way while still providing as much indexed
information as possible about a node.
PR Close#44825
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
The previous fix for correcting spans with comments in
59eef29a6c
had the unfortunate side effect of _breaking_ the spans with comments
when there was leading whitespace. This happened because the previous
fix was testing one without a comment, identifying that the offset shouldn't
have anything added to it, and then removing that offset adjustment
(`offsets[i] + (expressionText.length - sourceToLex.length)`).
Upon further investigation, this offset adjustment _was actually
necessary_ for when the input had comments, but this was only because
the `stripComments` function used `trim` to remove whitespace for these
cases. This is the real problem -- not only does it create a ton of confusion
but also it means that the behavior of the lexer and resulting spans is
different between inputs with comments and inputs without comments.
After reviewing how the `inputLength` of `_ParseAST` was used, it
appears that the correct behavior would be to _not_ trim the input. The
`inputLength` is used to advance the current index beyond points which
have been processed. This _should_ include any whitespace. Additionally,
`inputLength` doesn't appear to be needed at all. When there was no
comment in the input, it was always equal to the `input.length` anyways.
When there _is_ a comment, it should include that comment anyways to
advance the index beyond the comment.
PR Close#44785
The original fix for svg elements in
92b23f4851
did not account for svg elements when they also had a structural
directive on them, making the node a template. This resulted in the
logic added in fix above not being applied.
PR Close#44785
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.
There were two remaining places where `_extendedTemplateDiagnostics` needed to be set which should have been removed in #44712 but got missed. This updates them to only require `strictTemplates` and not `_extendedTemplateDiagnostics` so the feature is properly enabled in production.
PR Close#44777
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
This commit reduces the analysis work that needs to happen during an
incremental rebuild by properly recording files for which no traits were found
in the set of files that have no traits, such that the same file doesn't have
to be reanalyzed during subsequent rebuilds. It also excludes shim files from
analysis.
PR Close#44731
Bundlers like Rollup may use an element access expression for an export
declaration, which causes ngcc to ignore those export declarations possibly
resulting in incomplete processing of packages.
Element access syntax may be used when the declared name is not considered
as valid JS identifier, but bundlers may be conservative in determining whether
an identifier can be used (to emit a property access) and opt for a string
literal in an element access instead.
The element access syntax introduces a problem for ngcc, where it wouldn't
consider such export as class declaration, causing them to be skipped. The
ngtsc compiler is implemented with the assumption that all class declarations
use a `ts.Identifier` as name, whereas the element access is using a string
literal for the declared name. This makes it troublesome for ngcc to support
this syntax form in UMD bundles.
To work around the problem, this function transforms these access expressions
into regular property accesses. The source text is parsed to an AST to allow
finding the element accesses in a robust way, after which the affected text
ranges are replaced with property accesses in the original source text.
Closes#44037
PR Close#44669
In the `Element` node of a parsed `<svg>` element, the `name` is recorded
as `:svg:svg`. When the Angular Indexer ran over this element, it would
attempt to find this name in the template text and fail, as the namespace
portion of the name was added automatically at parse time and is of course
missing from the original template.
This commit changes the indexer to detect these namespaced elements and only
search for the tag name.
PR Close#44678
When parsing a binding with a comment at the end of the expression, the
parser previously had logic to offset the parsed spans by the length of the
comment. This logic seemed not to serve any useful purpose, and instead
resulted in the corruption of the spans. For example, in the expression
`{{foo // comment}}`, the parser would map the parsed `foo` `PropertyRead`
node at the location of the characters 'ent' from the comment suffix.
This commit removes that logic, correcting the parsed spans of such nodes in
the presence of comments. Removing this logic does not seem to have caused
any tests to fail.
PR Close#44678
Previously, the Angular Indexer made an assumption that in any binding to
a property of an `ImplicitReceiver`, the property name begins at the start
of the binding. This is true for normal reads from `ImplicitReceiver` as
the implicit receiver has no representation in the template.
However, `ThisReceiver` inherits from `ImplicitReceiver` and _does_ have a
template representation. Such a binding looks like `{{this.foo}}`. This
commit corrects the logic of the indexer to use the `nameSpan` of the
property binding instead of its `sourceSpan` to locate the identifier.
PR Close#44678
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 includes a mapping of extended template diagnostics to their associated diagnostic category. It is generated from the list of diagnostic names, so the list should always be implicitly kept up to date. Usage looks like:
```json
{
"angularCompilerOptions": {
"extendedDiagnostics": {
"checks": {
"invalidBananaInBox": "error",
"nullishCoalescingNotNullable": "suppress"
}
}
}
}
```
PR Close#44391
Refs #42966.
This is a static array of all the 1P extended template diagnostic factories built into the Angular compiler directly. It provides an encapsulated list of all diagnostics rather than requiring users to manually list each one individually.
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
Refs #42966.
The enum of extended template diagnostic names allows a global registry of first-party diagnostics with a developer-friendly string name which can be used for configuration. This name is used in the new `TemplateCheckFactory` to bind the name to a particular `ErrorCode` and make both available *before* constructing the actual template check, which is necessary to configure it appropriately.
PR Close#44391
In `language-service`, the `checker.getDirectiveMetadata` doesn't return the animations meta of the `Component`.
but it's useful for animation completion.
PR Close#44630
The changes in 2028c3933f caused method
calls to be emitted using additional parenthesis into the TCB, which in
turn prevented proper type narrowing when the method acts as a type
guard. This commit special-cases method calls from property reads to
avoid the additional parenthesis.
Fixes#44353
PR Close#44447
This was flagged during the code review of #44580. When generating a type check block, we were interpreting any call to `$any` as an `as any` cast, even if it's part of a `PropertyRead` (e.g. `foo.$any(1)`). This is handled correctly in other parts of the compiler, but it looks like it was missed in the type checker.
PR Close#44657
This page exists in the most recent angular.io version (v13 currently), so there's no need to link to an old version. The hash also refers to the title section of the page, which isn't necessary and is now dropped.
PR Close#44649
Dev mode output was switched from ES5 -> ES2015 recently and as a part of those changes, some target names that contained `_es5` postfixes were changes to `_es2015` instead. This commit fixes the issue with one of the recently merged BUILD files that contained the old (`_es5`) postfix.
PR Close#44651
When building a library, the `rootDir` option is configured to ensure
that all source files are present within the entry-point that is being
build. This imposes an extra constraint on the reference emit logic,
which does not allow emitting a reference into a source file outside of
this `rootDir`.
During the generation of type-check blocks we used to make a best-effort
estimation of whether a type reference can be emitted into the
type-check file. This check was relaxed in #42492 to support emitting
more syntax forms and type references, but this change did not consider
the `rootDir` constraint that is present in library builds. As such, the
compiler might conclude that a type reference is eligible for emit into
the type-check file, whereas in practice this would cause a failure.
This commit changes the best-effort estimation into a "preflight"
reference emit that is fully accurate as to whether emitting a type
reference is possible.
Fixes#43624
PR Close#44587
The `NgtscCompilerHost` is implemented using the `FileSystem`
abstraction of the compiler, which is implemented for tests using an
in-memory `MockFileSystem`. If the in-memory filesystem contains
symlinks, then using `NgtscCompilerHost` would not reflect their
resolved real path. Instead, the TypeScript compiler would use its
default implementation based on the real filesystem, which is unaware of
the in-memory `MockFileSystem` setup.
This change does not currently address any issues, but is being fixed
as it prevented a reproduction scenario from behaving correctly.
PR Close#44587
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
This commit moves some logic to make the location of runtime error codes consistent across packages. Now all error codes are located in `packages/core/src/errors.ts` file.
PR Close#44398
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
The Ivy compiler no longer generates code for type-checking purposes
using the output AST types. Instead, it uses TypeScript AST nodes and
its printer for type-checking. This commit removes the type-related
output nodes.
PR Close#44411
The `ng_rollup_bundle` rule has been replaced with a new rule called
`app_bundle`. This rule replicates the Angular v13 optimization
pipeline in the CLI, so that we can get better benchmarking results.
Also the rule is much simpler to maintain as it relies on ESbuild.
The old `ng_rollup_bundle` rule did rely on e.g. build-optimizer that no
longer has an effect on v13 Angular packages, so technically size
tests/symbol tests were no longer as correct as they were before. This
commit fixes that.
A couple of different changes and their explanation:
* Language-service will no longer use the benchmark rule for creating
its NPM bundles! It will use plain `rollup_bundle`. ESBuild would have
been nice but the language-service relies on AMD that ESBuild cannot
generate (yet?)
* Service-worker ngsw-worker.js file was generated using the benchmark
bundle rule. This is wrong. We will use a simple ESbuild rule in the
future. The output is more predictable that way, and we can have a
clear use of the benchmark bundle rule..
* A couple of benchmarks in `modules/` had to be updated to use e.g.
`initTableUtils` calls. This is done because with the new rule, all
files except for the entry-point are considered side-effect free. The
utilities for benchmarks relied on side-effects in some
transitively-loaded file (bad practice anyway IMO). We are now
initializing the utilities using a proper init function that is
exported...
PR Close#44490
Cleans up some of the temporary workarounds that were necessary in order to land support for TypeScript 4.5 since they're no longer necessary.
PR Close#44477
Now that ViewEngine libraries can no longer be created, the latest
TypeScript version that ngcc should be able to process is TypeScript 4.3,
i.e. the version of TypeScript that was supported in Angular 12.
However, ngcc's integration tests used the TypeScript version of the
workspace to create the JavaScript files from TypeScript sources on
demand. This introduces friction when upgrading the TypeScript version
within the workspace, as changes to TypeScript's emit format may affect
ngcc's ability to process it correctly.
The on demand creation of JavaScript files was convenient for authoring
tests, but it also helped to detect incompatibilities with newer
versions of TypeScript. Now that ngcc no longer has to process newer
versions of TypeScript, we want to pin the integration suite to use
JavaScript code as if it were compiled using TypeScript 4.3, i.e. a
version of TypeScript that is actually supported by ngcc.
This commit updates the integration test to inline all the generated
files directly in the tests, instead of compiling them on demand. This
was done by temporarily installing TypeScript 4.3 and using it to create
the `loadTestFiles` statement from the original TypeScript inputs.
An alternative could have been to install TypeScript 4.3 as an actual
dependency in the workspace and using that to continue compiling the
integration suite on demand, but this brings some overhead in package
installations (TypeScript is ~60MB) and the authoring aspect of ngcc
integration test is expected to diminish, now that ngcc's support is no
longer a moving target.
PR Close#44448
