ngcc - handle multiple basePaths & use typings for dependency resolution#29643
Closed
petebacondarwin wants to merge 13 commits into
Closed
ngcc - handle multiple basePaths & use typings for dependency resolution#29643petebacondarwin wants to merge 13 commits into
petebacondarwin wants to merge 13 commits into
Conversation
60e6757 to
e764fa1
Compare
alan-agius4
reviewed
Apr 5, 2019
c1f6efa to
a99108f
Compare
…ters The `Transformer` and `Renderer` classes do not actually need a `sourcePath` value as by the time they are doing their work we are only working directly with full absolute paths.
This method was poorly named for what it does, and did not have a return type.
The test now attempts to compile an entry-point (@angular/common/http/testing) that has a transient "private" dependency. A private dependency is one that is only visible by looking at the compiled JS code, rather than the generated TS typings files. This proves that we can't rely on typings files alone for computing the dependencies between entry-points.
Previously we completely ignored entry-points that had not been compiled with Angular, since we do not need to compile them with ngcc. But this makes it difficult to reason about dependencies between entry-points that were compiled with Angular and those that were not. Now we do track these non-Angular compiled entry-points but they are marked as `compiledByAngular: false`.
When working out the dependencies between entry-points ngcc must parse the import statements and then resolve the import path to the actual file. This is complicated because module resolution is not trivial. Previously ngcc used the node.js `require.resolve`, with some hacking to resolve modules. This change refactors the `DependencyHost` to use a new custom `ModuleResolver`, which is optimized for this use case. Moreover, because we are in full control of the resolution, we can support TS `paths` aliases, where not all imports come from `node_modules`. This is the case in some CLI projects where there are compiled libraries that are stored locally in a `dist` folder. See //FW-1210.
By passing a `pathMappings` configuration (a subset of the `ts.CompilerOptions` interface), we can instuct ngcc to process additional paths outside the `node_modules` folder.
For UMD/RequireJS support we will need to have multiple `DependencyHost` implementations. This commit prepares the ground for that.
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.
…al-path` The ngcc code now uses `AbsoluteFsPath` and `PathSegment` to do all its path manipulation.
a99108f to
e297d92
Compare
Contributor
Author
|
Just merging into master now that RC has been released. |
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
) This method was poorly named for what it does, and did not have a return type. PR Close #29643
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
The test now attempts to compile an entry-point (@angular/common/http/testing) that has a transient "private" dependency. A private dependency is one that is only visible by looking at the compiled JS code, rather than the generated TS typings files. This proves that we can't rely on typings files alone for computing the dependencies between entry-points. PR Close #29643
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
Previously we completely ignored entry-points that had not been compiled with Angular, since we do not need to compile them with ngcc. But this makes it difficult to reason about dependencies between entry-points that were compiled with Angular and those that were not. Now we do track these non-Angular compiled entry-points but they are marked as `compiledByAngular: false`. PR Close #29643
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
When working out the dependencies between entry-points ngcc must parse the import statements and then resolve the import path to the actual file. This is complicated because module resolution is not trivial. Previously ngcc used the node.js `require.resolve`, with some hacking to resolve modules. This change refactors the `DependencyHost` to use a new custom `ModuleResolver`, which is optimized for this use case. Moreover, because we are in full control of the resolution, we can support TS `paths` aliases, where not all imports come from `node_modules`. This is the case in some CLI projects where there are compiled libraries that are stored locally in a `dist` folder. See //FW-1210. PR Close #29643
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
By passing a `pathMappings` configuration (a subset of the `ts.CompilerOptions` interface), we can instuct ngcc to process additional paths outside the `node_modules` folder. PR Close #29643
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
) For UMD/RequireJS support we will need to have multiple `DependencyHost` implementations. This commit prepares the ground for that. PR Close #29643
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
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
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2019
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves FW-1204