Repository navigation
perf(@angular/build): avoid duplicate Domino bundling in server bundles - #34527
alan-agius4 wants to merge 1 commit into
Conversation
angular-robot
Bot
added
area: performance
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two new esbuild plugins to ensure Domino is bundled only once for the server. The first plugin exposes the Domino instance on globalThis within the server polyfills bundle, and the second redirects Domino imports in the main server bundle to consume this global instance. Behavior tests have also been added to verify this sharing. The review feedback highlights a potential runtime issue where exporting Domino as a default export in the consumer plugin could break namespace imports, and suggests using CommonJS module.exports to ensure compatibility.
alan-agius4
added
the
target: patch
alan-agius4
added
freq4: critical
Avoid bundling duplicate copies of Domino in both the server polyfills bundle and the main server bundle. The server polyfills bundle exposes the Domino instance attached to globalThis['ɵdomino'] when @angular/platform-server/init is bundled, and the main server bundle redirects Domino imports to this shared instance.
7814f94 to
58f0583
Compare
| * | ||
| * @returns An esbuild plugin. | ||
| */ | ||
| export function createPlatformServerDominoPlugin(): Plugin { |
There was a problem hiding this comment.
nit: Maybe createPlatformServerDominoRedirectPlugin or similar for symmetry with the above plugin?
| ? { | ||
| path: resolve(args.resolveDir, args.path), | ||
| namespace: 'angular:domino-expose', | ||
| } |
There was a problem hiding this comment.
| ? { | |
| path: resolve(args.resolveDir, args.path), | |
| namespace: 'angular:domino-expose', | |
| } | |
| ? { | |
| path: 'angular:platform-server-domino', | |
| namespace: 'angular:domino-expose', | |
| pluginData: { | |
| importPath: args.path, | |
| resolveDir: args.resolveDir, | |
| }, | |
| } |
To avoid passing the absolute path into the build outputs. Relevant changes would be needed to consume that in the onLoad hook.
PR Checklist
[✓] The commit message follows our guidelines: https://github.com/angular/angular-cli/blob/main/CONTRIBUTING.md#-commit-message-guidelines
[✓] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features)
PR Type
[ ] Bugfix
[✓] Feature / Performance
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:
What is the current behavior?
When bundling an application with server-side rendering, Domino (~560 KB unminified) is bundled into both
polyfills.server.mjs(via@angular/platform-server/init) andmain.server.mjs(via@angular/platform-server).What is the new behavior?
Domino is bundled only once in
polyfills.server.mjsand attached toglobalThis['ɵdomino']. The main server bundle redirects Domino imports to this shared instance.This replaces the redundant bundled copy of Domino in
main.server.mjswithout requiring manual filesystem probing oradditionalContentparameter threading in polyfill options.Does this PR introduce a breaking change?
[ ] Yes
[✓] No