fix(platform-browser-dynamic): ensure compiler is loaded before @angular/common#60458
Closed
devversion wants to merge 1 commit intoangular:mainfrom
Closed
fix(platform-browser-dynamic): ensure compiler is loaded before @angular/common#60458devversion wants to merge 1 commit intoangular:mainfrom
@angular/common#60458devversion wants to merge 1 commit intoangular:mainfrom
Conversation
alan-agius4
approved these changes
Mar 19, 2025
…ular/common` With the recent changes to the APF bundling rules, we turned on tree-shaking in rollup to support proper code splitting for FESM bundles. This resulted in Rollup re-ordering imports in the FESM bundles of `@angular/platform-browser-dynamic`— highlighting that over the past years, this package "accidentally" resulted in the side-effects of the compiler registering itself globally. This continues to be the case, and our changes generally didn't cause any issues in CLI applications because the CLI explicitly wires up the compiler (as expected) before `-dynamic` is even loaded. For custom setup, like Analog, this order change surfaced a breakage because e.g. `@angular/common` with its JIT decorators of e.g. directives/services are triggered before the compiler is actually loaded/made available. This commit fixes this. The explicit imports in practice are a noop because our FESM bundling doesn't recognize compiler as side-effects true; but marking the whole -dynamic package as having side-effects; prevents rollup from swapping the import order. Long-term, we should look into improving this by teaching `ng_package` that e.g. the compiler has side-effects; so that the `import @angular/compiler` statement is not dropped when constructing FESM bundles.
59c693e to
e1518da
Compare
Member
|
This PR was merged into the repository by commit 3606aab. The changes were merged into the following branches: main, 19.2.x |
pkozlowski-opensource
pushed a commit
that referenced
this pull request
Mar 19, 2025
…ular/common` (#60458) With the recent changes to the APF bundling rules, we turned on tree-shaking in rollup to support proper code splitting for FESM bundles. This resulted in Rollup re-ordering imports in the FESM bundles of `@angular/platform-browser-dynamic`— highlighting that over the past years, this package "accidentally" resulted in the side-effects of the compiler registering itself globally. This continues to be the case, and our changes generally didn't cause any issues in CLI applications because the CLI explicitly wires up the compiler (as expected) before `-dynamic` is even loaded. For custom setup, like Analog, this order change surfaced a breakage because e.g. `@angular/common` with its JIT decorators of e.g. directives/services are triggered before the compiler is actually loaded/made available. This commit fixes this. The explicit imports in practice are a noop because our FESM bundling doesn't recognize compiler as side-effects true; but marking the whole -dynamic package as having side-effects; prevents rollup from swapping the import order. Long-term, we should look into improving this by teaching `ng_package` that e.g. the compiler has side-effects; so that the `import @angular/compiler` statement is not dropped when constructing FESM bundles. PR Close #60458
|
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.
3 participants
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.
Note: CLI users were not affected by this. This is only for custom setups.
--
With the recent changes to the APF bundling rules, we turned on tree-shaking in rollup to support proper code splitting for FESM bundles.
This resulted in Rollup re-ordering imports in the FESM bundles of
@angular/platform-browser-dynamic— highlighting that over the past years, this package "accidentally" resulted in the side-effects of the compiler registering itself globally.This continues to be the case, and our changes generally didn't cause any issues in CLI applications because the CLI explicitly wires up the compiler (as expected) before
-dynamicis even loaded. For custom setup, like Analog, this order change surfaced a breakage because e.g.@angular/commonwith its JIT decorators of e.g. directives/services are triggered before the compiler is actually loaded/made available.This commit fixes this. The explicit imports in practice are a noop because our FESM bundling doesn't recognize compiler as side-effects true; but marking the whole -dynamic package as having side-effects; prevents rollup from swapping the import order. Long-term, we should look into improving this by teaching
ng_packagethat e.g. the compiler has side-effects; so that theimport @angular/compilerstatement is not dropped when constructing FESM bundles.