-
Notifications
You must be signed in to change notification settings - Fork 25.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ngcc): capture entry-point dependencies from typings as well as source #34494
fix(ngcc): capture entry-point dependencies from typings as well as source #34494
Conversation
f556191
to
fd0629f
Compare
packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts
Outdated
Show resolved
Hide resolved
packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts
Show resolved
Hide resolved
Could you update the commit message to include the fixes #34411 reference? |
8675f98
to
1bdb64e
Compare
1bdb64e
to
75dadbf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OOC, have you done any measurements regarding how much in the overall performance affected by this change?
packages/compiler-cli/ngcc/test/dependencies/dependency_resolver_spec.ts
Show resolved
Hide resolved
Rather than return a new object of dependency info from calls to `collectDependencies()` we now pass in an object that will be updated with the dependency info. This is in preparation of a change where we will collect dependency information from more than one `DependencyHost`. Also to better fit with this approach the name is changed from `findDependencies()` to `collectDependencies()`.
75dadbf
to
4ca6faf
Compare
The `DependencyHost` implementations were duplicating the "postfix" strings which are used to find matching paths when resolving module specifiers. Now the hosts reuse the postfixes given to the `ModuleResolver` that is passed to the host.
…ource ngcc computes a dependency graph of entry-points to ensure that entry-points are processed in the correct order. Previously only the imports in source files were analysed to determine the dependencies for each entry-point. This is not sufficient when an entry-point has a "type-only" dependency - for example only importing an interface from another entry-point. In this case the "type-only" import does not appear in the source code. It only appears in the typings files. This can cause a dependency to be missed on the entry-point. This commit fixes this by additionally processing the imports in the typings program, as well as the source program. Note that these missing dependencies could cause unexpected flakes when running ngcc in async mode on multiple processes due to the way that ngcc caches files when they are first read from disk. Fixes angular#34411 // FW-1781
4ca6faf
to
d21b34e
Compare
RE performance change: note that this change only affects the dependency analysis phase, which is around 7.4% of the total processing time (using integration/cli-hello-world-ivy-compat). After this PR the average percentage goes up to around 7.8%. |
Rather than return a new object of dependency info from calls to `collectDependencies()` we now pass in an object that will be updated with the dependency info. This is in preparation of a change where we will collect dependency information from more than one `DependencyHost`. Also to better fit with this approach the name is changed from `findDependencies()` to `collectDependencies()`. PR Close #34494
…ource (#34494) ngcc computes a dependency graph of entry-points to ensure that entry-points are processed in the correct order. Previously only the imports in source files were analysed to determine the dependencies for each entry-point. This is not sufficient when an entry-point has a "type-only" dependency - for example only importing an interface from another entry-point. In this case the "type-only" import does not appear in the source code. It only appears in the typings files. This can cause a dependency to be missed on the entry-point. This commit fixes this by additionally processing the imports in the typings program, as well as the source program. Note that these missing dependencies could cause unexpected flakes when running ngcc in async mode on multiple processes due to the way that ngcc caches files when they are first read from disk. Fixes #34411 // FW-1781 PR Close #34494
…ource (#34494) ngcc computes a dependency graph of entry-points to ensure that entry-points are processed in the correct order. Previously only the imports in source files were analysed to determine the dependencies for each entry-point. This is not sufficient when an entry-point has a "type-only" dependency - for example only importing an interface from another entry-point. In this case the "type-only" import does not appear in the source code. It only appears in the typings files. This can cause a dependency to be missed on the entry-point. This commit fixes this by additionally processing the imports in the typings program, as well as the source program. Note that these missing dependencies could cause unexpected flakes when running ngcc in async mode on multiple processes due to the way that ngcc caches files when they are first read from disk. Fixes #34411 // FW-1781 PR Close #34494
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. |
See FW-1781 for the background
Fixes #34411