fix(rspeedy): duplicate chunks in chunkSplit with multiple entries#502
fix(rspeedy): duplicate chunks in chunkSplit with multiple entries#502
Conversation
🦋 Changeset detectedLatest commit: 0ba51a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #502 will not alter performanceComparing Summary
|
packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts
Outdated
Show resolved
Hide resolved
packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts
Outdated
Show resolved
Hide resolved
packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts
Outdated
Show resolved
Hide resolved
| .join(','), | ||
| this.#appServiceFooter(), | ||
| ].join(''), | ||
| ...(Object.fromEntries( |
There was a problem hiding this comment.
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 output.inlineScripts?
| this.#appServiceBanner(), | ||
| Object.keys(manifest) | ||
| .map((name) => | ||
| `module.exports=lynx.requireModule('${ |
There was a problem hiding this comment.
Consider using lynx.requireModuleAsync when dealing with multiple chunks, as it allows scripts to load in parallel.
There was a problem hiding this comment.
Maybe this could be done in future PRs. Using a new option like scriptLoading
| this.#appServiceBanner(), | ||
| Object.keys(manifest) | ||
| .map((name) => | ||
| `module.exports=lynx.requireModule('${ |
There was a problem hiding this comment.
nitpick: can we avoid the multiple module.exports?
gaoachao
force-pushed
the
fix/chunk-split-multiple-entries
branch
2 times, most recently
from
85c9dcd to
0494a43
Compare
gaoachao
force-pushed
the
fix/chunk-split-multiple-entries
branch
from
6a23eba to
0ba51a1
Compare
|
see #874 |
<!-- Thank you for submitting a pull request! We appreciate the time and effort you have invested in making these changes. Please ensure that you provide enough information to allow others to review your pull request. Upon submission, your pull request will be automatically assigned with reviewers. If you want to learn more about contributing to this project, please visit: https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md. --> ## Summary <!-- Can you explain the reasoning behind implementing this change? What problem or issue does this pull request resolve? --> <!-- It would be helpful if you could provide any relevant context, such as GitHub issues or related discussions. --> #870 follow up #502 Support `output.inlineScripts`, which controls whether to inline scripts files when LynxEncodePlugin generates the manifest file. ## Checklist <!--- Check and mark with an "x" --> - [x] Tests updated (or not required). - [x] Documentation updated (or not required). --------- Signed-off-by: BitterGourd <91231822+gaoachao@users.noreply.github.com> Co-authored-by: Qingyu Wang <40660121+colinaaa@users.noreply.github.com>
3 participants
Summary
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.
Caution
BREAKING CHANGE:
encodeData.manifestnow only contains the/app-service.jsentry.TODO
Add support for synchronous chunk loading.
Checklist