fix(multiple): ensure re-exported module symbols can be imported #30667
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
crisbeto
approved these changes
Mar 20, 2025
devversion
force-pushed
the
fix-exports-modules
branch
from
f129c5c to
5885f76
Compare
devversion
requested review from
mmalerba and
andrewseguin
and removed request for
a team
1 task
devversion
added
action: merge
devversion
changed the title
devversion
added
the
merge: fix commit message
devversion
force-pushed
the
fix-exports-modules
branch
from
5885f76 to
cec46d2
Compare
devversion
added a commit
to devversion/angular
that referenced
this pull request
Mar 20, 2025
…rted This commit adds a new integration test which will help ensure that all exported `@NgModule`'s of framework packages can be imported by users without any errors. This test is generally useful, but with our upcoming changes with relative imports, this is a good safety-net. Relative imports could break re-exported NgModules inside NgModule's. For more details, see: angular/components#30667 Notably we don't expect any issues for framework package as re-exporting `@NgModule`'s inside `@NgModule`'s is seemingly a rather rare pattern for APF libraries (confirmed by Material only having like 4-5 instances).
As of recently, we switched our imports from module imports to relative imports, when inside the same package. This is expected for proper code splitting, and also for our upcoming `rules_js` migration. Unfortunately this highlights an issue with the Angular compiler. We occasionally re-export other entry-point modules from one entry-point module. This is likely done for convenience, but we should stop doing that going forward (and likely will naturally resolve this over time with standalone either way). The problem is that the Angular compiler, especially with HMR enabled (i.e. no tree-shaking of imports), will try to generate imports in the user code to symbols that are indirectly re-exported. This worked before because the Angular compiler leveraged the "best guessed module", based on the "last module import" it saw before e.g. discovering the re-exported `ScrollingModule`; hence it assumed all exports of that module are available from `@angular/cdk/scrolling`. This assumption no longer works because the last module import would be e.g. `cdk/overlay`, which relatively imports from scrolling to re-export the module then. There are a few options: - teach the compiler about relative imports inside APF packages with secondary entry-points. Possible, but won't work for users with older compiler versions. Maybe a long-term good thing to do; on the other hand, standalone is the new future. - switch back to module imports. Not possible and relative imports should work inside a package IMO. - re-export the exposed symbols, under a private name. This is the easiest approach and there also aren't a lot of module re-exports; so this is a viable approach taken by this commit. Inside Google, the latter approach is automatically emitted by the compiler, when everything is built from source; but that's not usable here; but confirms this approach as being reasonable. Ideally we will stop re-exporting modules in APF packages, and long-term we start supporting the proper module guessing with relative imports. Fixes angular#30663.
devversion
force-pushed
the
fix-exports-modules
branch
from
cec46d2 to
204b91a
Compare
devversion
removed
the
merge: fix commit message
alxhub
pushed a commit
to angular/angular
that referenced
this pull request
Mar 20, 2025
…rted (#60489) This commit adds a new integration test which will help ensure that all exported `@NgModule`'s of framework packages can be imported by users without any errors. This test is generally useful, but with our upcoming changes with relative imports, this is a good safety-net. Relative imports could break re-exported NgModules inside NgModule's. For more details, see: angular/components#30667 Notably we don't expect any issues for framework package as re-exporting `@NgModule`'s inside `@NgModule`'s is seemingly a rather rare pattern for APF libraries (confirmed by Material only having like 4-5 instances). PR Close #60489
alxhub
pushed a commit
to angular/angular
that referenced
this pull request
Mar 20, 2025
…rted (#60489) This commit adds a new integration test which will help ensure that all exported `@NgModule`'s of framework packages can be imported by users without any errors. This test is generally useful, but with our upcoming changes with relative imports, this is a good safety-net. Relative imports could break re-exported NgModules inside NgModule's. For more details, see: angular/components#30667 Notably we don't expect any issues for framework package as re-exporting `@NgModule`'s inside `@NgModule`'s is seemingly a rather rare pattern for APF libraries (confirmed by Material only having like 4-5 instances). PR Close #60489
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As of recently, we switched our imports from module imports to relative imports, when inside the same package. This is expected for proper code splitting, and also for our upcoming
rules_jsmigration.Unfortunately this highlights an issue with the Angular compiler. We occasionally re-export other entry-point modules from one entry-point module. This is likely done for convenience, but we should stop doing that going forward (and likely will naturally resolve this over time with standalone either way).
The problem is that the Angular compiler, especially with HMR enabled (i.e. no tree-shaking of imports), will try to generate imports in the user code to symbols that are indirectly re-exported. This worked before because the Angular compiler leveraged the "best guessed module", based on the "last module import" it saw before e.g. discovering the re-exported
ScrollingModule; hence it assumed all exports of that module are available from@angular/cdk/scrolling. This assumption no longer works because the last module import would be e.g.cdk/overlay, which relatively imports from scrolling to re-export the module then.There are a few options:
teach the compiler about relative imports inside APF packages with secondary entry-points. Possible, but won't work for users with older compiler versions. Maybe a long-term good thing to do; on the other hand, standalone is the new future.
switch back to module imports. Not possible and relative imports should work inside a package IMO.
re-export the exposed symbols, under a private name. This is the easiest approach and there also aren't a lot of module re-exports; so this is a viable approach taken by this commit.
Fixes #30663.