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
