Currently the decorator handlers are run against all `SourceFile`s in the compilation, but we shouldn't be doing it against declaration files. This initially came up as a CI issue in #33264 where it was worked around only for the `DirectiveDecoratorHandler`. These changes move the logic into the `TraitCompiler` and `DecorationAnalyzer` so that it applies to all of the handlers.
PR Close#34557
Now that the source to typings matching is able to handle
aliasing of exports, there is no need to handle aliases in private
declarations analysis.
These were originally added to cope when the typings files had
to use the name that the original source files used when exporting.
PR Close#34254
The naïve matching algorithm we previously used to match declarations in
source files to declarations in typings files was based only on the name
of the thing being declared. This did not handle cases where the declared
item had been exported via an alias - a common scenario when one of the two
file sets (source or typings) has been flattened, while the other has not.
The new algorithm tries to overcome this by creating two maps of export
name to declaration (i.e. `Map<string, ts.Declaration>`).
One for the source files and one for the typings files.
It then joins these two together by matching export names, resulting in a
new map that maps source declarations to typings declarations directly
(i.e. `Map<ts.Declaration, ts.Declaration>`).
This new map can handle the declaration names being different between the
source and typings as long as they are ultimately both exported with the
same alias name.
Further more, there is one map for "public exports", i.e. exported via the
root of the source tree (the entry-point), and another map for "private
exports", which are exported from individual files in the source tree but
not necessarily from the root. This second map can be used to "guess"
the mapping between exports in a deep (non-flat) file tree, which can be
used by ngcc to add required private exports to the entry-point.
Fixes#33593
PR Close#34254
Previously individual properties of the src bundle program were
passed to the reflection host constructors. But going forward,
more properties will be required. To prevent the signature getting
continually larger and more unwieldy, this change just passes the
whole src bundle to the constructor, allowing it to extract what it
needs.
PR Close#34254
Previously, the compiler performed an incremental build by analyzing and
resolving all classes in the program (even unchanged ones) and then using
the dependency graph information to determine which .js files were stale and
needed to be re-emitted. This algorithm produced "correct" rebuilds, but the
cost of re-analyzing the entire program turned out to be higher than
anticipated, especially for component-heavy compilations.
To achieve performant rebuilds, it is necessary to reuse previous analysis
results if possible. Doing this safely requires knowing when prior work is
viable and when it is stale and needs to be re-done.
The new algorithm implemented by this commit is such:
1) Each incremental build starts with knowledge of the last known good
dependency graph and analysis results from the last successful build,
plus of course information about the set of files changed.
2) The previous dependency graph's information is used to determine the
set of source files which have "logically" changed. A source file is
considered logically changed if it or any of its dependencies have
physically changed (on disk) since the last successful compilation. Any
logically unchanged dependencies have their dependency information copied
over to the new dependency graph.
3) During the `TraitCompiler`'s loop to consider all source files in the
program, if a source file is logically unchanged then its previous
analyses are "adopted" (and their 'register' steps are run). If the file
is logically changed, then it is re-analyzed as usual.
4) Then, incremental build proceeds as before, with the new dependency graph
being used to determine the set of files which require re-emitting.
This analysis reuse avoids template parsing operations in many circumstances
and significantly reduces the time it takes ngtsc to rebuild a large
application.
Future work will increase performance even more, by tackling a variety of
other opportunities to reuse or avoid work.
PR Close#34288
Prior to this commit, the `IvyCompilation` tracked the state of each matched
`DecoratorHandler` on each class in the `ts.Program`, and how they
progressed through the compilation process. This tracking was originally
simple, but had grown more complicated as the compiler evolved. The state of
each specific "target" of compilation was determined by the nullability of
a number of fields on the object which tracked it.
This commit formalizes the process of compilation of each matched handler
into a new "trait" concept. A trait is some aspect of a class which gets
created when a `DecoratorHandler` matches the class. It represents an Ivy
aspect that needs to go through the compilation process.
Traits begin in a "pending" state and undergo transitions as various steps
of compilation take place. The `IvyCompilation` class is renamed to the
`TraitCompiler`, which manages the state of all of the traits in the active
program.
Making the trait concept explicit will support future work to incrementalize
the expensive analysis process of compilation.
PR Close#34288
When ngcc is analyzing synthetically inserted decorators from a
migration, it is typically not expected that any diagnostics are
produced. In the situation where a diagnostic is produced, however, the
diagnostic would not be reported at all. This commit ensures that
diagnostics in migrations are reported.
PR Close#34014
In #32902 a bug was supposedly fixed where internal classes as used
within `ModuleWithProviders` are publicly exported, even when the
typings file already contained the generic type on the
`ModuleWithProviders`. This fix turns out to have been incomplete, as
the `ModuleWithProviders` analysis is not done when not processing the
typings files.
The effect of this bug is that formats that are processed after the
initial format had been processed would not have exports for internal
symbols, resulting in "export '...' was not found in '...'" errors.
This commit fixes the bug by always running the `ModuleWithProviders`
analyzer. An integration test has been added that would fail prior to
this change.
Fixes#33701
PR Close#33875
When upgrading an Angular application to a new version using the Angular
CLI, built-in schematics are being run to update user code from
deprecated patterns to the new way of working. For libraries that have
been built for older versions of Angular however, such schematics have
not been executed which means that deprecated code patterns may still be
present, potentially resulting in incorrect behavior.
Some of the logic of schematics has been ported over to ngcc migrations,
which are automatically run on libraries. These migrations achieve the
same goal of the regular schematics, but operating on published library
sources instead of used code.
PR Close#33362
A class that is provided as Angular service is required to have an
`@Injectable()` decorator so that the compiler generates its injectable
definition for the runtime. Applications are automatically migrated
using the "missing-injectable" schematic, however libraries built for
older version of Angular may not yet satisfy this requirement.
This commit ports the "missing-injectable" schematic to a migration that
is ran when ngcc is processing a library. This ensures that any service
that is provided from an NgModule or Directive/Component will have an
`@Injectable()` decorator.
PR Close#33362
This commit adapts the private NgModule re-export system (using aliasing) to
ngcc. Not all ngcc compilations are compatible with these re-exports, as
they assume a 1:1 correspondence between .js and .d.ts files. The primary
concern here is supporting them for commonjs-only packages.
PR Close#33177
ngcc may need to insert public exports into the bundle's source as well
as to the entry-point's declaration file, as the Ivy compiler may need
to create import statements to internal library types. The way ngcc
knows which exports to add is through the references registry, to which
references to things that require a public export are added by the
various analysis steps that are executed.
One of these analysis steps is the augmentation of declaration files
where functions that return `ModuleWithProviders` are updated so that a
generic type argument is added that corresponds with the `NgModule` that
is actually imported. This type has to be publicly exported, so the
analyzer step has to add the module type to the references registry.
A problem occurs when `ModuleWithProviders` already has a generic type
argument, in which case no update of the declaration file is necessary.
This may happen when 1) ngcc is processing additional bundle formats, so
that the declaration file has already been updated while processing the
first bundle format, or 2) when a package is processed which already
contains the generic type in its source. In both scenarios it may occur
that the referenced `NgModule` type does not yet have a public export,
so it is crucial that a reference to the type is added to the
references registry, which ngcc failed to do.
This commit fixes the issue by always adding the referenced `NgModule`
type to the references registry, so that a public export will always be
created if necessary.
Resolves FW-1575
PR Close#32902
In ngcc's reflection hosts for compiled JS bundles, such as ESM2015,
special care needs to be taken for classes as there may be an outer
declaration (referred to as "declaration") and an inner declaration
(referred to as "implementation") for a given class. Therefore, there
will also be two `ts.Symbol`s bound per class, and ngcc needs to switch
between those declarations and symbols depending on where certain
information can be found.
Prior to this commit, the `NgccReflectionHost` interface had methods
`getClassSymbol` and `findClassSymbols` that would return a `ts.Symbol`.
These class symbols would be used to kick off compilation of components
using ngtsc, so it is important for these symbols to correspond with the
publicly visible outer declaration of the class. However, the ESM2015
reflection host used to return the `ts.Symbol` for the inner
declaration, if the class was declared as follows:
```javascript
var MyClass = class MyClass {};
```
For the above code, `Esm2015ReflectionHost.getClassSymbol` would return
the `ts.Symbol` corresponding with the `class MyClass {}` declaration,
whereas it should have corresponded with the `var MyClass` declaration.
As a consequence, no `NgModule` could be resolved for the component, so
no components/directives would be in scope for the component. This
resulted in errors during runtime.
This commit resolves the issue by introducing a `NgccClassSymbol` that
contains references to both the outer and inner `ts.Symbol`, instead of
just a single `ts.Symbol`. This avoids the unclarity of whether a
`ts.Symbol` corresponds with the outer or inner declaration.
More details can be found here: https://hackmd.io/7nkgWOFWQlSRAuIW_8KPPwFixes#32078
Closes FW-1507
PR Close#32539
Remove the `formatProperty` property from the `EntryPointBundle`
interface, because the property is not directly related to that type.
It was only used in one place, when calling `fileWriter.writeBundle()`,
but we can pass `formatProperty` directrly to `writeBundle()`.
PR Close#32052
Our module resolution prefers `.js` files over `.d.ts` files because
occasionally libraries publish their typings in the same directory
structure as the compiled JS files, i.e. adjacent to each other.
The standard TS module resolution would pick up the typings
file and add that to the `ts.Program` and so they would be
ignored by our analyzers. But we need those JS files, if they
are part of the current package.
But this meant that we also bring in JS files from external
imports from outside the package, which is not desired.
This was happening for the `@fire/storage` enty-point
that was importing the `firebase/storage` path.
In this commit we solve this problem, for the case of imports
coming from a completely different package, by saying that any
file that is outside the package root directory must be an external
import and so we do not analyze those files.
This does not solve the potential problem of imports between
secondary entry-points within a package but so far that does
not appear to be a problem.
PR Close#30591
Rather than passing a number of individual arguments, we can
just pass an `EntryPointBundle`, which already contains them.
This is also a precursor to using more of the properties in the bundle.
PR Close#30591
This will allow users of the `EntryPointBundle` to use some of the `EntryPoint`
properties without us having to pass them around one by one.
PR Close#30591
To improve cross platform support, all file access (and path manipulation)
is now done through a well known interface (`FileSystem`).
For testing a number of `MockFileSystem` implementations are provided.
These provide an in-memory file-system which emulates operating systems
like OS/X, Unix and Windows.
The current file system is always available via the static method,
`FileSystem.getFileSystem()`. This is also used by a number of static
methods on `AbsoluteFsPath` and `PathSegment`, to avoid having to pass
`FileSystem` objects around all the time. The result of this is that one
must be careful to ensure that the file-system has been initialized before
using any of these static methods. To prevent this happening accidentally
the current file system always starts out as an instance of `InvalidFileSystem`,
which will throw an error if any of its methods are called.
You can set the current file-system by calling `FileSystem.setFileSystem()`.
During testing you can call the helper function `initMockFileSystem(os)`
which takes a string name of the OS to emulate, and will also monkey-patch
aspects of the TypeScript library to ensure that TS is also using the
current file-system.
Finally there is the `NgtscCompilerHost` to be used for any TypeScript
compilation, which uses a given file-system.
All tests that interact with the file-system should be tested against each
of the mock file-systems. A series of helpers have been provided to support
such tests:
* `runInEachFileSystem()` - wrap your tests in this helper to run all the
wrapped tests in each of the mock file-systems.
* `addTestFilesToFileSystem()` - use this to add files and their contents
to the mock file system for testing.
* `loadTestFilesFromDisk()` - use this to load a mirror image of files on
disk into the in-memory mock file-system.
* `loadFakeCore()` - use this to load a fake version of `@angular/core`
into the mock file-system.
All ngcc and ngtsc source and tests now use this virtual file-system setup.
PR Close#30921
Currently undecorated classes are intentionally not processed
with ngcc. This is causing unexpected behavior because decorator
handlers such as `base_def.ts` are specifically interested in class
definitions without top-level decorators, so that the base definition
can be generated if there are Angular-specific class members.
In order to ensure that undecorated base-classes work as expected
with Ivy, we need to run the decorator handlers for all top-level
class declarations (not only for those with decorators). This is similar
to when `ngtsc` runs decorator handlers when analyzing source-files.
Resolves FW-1355. Fixes https://github.com/angular/components/issues/16178
PR Close#30821
This commit introduces a new interface, which abstracts access
to the underlying `FileSystem`. There is initially one concrete
implementation, `NodeJsFileSystem`, which is simply wrapping the
`fs` library of NodeJs.
Going forward, we can provide a `MockFileSystem` for test, which
should allow us to stop using `mock-fs` for most of the unit tests.
We could also implement a `CachedFileSystem` that may improve the
performance of ngcc.
PR Close#29643
crisbeto
Pete Bacon Darwin
Alex Rickabaugh
JoostK
George Kalpakas
Paul Gschwendtner