-
Notifications
You must be signed in to change notification settings - Fork 122
fix(rspeedy): duplicate chunks in chunkSplit with multiple entries #502
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
Changes from all commits
5abce14
602ea64
47ebbe1
869d722
ba1c227
679d4fe
e48521b
54b3d42
917ee38
80b2be0
0ba51a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "@lynx-js/template-webpack-plugin": minor | ||
| --- | ||
|
|
||
| Fixed an issue in rspeedy where chunks weren't being properly shared between multiple entries during chunk splitting. This resulted in duplicate chunks being generated, increasing bundle size and reducing performance. | ||
|
|
||
| **BREAKING CHANGE**: `encodeData.manifest` now only contains the `/app-service.js` entry. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -146,6 +146,17 @@ export class LynxEncodePluginImpl { | |
| const { encodeData } = args; | ||
| const { manifest } = encodeData; | ||
|
|
||
| let publicPath = '/'; | ||
| if (typeof compilation?.outputOptions.publicPath === 'function') { | ||
| compilation.errors.push( | ||
| new compiler.webpack.WebpackError( | ||
| '`publicPath` as a function is not supported yet.', | ||
| ), | ||
| ); | ||
| } else { | ||
| publicPath = compilation?.outputOptions.publicPath ?? '/'; | ||
| } | ||
|
|
||
| if (!isDebug() && !isDev && !isRsdoctor()) { | ||
| [ | ||
| encodeData.lepusCode.root, | ||
|
|
@@ -169,27 +180,19 @@ export class LynxEncodePluginImpl { | |
| // lynx.requireModule('initial-chunk1') | ||
| // lynx.requireModule('initial-chunk2') | ||
| // `, | ||
| // 'initial-chunk1': `<content-of-initial-chunk>`, | ||
| // 'initial-chunk2': `<content-of-initial-chunk>`, | ||
| // }, | ||
| // ``` | ||
| '/app-service.js': [ | ||
| this.#appServiceBanner(), | ||
| Object.keys(manifest) | ||
| .map((name) => | ||
| `module.exports=lynx.requireModule('${ | ||
|
gaoachao marked this conversation as resolved.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this could be done in future PRs. Using a new option like
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: can we avoid the multiple |
||
| this.#formatJSName(name) | ||
| this.#formatJSName(name, publicPath) | ||
| }',globDynamicComponentEntry?globDynamicComponentEntry:'__Card__')` | ||
| ) | ||
| .join(','), | ||
| this.#appServiceFooter(), | ||
| ].join(''), | ||
| ...(Object.fromEntries( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. directly removing all the manifest would be a huge breaking change (and I don't think we are ready for that). So maybe add an option like |
||
| Object.entries(manifest).map(([name, source]) => [ | ||
| this.#formatJSName(name), | ||
| source, | ||
| ]), | ||
| )), | ||
| }; | ||
|
|
||
| return args; | ||
|
|
@@ -230,8 +233,8 @@ export class LynxEncodePluginImpl { | |
| return amdFooter + loadScriptFooter; | ||
| } | ||
|
|
||
| #formatJSName(name: string): string { | ||
| return `/${name}`; | ||
| #formatJSName(name: string, publicPath: string): string { | ||
| return publicPath + name; | ||
| } | ||
|
|
||
| protected options: Required<LynxEncodePluginOptions>; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.