Commits on Dec 3, 2020

1. fix(compiler-cli): remove the concept of an errored trait …

   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.

   

   alxhub committed Dec 3, 2020
   Copy the full SHA

   3131535 View commit details

   Browse the repository at this point in the history

2. fix(compiler-cli): track poisoned scopes with a flag …

   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.

   

   alxhub committed Dec 3, 2020
   Copy the full SHA

   a86ad4f View commit details

   Browse the repository at this point in the history

3. fix(compiler-cli): correct incremental behavior even with broken imports …

   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.

   

   alxhub committed Dec 3, 2020
   Copy the full SHA

   d3e26ac View commit details

   Browse the repository at this point in the history