When the compiler is invoked via ngc or the Angular CLI, its APIs are used
under the assumption that Angular analysis/diagnostics are only requested if
the program has no TypeScript-level errors. A result of this assumption is
that the incremental engine has not needed to resolve changes via its
dependency graph when the program contained broken imports, since broken
imports are a TypeScript error.
The Angular Language Service for Ivy is using the compiler as a backend, and
exercising its incremental compilation APIs without enforcing this
assumption. As a result, the Language Service has run into issues where
broken imports cause incremental compilation to fail and produce incorrect
results.
This commit introduces a mechanism within the compiler to keep track of
files for which dependency analysis has failed, and to always treat such
files as potentially affected by future incremental steps. This is tested
via the Language Service infrastructure to ensure that the compiler is doing
the right thing in the case of invalid imports.
PR Close#39923
Previously, if a component had an external template with a hard error, the
compiler would "forget" the link between that component and its NgModule.
Additionally, the NgModule would be marked as being in error, because the
template issue would prevent the compiler from registering the component
class as a component, so from the NgModule it would look like a declaration
of a non-directive/pipe class. As a combined result, the next incremental
step could fix the template error, but would not refresh diagnostics for the
NgModule, leading to an incrementality issue.
The various facets of this problem were fixed in prior commits. This commit
adds a test verifying the above case works now as expected.
PR Close#39923
To avoid overwhelming a user with secondary diagnostics that derive from a
"root cause" error, the compiler has the notion of a "poisoned" NgModule.
An NgModule becomes poisoned when its declaration contains semantic errors:
declarations which are not components or pipes, imports which are not other
NgModules, etc. An NgModule also becomes poisoned if it imports or exports
another poisoned NgModule.
Previously, the compiler tracked this poisoned status as an alternate state
for each scope. Either a correct scope could be produced, or the entire
scope would be set to a sentinel error value. This meant that the compiler
would not track any information about a scope that was determined to be in
error.
This method presents several issues:
1. The compiler is unable to support the language service and return results
when a component or its module scope is poisoned.
This is fine for compilation, since diagnostics will be produced showing the
error(s), but the language service needs to still work for incorrect code.
2. `getComponentScopes()` does not return components with a poisoned scope,
which interferes with resource tracking of incremental builds.
If the component isn't included in that list, then the NgModule for it will
not have its dependencies properly tracked, and this can cause future
incremental build steps to produce incorrect results.
This commit changes the tracking of poisoned module scopes to use a flag on
the scope itself, rather than a sentinel value that replaces the scope. This
means that the scope itself will still be tracked, even if it contains
semantic errors. A test is added to the language service which verifies that
poisoned scopes can still be used in template type-checking.
PR Close#39923
Previously, if a trait's analysis step resulted in diagnostics, the trait
would be considered "errored" and no further operations, including register,
would be performed. Effectively, this meant that the compiler would pretend
the class in question was actually undecorated.
However, this behavior is problematic for several reasons:
1. It leads to inaccurate diagnostics being reported downstream.
For example, if a component is put into the error state, for example due to
a template error, the NgModule which declares the component would produce a
diagnostic claiming that the declaration is neither a directive nor a pipe.
This happened because the compiler wouldn't register() the component trait,
so the component would not be recorded as actually being a directive.
2. It can cause incorrect behavior on incremental builds.
This bug is more complex, but the general issue is that if the compiler
fails to associate a component and its module, then incremental builds will
not correctly re-analyze the module when the component's template changes.
Failing to register the component as such is one link in the larger chain of
issues that result in these kinds of issues.
3. It lumps together diagnostics produced during analysis and resolve steps.
This is not causing issues currently as the dependency graph ensures the
right classes are re-analyzed when needed, instead of showing stale
diagnostics. However, the dependency graph was not intended to serve this
role, and could potentially be optimized in ways that would break this
functionality.
This commit removes the concept of an "errored" trait entirely from the
trait system. Instead, analyzed and resolved traits have corresponding (and
separate) diagnostics, in addition to potentially `null` analysis results.
Analysis (but not resolution) diagnostics are carried forward during
incremental build operations. Compilation (emit) is only performed when
a trait reaches the resolved state with no diagnostics.
This change is functionally different than before as the `register` step is
now performed even in the presence of analysis errors, as long as analysis
results are also produced. This fixes problem 1 above, and is part of the
larger solution to problem 2.
PR Close#39923
If the testcase has not specified that errors were expected, then any
errors that have occurred should be reported. These errors may have
prevented an output file from being generated, which resulted in hard
to debug test failures due to missing files.
PR Close#39862
This commit adds a few tests to verify that the `onDestroy` callbacks are invoked when `ComponentRef` instance
is destroyed and the logic is consistent between ViewEngine and Ivy.
PR Close#39876
In the new behavior Angular removes applications from the testability registry when the
root view gets destroyed. This eliminates a memory leak, because before that the
TestabilityRegistry holds references to HTML elements, thus they cannot be GCed.
PR Close#22106
PR Close#39876
The Language Service "find references" currently uses the
`ngtypecheck.ts` suffix to determine if a file is a shim file. Instead,
a better API would be to expose a method in the template type checker
that does this verification so that the LS does not have to "know" about
the typecheck suffix. This also fixes an issue (albeit unlikely) whereby a file
in the user's program that _actually_ is named with the `ngtypecheck.ts`
suffix would have been interpreted as a shim file.
PR Close#39768
This commit adds "find references" functionality to the Ivy integrated
language service. The basic approach is as follows:
1. Generate shims for all files to ensure we find references in shims
throughout the entire program
2. Determine if the position for the reference request is within a
template.
* Yes, it is in a template: Find which node in the template AST the
position refers to. Then find the position in the shim file for that
template node. Pass the shim file and position in the shim file along
to step 3.
* No, the request for references was made outside a template: Forward
the file and position to step 3.
3. (`getReferencesAtTypescriptPosition`): Call the native TypeScript LS
`getReferencesAtPosition`. For each reference that is in a shim file, map those
back to a template location, otherwise return it as-is.
PR Close#39768
There were two issues with the current TCB:
1. The logic for only wrapping the right hand side of the property write
if it was not already a parenthesized expression was incorrect. A
parenthesized expression could still have a trailing comment, and if
that were the case, that span comment would still be ambiguous, as explained
by the comment in the code before `wrapForTypeChecker`.
2. The right hand side of keyed writes was not wrapped in parens at all
PR Close#39768
In order to map the a safe property read's method access in the type check block
directly back to the property in the template source, we need to
include the `SafePropertyRead`'s `nameSpan` with the `ts.propertyAccess` for
the pipe's transform method.
Note that this is specifically relevant to the Language Service's "find
references" feature. As an example, with something like `{{a?.value}}`,
when calling "find references" on the 'value' we want the text
span of the reference to just be `value` rather than the entire source
`a?.value`.
PR Close#39768
In order to map the pipe's `transform` method in the type check block
directly back to the pipe name in the template source, we need to
include the `BindingPipe`'s `nameSpan` with the `ts.methodAccess` for
the pipe's transform method.
Note that this is specifically relevant to the Language Service's "find
references" feature. As an example, with something like `-2.5 | number:'1.0-0'`,,
when calling "find references" on the 'number' pipe we want the text
span of the reference to just be `number` rather than the entire binding
pipe's source `-2.5 | number:'1.0-0'`.
PR Close#39768
Fix a case where, if the parent class had already been patched, it would
not patch the child class. In addition to checking if the method is
defined in the prototype, and not inherited, it also does the same for
the unpatched method.
PR Close#39850
Previously, test descriptions used `latest` to refer to the most
recent/highest version. This was ambiguous, because `latest` can also
refer to the stable version of a package (e.g. see `@latest` npm tag).
This commit replaces `latest` with `highest` (or `highest for major`) to
reduce ambiguity.
Discussed in:
https://github.com/angular/angular/pull/39853#discussion_r531730317
PR Close#39853
Currently there is an issue with redirecting `rc.angular.io` to
`angular.io` when there is no active RC. If a user has visited
`rc.angular.io` before and has a ServiceWorker registered for that
subdomain, they will never "see" the redirect to `angular.io`.
This commit fixes the problem by doing an additional deployment from the
stable branch to the `rc-angular-io-site` Firebase site when there is no
active RC. This additional deployment will ensure that:
1. Users will be temporarily redirected from `rc.angular.io` to
`angular.io`.
2. Users with a registered ServiceWorker (who don't see the redirect)
will have their ServiceWorker unregistered on the next visit.
3. The content on both sites is identical.
See #39760 for more details on the problem and the solution.
NOTE:
As mentioned in #39760, for this fix to take affect, we need to remove
the redirect from `rc.angular.io` to `angular.io` in the Firebase
console for site `rc-angular-io-site`.
Fixes#39760
PR Close#39853
Previously, a branch would be deployed as `archive` even if it had a
major version that was equal/higher than that of the stable branch (as
long as it was not the RC branch - i.e. not the most recent minor
branch). For example, with `11.0.x` as the stable branch and `12.0.x`
as the RC branch, `11.1.x` would be deployed as archive.
Theoretically, we should never find ourselves in such a situation.
Typically, there will only be at most one minor branch most recent than
the stable one (and that branch will be the RC branch). However, it
is possible under unusual circumstances.
This commit adds additional checks to guard against this problem. It
also refactors the code in preparation of fixing an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker, which will
require identifying whether there is an active RC version or not.
See #39760 for more details.
PR Close#39853
Previously, the latest commit for branch may be computed multiple times
in the `deploy-to-firebase.js` tests.
This commit avoids the unnecessary re-computations by computing the
latest commits for the necessary branches at the beginning and using the
computed values throughout the tests.
PR Close#39853
Previously, the `deploy-to-firebase.js` script would only perform one
deployment operation on each run.
This commit adds support for performing multiple deployment operations.
NOTE:
In a subsequent commit, this will be leveraged fix an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker by
deploying the same artifacts to multiple Firebase projects/sites.
See #39760 for more details.
PR Close#39853
Previously, the `deploy()` function in `deploy-to-firebase.js` would
also perform other operations (beyond deploying), such as building the
app, checking the generated payload size, testing the PWA score of the
deployed app.
This commit decouples these operations, so that deploying can be
performed independently.
NOTE:
In a subsequent commit, this will be leveraged fix an issue with
`rc.angular.io` redirects in the presence of a ServiceWorker by
deploying the same artifacts to multiple Firebase projects/sites.
See #39760 for more details.
PR Close#39853
Since we turned off the Selenium Promise Manager in #39600, the AIO e2e
tests have started flaking on CI. After trying out several things, the
only change that seems to eliminate the flakiness is turning the
Selenium Promise Manager back on (see #39873 for more details).
This commit turns the Selenium Project Manager on to get rid of the
flakiness.
Fixes#39872
PR Close#39905
In #32497, an option was introduced to retry failed docs examples tests
a second time. This was done to work around some then recently
introduced flakiness. After inspecting ~50 recent CI docs examples jobs,
all tests passed on the first try.
This commit gets rid of the retry attempt. This will avoid covering up
any new flakiness introduces in the future.
PR Close#39905
This commit edits the copy on the interpolation page.
Tightens language, clarifies headers, and streamlines text.
There are no content or code changes.
PR Close#38687
Currently we convert objects to strings using `'' + value` which is quickest,
but it stringifies the value using its `valueOf`, rather than `toString`. These
changes switch to using `String(value)` which has identical performance
and calls the `toString` method as expected. Note that another option
was calling `toString` directly, but benchmarking showed it to be slower.
I've included the benchmark I used to verify the performance so we have it
for future reference and we can reuse it when making changes to `renderStringify`
in the future.
Also for reference, here are the results of the benchmark:
```
Benchmark: renderStringify
concat: 2.006 ns(0%)
concat with toString: 2.201 ns(-10%)
toString: 237.494 ns(-11741%)
toString with toString: 121.072 ns(-5937%)
constructor: 2.201 ns(-10%)
constructor with toString: 2.201 ns(-10%)
toString mono: 14.536 ns(-625%)
toString with toString mono: 9.757 ns(-386%)
```
Fixes#38839.
PR Close#39843
Jessica Janiuk
Alex Rickabaugh
JoostK
Alvaro
Keen Yee Liau
Trotyl
Kristiyan Kostadinov
Jeremy Elbourn
Andrew Kushnir
arturovt
Andrew Scott
Eduardo Speroni
George Kalpakas
Wes Grimes
Nicholas Papadopoulos
Kapunahele Wong
Иванов Дмитрий
Fabian Wiles
Jaime Oliveira
Tzimpoulas Nikos
