feat: allow globalObject config and reuse fetchBundle result#2123
feat: allow globalObject config and reuse fetchBundle result#2123
Conversation
🦋 Changeset detectedLatest commit: f38c900 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 |
📝 WalkthroughWalkthroughAdds a new optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
upupming
force-pushed
the
feat/globalObject
branch
from
e40b634 to
dbe393b
Compare
Merging this PR will degrade performance by 11.15%
Performance Changes
Comparing 
Footnotes
|
Web Explorer#7323 Bundle Size — 384.14KiB (0%).f38c900(current) vs 444f83b main#7318(baseline) Bundle metrics
|
| Current #7323 |
Baseline #7318 |
|
|---|---|---|
 Initial JS |
154.12KiB |
154.12KiB |
| Initial CSS |
35.05KiB |
35.05KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
238(-0.42%) |
239 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
2.99% |
2.99% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7323 |
Baseline #7318 |
|
|---|---|---|
| JS |
252.07KiB |
252.07KiB |
| Other |
97.02KiB |
97.02KiB |
| CSS |
35.05KiB |
35.05KiB |
Bundle analysis report Branch feat/globalObject Project dashboard
Generated by RelativeCI Documentation Report issue
upupming
force-pushed
the
feat/globalObject
branch
from
dbe393b to
7d6df7b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.changeset/wet-fans-sleep.md:
- Line 7: In the changelog sentence about Add `globalObject` config, change the
verb "config" to "configure" so it reads e.g. "user can configure it to
`globalThis` for BTS external bundle sharing"; update the phrase referencing
`globalObject` to use "configure" to correct the grammar while leaving the rest
of the text intact.
In `@packages/rspeedy/lynx-bundle-rslib-config/test/external-bundle.test.ts`:
- Around line 259-296: Update the two failing expectations in the "should mount
externals library to globalThis" test: when rslibConfig sets output.globalObject
= 'globalThis' the bundle will reference
globalThis[Symbol.for("__LYNX_EXTERNAL_GLOBAL__")], so change the asserted
substrings in the expect calls that inspect
decodedResult['custom-sections']['utils'] and
decodedResult['custom-sections']['utils__main-thread'] from
'lynx[Symbol.for("__LYNX_EXTERNAL_GLOBAL__")].ReactLynx.React' to
'globalThis[Symbol.for("__LYNX_EXTERNAL_GLOBAL__")].ReactLynx.React' so the test
matches the configured globalObject behavior.
In `@packages/webpack/externals-loading-webpack-plugin/src/index.ts`:
- Around line 314-316: The current runtimeGlobalsInit string unconditionally
overwrites the shared global returned by
getLynxExternalGlobal(externalsLoadingPluginOptions.globalObject); change it to
only initialize if the global is absent (avoid clobbering existing externals).
Update the runtimeGlobalsInit construction (the code that builds the string
assigned to runtimeGlobalsInit) to perform a conditional init (e.g., a
typeof/undefined check or guard) so it only sets the global to {} when it does
not already exist, using the same getLynxExternalGlobal(...) symbol in the
check.
🧹 Nitpick comments (5)
packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/filter-duplicate-externals/index.js (1)
19-29: Make the match whitespace-agnostic to avoid brittle tests.The updated split still depends on exact spaces, which can change with formatting/minification. A regex with
\s*keeps the intent while reducing false negatives.♻️ Suggested refactor
const mainThread = fs.readFileSync( path.resolve(__dirname, 'main:main-thread.js'), 'utf-8', ); + const externalAssignRe = + /lynx\[Symbol\.for\('__LYNX_EXTERNAL_GLOBAL__'\)\]\["Foo"\]\s*=\s*/g; + const countExternalAssign = (s) => (s.match(externalAssignRe) ?? []).length; - expect( - background.split( - `lynx[Symbol.for('__LYNX_EXTERNAL_GLOBAL__')]["Foo"]` - + ' = ', - ).length - 1, - ).toBe(1); + expect(countExternalAssign(background)).toBe(1); expect( - mainThread.split( - `lynx[Symbol.for('__LYNX_EXTERNAL_GLOBAL__')]["Foo"] ` - + '= ', - ).length - 1, - ).toBe(1); + countExternalAssign(mainThread), + ).toBe(1); });packages/webpack/externals-loading-webpack-plugin/etc/externals-loading-webpack-plugin.api.md (1)
20-21: Consider adding a@defaultTSDoc annotation in the source.The API Extractor warning indicates that the
globalObjectproperty is missing a@defaulttag in the source TypeScript definition. Based on the PR context, the default appears to be'lynx'. Adding this annotation would improve documentation and eliminate this warning.packages/webpack/externals-loading-webpack-plugin/test/cases/externals-loading/globalObject-customize/index.js (1)
17-28: Inconsistent string concatenation style in assertions.The split patterns are written with different whitespace placement:
- Line 19-20:
]["Foo"]+' = '- Line 25-26:
]["Foo"]+'= 'Both produce the same result
]["Foo"] =, but the inconsistency could confuse future maintainers. Consider using a consistent style.🔧 Suggested fix for consistency
expect( background.split( - `globalThis[Symbol.for('__LYNX_EXTERNAL_GLOBAL__')]["Foo"]` - + ' = ', + `globalThis[Symbol.for('__LYNX_EXTERNAL_GLOBAL__')]["Foo"] = `, ).length - 1, ).toBe(1); expect( mainThread.split( - `globalThis[Symbol.for('__LYNX_EXTERNAL_GLOBAL__')]["Foo"] ` - + '= ', + `globalThis[Symbol.for('__LYNX_EXTERNAL_GLOBAL__')]["Foo"] = `, ).length - 1, ).toBe(1);packages/rspeedy/lynx-bundle-rslib-config/test/external-bundle.test.ts (1)
230-230: Consider using distinct bundle IDs for better test isolation.Both tests use
id: 'utils-reactlynx'and write to the same output directory. While this works with sequential test execution, using distinct IDs (e.g.,'utils-reactlynx-default'and'utils-reactlynx-globalThis') would improve test isolation and make debugging easier.Also applies to: 267-267
packages/rspeedy/lynx-bundle-rslib-config/src/externalBundleRslibConfig.ts (1)
90-103: TightenglobalObjecttype to preserve API safety.
globalObject?: stringallows invalid values to compile and generate broken externals. Consider typing it asOutputConfig['globalObject'](or the same union) to keep config validation at compile time.♻️ Suggested refactor
-function transformExternals( - externals?: Externals, - globalObject?: string, -): Required<LibOutputConfig>['externals'] { +function transformExternals( + externals?: Externals, + globalObject?: OutputConfig['globalObject'], +): Required<LibOutputConfig>['externals'] {
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/react@0.116.1 ### Patch Changes - Fix the issue that lazy bundle HMR will lost CSS. ([#2134](#2134)) ## @lynx-js/rspeedy@0.13.2 ### Patch Changes - Bump Rsbuild 1.7.2 with Rspack 1.7.1. ([#2136](#2136)) ## @lynx-js/lynx-bundle-rslib-config@0.2.1 ### Patch Changes - Add [`globalObject`](https://webpack.js.org/configuration/output/#outputglobalobject) config for external bundle loading, user can configure it to `globalThis` for BTS external bundle sharing. ([#2123](#2123)) ## @lynx-js/external-bundle-rsbuild-plugin@0.0.2 ### Patch Changes - Add [`globalObject`](https://webpack.js.org/configuration/output/#outputglobalobject) config for external bundle loading, user can configure it to `globalThis` for BTS external bundle sharing. ([#2123](#2123)) - Updated dependencies \[[`959360c`](959360c)]: - @lynx-js/externals-loading-webpack-plugin@0.0.3 ## @lynx-js/externals-loading-webpack-plugin@0.0.3 ### Patch Changes - Add [`globalObject`](https://webpack.js.org/configuration/output/#outputglobalobject) config for external bundle loading, user can configure it to `globalThis` for BTS external bundle sharing. ([#2123](#2123)) ## create-rspeedy@0.13.2 ## upgrade-rspeedy@0.13.2 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist