perf(compiler-cli): optimize NgModule emit for standalone components #49837
Conversation
This file contains 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
alxhub
force-pushed
the
ngtsc/standalone/providers
branch
3 times, most recently
from
15a81a1
to
1946b14
Compare
atscott
previously requested changes
Apr 13, 2023
packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts
Outdated
Show resolved
Hide resolved
| */ | ||
| mayContainProviders( | ||
| ref: Reference<ClassDeclaration>, | ||
| callback?: (importRef: Reference<ClassDeclaration>) => void): boolean { |
There was a problem hiding this comment.
What is callback here supposed to be? I also don't see any calls to this function where the callback isn't passed so does it have to be optional?
There was a problem hiding this comment.
I renamed it to dependencyCallback and added jsdocs.
It's optional because, for the conceptual operation of "does X contain exported providers?", a consumer may only be interested in the answer and not in recording information about dependencies (for example, if producing a diagnostic instead of emitting code).
jessicajaniuk
added
the
area: compiler
alxhub
force-pushed
the
ngtsc/standalone/providers
branch
from
1946b14
to
aad4dec
Compare
atscott
approved these changes
Apr 14, 2023
NgModules which import standalone components currently list those components in their injector definitions, because we assume that any standalone component may export providers from its own imports. This commit adds an optimization for that emit, which attempts to statically analyze the NgModule imports and determine which standalone components, if any are present, do not export providers and thus can be omitted. This analysis is imperfect, because some imported components may be declared outside of the current compilation, or transitively import types which are declared outside the compilation. These types are therefore _assumed_ to carry providers and so the optimization isn't applied to them.
alxhub
force-pushed
the
ngtsc/standalone/providers
branch
from
aad4dec
to
bde2b44
Compare
alxhub
added
action: global presubmit
jessicajaniuk
added
target: rc
|
@alxhub I've updated this to |
|
This PR was merged into the repository by commit 78c76ce. |
jessicajaniuk
pushed a commit
that referenced
this pull request
Apr 17, 2023
…49837) NgModules which import standalone components currently list those components in their injector definitions, because we assume that any standalone component may export providers from its own imports. This commit adds an optimization for that emit, which attempts to statically analyze the NgModule imports and determine which standalone components, if any are present, do not export providers and thus can be omitted. This analysis is imperfect, because some imported components may be declared outside of the current compilation, or transitively import types which are declared outside the compilation. These types are therefore _assumed_ to carry providers and so the optimization isn't applied to them. PR Close #49837
|
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. |
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.
NgModules which import standalone components currently list those components in their injector definitions, because we assume that any standalone component may export providers from its own imports.
This commit adds an optimization for that emit, which attempts to statically analyze the NgModule imports and determine which standalone components, if any are present, do not export providers and thus can be omitted.
This analysis is imperfect, because some imported components may be declared outside of the current compilation, or transitively import types which are declared outside the compilation. These types are therefore assumed to carry providers and so the optimization isn't applied to them.