feat: support load bts chunk from remote address #1834
Conversation
🦋 Changeset detectedLatest commit: 89fa876 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 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 background-thread chunk loader with sync/async read APIs and per-instance template cache, introduces BTSChunkEntry and NativeApp.readScript, changes template emission to file-name-based sourceURL annotations and lepusCode filtering, refactors native-app loading to use the new loader, and updates tests and manifest entries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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! |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
105-114: Don’t force eager parse for BTS/lazy templatesThe comment above notes BTS should not be parsed eagerly to avoid memory bloat, but the code always passes
true. Make it conditional on appType.Apply this diff:
- true, + template.appType! === 'card',
🧹 Nitpick comments (5)
packages/web-platform/web-constants/src/types/NativeApp.ts (1)
126-134: Document readScript semantics and constraintsConsider a brief JSDoc for readScript (sync, returns raw text; allowed in Worker, CORS applies) and loadScriptAsync’s optional entryName.
packages/web-platform/web-tests/tests/web-core.test.ts (1)
208-224: Avoid fixed 3s sleep; poll for readiness to deflakeReplace the hard wait with a short polling loop against readScript to reduce flakes and test time.
Apply this diff:
await mainWorker.evaluate(() => { globalThis.runtime.renderPage = () => {}; }); - await wait(3000); - const backWorker = await getBackgroundThreadWorker(page); - const jsonContent = await backWorker.evaluate(() => { - const nativeApp = globalThis.runtime.lynx.getNativeApp(); - return nativeApp.readScript('json'); - }); - await wait(100); + const backWorker = await getBackgroundThreadWorker(page); + const jsonContent = await backWorker.evaluate(async () => { + // retry up to ~3s + for (let i = 0; i < 30; i++) { + try { + const content = globalThis.runtime.lynx.getNativeApp().readScript('json'); + if (content) return content; + } catch (_) {} + await new Promise(r => setTimeout(r, 100)); + } + return ''; + }); expect(jsonContent).toBe('{}');packages/web-platform/web-constants/src/types/LynxModule.ts (1)
44-82: Guard against arg-order drift between type and runtime new Function usageThis parameter list must exactly match the arg names passed in createChunkLoading.ts’s new Function. To prevent silent breakage, consider centralizing the arg names in a shared const tuple and reusing it both here (for the type) and there (for the runtime constructor).
If you’d like, I can draft a small shared “BTS_ARG_NAMES” tuple and wire both sites to it to eliminate this class of regressions.
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
66-77: Filename sanitization: avoid extension duplication and key collisions
name.replaceAll('/', '_') + '.js'can produce double extensions (e.g.,app-service.js.js) and collisions (a/bvsa_b).- Prefer stripping a trailing
.jsand then appending.js. Optionally,encodeURIComponentor a stronger sanitizer to avoid collisions.Apply this diff:
- const fileName = `${templateName}/${name.replaceAll('/', '_')}.js`; + const sanitized = name.replaceAll('/', '_').replace(/\.js$/i, ''); + const fileName = `${templateName}/${sanitized}.js`;packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (1)
101-106: Avoid double sourceURL directives when content already has oneIf pre-generated chunks already include a
//# sourceURL=..., appending another is redundant and can confuse tooling. Only append when missing.Apply this diff:
- [ - jsContent, - '\n//# sourceURL=', - fileName, - ].join(''), + jsContent.includes('\n//# sourceURL=') + ? jsContent + : `${jsContent}\n//# sourceURL=${fileName}`,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.changeset/puny-pens-cry.md(1 hunks)packages/web-platform/web-constants/src/types/LynxModule.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(4 hunks)packages/web-platform/web-core/src/apis/LynxView.ts(1 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-tests/resources/web-core.main-thread.json(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(2 hunks)packages/web-platform/web-tests/tests/web-core.test.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/web-platform/web-tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place Playwright E2E tests in the web platform’s web-tests suite
Files:
packages/web-platform/web-tests/tests/react.spec.tspackages/web-platform/web-tests/tests/web-core.test.tspackages/web-platform/web-tests/resources/web-core.main-thread.json
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/puny-pens-cry.md
🧠 Learnings (10)
📚 Learning: 2025-09-09T12:38:10.450Z
Learnt from: CR
PR: lynx-family/lynx-stack#0
File: AGENTS.md:0-0
Timestamp: 2025-09-09T12:38:10.450Z
Learning: Applies to packages/web-platform/web-tests/** : Place Playwright E2E tests in the web platform’s web-tests suite
Applied to files:
packages/web-platform/web-tests/tests/web-core.test.ts
📚 Learning: 2025-08-27T11:36:36.415Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1589
File: packages/web-platform/web-tests/resources/web-core.main-thread.json:4-4
Timestamp: 2025-08-27T11:36:36.415Z
Learning: User PupilTong indicated that packages/web-platform/web-tests/resources/web-core.main-thread.json is considered a binary file, despite it appearing as a JSON text file with readable JavaScript code strings in the content.
Applied to files:
packages/web-platform/web-tests/resources/web-core.main-thread.json
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/web-platform/web-constants/src/types/LynxModule.ts
📚 Learning: 2025-08-27T11:37:38.587Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1589
File: packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts:36-49
Timestamp: 2025-08-27T11:37:38.587Z
Learning: In web worker script loading (loadScript function), the pattern of calling fetch() before importScripts() is intentional for caching benefits, not redundant networking. The fetch() ensures HTTP caching is utilized before the synchronous importScripts() call.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.tspackages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
🧬 Code graph analysis (5)
packages/web-platform/web-tests/tests/web-core.test.ts (1)
packages/web-platform/web-tests/tests/coverage-fixture.ts (1)
test(11-60)
packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
packages/web-platform/web-core/src/apis/LynxView.ts (2)
url(103-105)url(106-109)
packages/web-platform/web-constants/src/types/LynxModule.ts (2)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
requestAnimationFrame(95-97)cancelAnimationFrame(98-100)packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts (2)
requestAnimationFrame(21-23)cancelAnimationFrame(24-26)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (2)
packages/web-platform/web-constants/src/types/LynxModule.ts (2)
LynxTemplate(21-42)BTSChunkEntry(44-82)packages/web-platform/web-constants/src/types/NativeApp.ts (2)
NativeApp(105-236)BundleInitReturnObj(75-85)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (1)
createChunkLoading(12-175)
🔇 Additional comments (11)
packages/web-platform/web-core/src/apis/LynxView.ts (1)
459-475: Source map line offset fix looks correctSetting offset.line to 0 aligns with the updated mapping and test expectations.
packages/web-platform/web-tests/resources/web-core.main-thread.json (1)
9-13: Manifest entry for JSON is fineAdding "/json": "{}" is consistent with readScript('json') resolving manifest['/json'].
packages/web-platform/web-tests/tests/react.spec.ts (2)
1139-1141: Update expected line offset to 0 — OKMatches the corrected source mapping.
1263-1264: Update expected line offset to 0 — OKMatches the corrected source mapping.
.changeset/puny-pens-cry.md (1)
1-14: Changeset present and scoped correctlyPatch bumps for web-worker-runtime, web-constants, and web-core with concise notes. Good to ship.
packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
35-40: Pass encoded template name to generateTemplate — OKencodeURIComponent(url) is appropriate for stable sourceURL/file names.
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (3)
31-31: Good: centralized chunk loadingImporting createChunkLoading improves cohesion and removes bespoke loaders.
69-71: Delegating loaders/templateCache to chunk loader — LGTMWires readScript/loadScript/loadScriptAsync/templateCache from a single source.
92-95: Expose readScript/loadScript/loadScriptAsync on NativeApp — LGTMAPI surface matches updated types and tests.
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
51-53: LGTM: explicit sourceURLExplicit sourceURL improves DevTools debugging and aligns with the PR goals (breakpoints surviving reload).
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (1)
22-36: Synchronous XHR blocks the worker; confirm this is requiredBlocking XHR in workers can stall the background thread. If synchronous semantics are required for compatibility, okay; otherwise prefer async fetch path.
Do you need strict sync behavior for
loadScript? If not, I can help sketch an async-first loader with a compatibility shim for legacy call sites.Also applies to: 38-58
...s/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts
Show resolved
Hide resolved
...s/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts
Show resolved
Hide resolved
...s/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts
Show resolved
Hide resolved
* re-support chunk splitting * support lynx.requireModule with a json file * support lynx.requireModule, lynx.requireModuleAsync with a remote url * support to add a breakpoint in chrome after reloading the web page
PupilTong
force-pushed
the
p/hw/remove-wrapper
branch
from
400871a to
485e722
Compare
Web Explorer#5543 Bundle Size — 366.09KiB (+0.18%).89fa876(current) vs 6180e11 main#5529(baseline) Bundle metrics
|
| Current #5543 |
Baseline #5529 |
|
|---|---|---|
 Initial JS |
145.3KiB(-0.28%) |
145.71KiB |
 Initial CSS |
32KiB |
32KiB |
 Cache Invalidation |
47.65% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
220(+0.92%) |
218 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.36%(-0.3%) |
3.37% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 
1 regression
| Current #5543 |
Baseline #5529 |
|
|---|---|---|
| JS |
240.07KiB (+0.27%) |
239.42KiB |
| Other |
94.02KiB |
94.02KiB |
| CSS |
32KiB |
32KiB |
Bundle analysis report Branch PupilTong:p/hw/remove-wrapper Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#5546 Bundle Size — 237.56KiB (0%).89fa876(current) vs 6180e11 main#5534(baseline) Bundle metrics
|
| Current #5546 |
Baseline #5534 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
166 |
166 |
| Duplicate Modules |
68 |
68 |
| Duplicate Code |
46.81% |
46.81% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5546 |
Baseline #5534 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.8KiB |
91.8KiB |
Bundle analysis report Branch PupilTong:p/hw/remove-wrapper Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (1)
155-161: Avoid double-encoding entryName in sourceURL annotationIf callers already pass an encoded entryName (common), encoding again produces unreadable paths and complicates breakpoint mapping.
Apply this diff in both places:
- `${encodeURIComponent(entryName)}/${sourceURL}`, + `${entryName}/${sourceURL}`,Also applies to: 166-169
.changeset/puny-pens-cry.md (1)
1-13: Version bumps likely need to be minor (or major), not patchNew features + public API surface changes (e.g., NativeApp additions; generateTemplate signature change unless reverted) warrant at least minor. If keeping the breaking signature change, mark major and call it out.
Proposed bump levels:
-"@lynx-js/web-worker-runtime": patch -"@lynx-js/web-constants": patch -"@lynx-js/web-core": patch +"@lynx-js/web-worker-runtime": minor +"@lynx-js/web-constants": minor +"@lynx-js/web-core": minorIf you keep generateTemplate’s breaking signature, change web-constants to major and add a “BREAKING CHANGE” note explaining the new required parameter.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.changeset/puny-pens-cry.md(1 hunks)packages/web-platform/web-constants/src/types/LynxModule.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(4 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-tests/resources/web-core.main-thread.json(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(0 hunks)packages/web-platform/web-tests/tests/web-core.test.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(3 hunks)
💤 Files with no reviewable changes (1)
- packages/web-platform/web-tests/tests/react.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web-platform/web-tests/resources/web-core.main-thread.json
- packages/web-platform/web-tests/tests/web-core.test.ts
- packages/web-platform/web-core/src/utils/loadTemplate.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/puny-pens-cry.md
🧠 Learnings (8)
📚 Learning: 2025-08-21T08:46:54.494Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Applied to files:
packages/web-platform/web-constants/src/types/LynxModule.ts
📚 Learning: 2025-08-27T11:37:38.587Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1589
File: packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts:36-49
Timestamp: 2025-08-27T11:37:38.587Z
Learning: In web worker script loading (loadScript function), the pattern of calling fetch() before importScripts() is intentional for caching benefits, not redundant networking. The fetch() ensures HTTP caching is utilized before the synchronous importScripts() call.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.tspackages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/puny-pens-cry.md
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
🧬 Code graph analysis (3)
packages/web-platform/web-constants/src/types/LynxModule.ts (2)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
requestAnimationFrame(95-97)cancelAnimationFrame(98-100)packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts (2)
requestAnimationFrame(21-23)cancelAnimationFrame(24-26)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (2)
packages/web-platform/web-constants/src/types/LynxModule.ts (2)
LynxTemplate(21-42)BTSChunkEntry(44-82)packages/web-platform/web-constants/src/types/NativeApp.ts (2)
NativeApp(105-236)BundleInitReturnObj(75-85)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (1)
createChunkLoading(12-175)
🔇 Additional comments (9)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (3)
31-31: Good move: centralize chunk loading via createChunkLoadingImporting the loader reduces duplication and side effects in this module.
69-71: Wiring looks correctDestructuring readScript/loadScript/loadScriptAsync/templateCache from the loader aligns with NativeApp API needs.
92-94: Expose script loading APIs on NativeAppAdding readScript/loadScript/loadScriptAsync to NativeApp is consistent with the new loader flow.
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
44-82: Type addition aligns with new chunk loaderBTSChunkEntry signature matches the injected param list used by createChunkLoading. Using unknown keeps it permissive.
packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
18-22: Upgrader filter is sensibleFiltering lepusCode to string entries avoids surprising non-string payloads downstream.
27-33: SourceURL injection via fileName is correctPassing fileName into generateModuleContent and appending //# sourceURL improves breakpoint persistence after reloads.
Also applies to: 51-53
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (3)
26-36: Normalize manifest key and improve XHR fallback (status 0)Avoid double slashes and support status 0 with non-empty responseText (common in worker/file contexts).
Apply this diff:
- const jsContentInTemplate = templateCache.get(entryName!) - ?.manifest[`/${sourceURL}`]; + const manifestKey = sourceURL.startsWith('/') ? sourceURL : `/${sourceURL}`; + const jsContentInTemplate = templateCache.get(entryName!) + ?.manifest[manifestKey]; @@ - if (xhr.status === 200) { + if (xhr.status === 200 || (xhr.status === 0 && xhr.responseText)) { return xhr.responseText; }
42-45: Normalize manifest key in async path tooMirror the sync normalization for consistent behavior.
Apply this diff:
- const jsContentInTemplate = templateCache.get(entryName!) - ?.manifest[`/${sourceURL}`]; + const manifestKey = sourceURL.startsWith('/') ? sourceURL : `/${sourceURL}`; + const jsContentInTemplate = templateCache.get(entryName!) + ?.manifest[manifestKey];
162-171: Propagate errors to callback in loadScriptAsyncCurrently, failures never invoke callback, causing unhandled rejections and violating the contract.
Apply this diff:
- readScriptAsync(sourceURL, entryName).then((jsContent) => { - callback( - null, - createBundleInitReturnObj( - jsContent, - `${encodeURIComponent(entryName)}/${sourceURL}`, - ), - ); - }); + readScriptAsync(sourceURL, entryName) + .then((jsContent) => { + callback( + null, + createBundleInitReturnObj( + jsContent, + `${encodeURIComponent(entryName)}/${sourceURL}`, + ), + ); + }) + .catch((err) => { + const message = + err && typeof (err as any).message === 'string' + ? (err as any).message + : String(err); + callback(message); + });
packages/web-platform/web-constants/src/utils/generateTemplate.ts
Outdated
Show resolved
Hide resolved
CodSpeed Performance ReportMerging #1834 will degrade performances by 9.31%Comparing 🎉 Hooray!
|
| Benchmark | BASE |
HEAD |
Change | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
44.4 ms | 41.5 ms | +6.87% |
| ❌ | basic-performance-div-100 |
5.9 ms | 6.5 ms | -9.31% |
| ⚡ | basic-performance-large-css |
12.7 ms | 11.9 ms | +6.72% |
| ⚡ | basic-performance-scroll-view-100 |
9.6 ms | 8.9 ms | +7.77% |
| ⚡ | basic-performance-small-css |
7.4 ms | 6.9 ms | +7.66% |
Footnotes
-
3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (4)
45-57: Avoid the explicit Promise constructor; simplify with async/awaitCleaner and less nesting; behavior unchanged.
Apply this diff:
- return new Promise((resolve, reject) => { - fetch(sourceURL).then((response) => { - if (response.ok) { - response.text().then((text) => resolve(text), reject); - } else { - reject( - new Error( - `Failed to load ${sourceURL}, status: ${response.status}`, - ), - ); - } - }, reject); - }); + const response = await fetch(sourceURL); + if (!response.ok) { + throw new Error(`Failed to load ${sourceURL}, status: ${response.status}`); + } + return await response.text();
159-162: Sanitize fileName for sourceURL annotationPrevent malformed or multi-line sourceURL values from breaking devtools mapping.
Apply this diff:
- return createBundleInitReturnObj( - jsContent, - `${encodeURIComponent(entryName)}/${sourceURL}`, - ); + return createBundleInitReturnObj( + jsContent, + encodeURI(`${encodeURIComponent(entryName)}/${String(sourceURL)}`.replace(/[\r\n]/g, '')), + );
168-171: Sanitize fileName for sourceURL annotation (async path)Mirror the sync path sanitization.
Apply this diff:
- createBundleInitReturnObj( - jsContent, - `${encodeURIComponent(entryName)}/${sourceURL}`, - ), + createBundleInitReturnObj( + jsContent, + encodeURI(`${encodeURIComponent(entryName)}/${String(sourceURL)}`.replace(/[\r\n]/g, '')), + ),
164-164: Drop unnecessary async on loadScriptAsyncThe function returns void per NativeApp type; removing async avoids returning an unused Promise and clarifies intent.
Apply this diff:
- loadScriptAsync: async (sourceURL, callback, entryName = '__Card__') => { + loadScriptAsync: (sourceURL, callback, entryName = '__Card__') => {Please confirm no call sites rely on
await nativeApp.loadScriptAsync(...).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web-platform/web-constants/src/types/LynxModule.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web-platform/web-constants/src/types/LynxModule.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-25T14:03:25.547Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1834
File: packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts:162-171
Timestamp: 2025-09-25T14:03:25.547Z
Learning: In the lynx-stack codebase, for loadScriptAsync implementations in createChunkLoading.ts, unhandled promise rejections from readScriptAsync are intentionally not caught - the caller is expected to handle errors rather than the loadScriptAsync method itself invoking the callback with error messages.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts
📚 Learning: 2025-08-27T11:37:38.587Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1589
File: packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts:36-49
Timestamp: 2025-08-27T11:37:38.587Z
Learning: In web worker script loading (loadScript function), the pattern of calling fetch() before importScripts() is intentional for caching benefits, not redundant networking. The fetch() ensures HTTP caching is utilized before the synchronous importScripts() call.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts
🧬 Code graph analysis (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (2)
packages/web-platform/web-constants/src/types/LynxModule.ts (2)
LynxTemplate(21-42)BTSChunkEntry(44-83)packages/web-platform/web-constants/src/types/NativeApp.ts (2)
NativeApp(105-236)BundleInitReturnObj(75-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (4)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts (4)
26-27: Normalize manifest key to avoid double slashesAvoid
//...when callers already include a leading slash.Apply this diff:
- const jsContentInTemplate = templateCache.get(entryName!) - ?.manifest[`/${sourceURL}`]; + const manifestKey = sourceURL.startsWith('/') ? sourceURL : `/${sourceURL}`; + const jsContentInTemplate = templateCache.get(entryName!) + ?.manifest[manifestKey];
32-35: Handle XHR status 0 with non-empty responseText (worker/file:// contexts)Improves robustness in worker and file URLs.
Apply this diff:
- if (xhr.status === 200) { - return xhr.responseText; - } + if (xhr.status === 200 || (xhr.status === 0 && xhr.responseText)) { + return xhr.responseText; + }
42-43: Normalize manifest key in async path tooKeep manifest lookup behavior consistent across sync/async.
Apply this diff:
- const jsContentInTemplate = templateCache.get(entryName!) - ?.manifest[`/${sourceURL}`]; + const manifestKey = sourceURL.startsWith('/') ? sourceURL : `/${sourceURL}`; + const jsContentInTemplate = templateCache.get(entryName!) + ?.manifest[manifestKey];
59-154: init wrapper matches BTSChunkEntry signatureParameter order and bindings align with the declared BTSChunkEntry; sourceURL is correctly annotated for devtools.
For remote URLs requiring credentials, is the intended behavior to exclude cookies/credentials (default fetch same-origin)? If not, you may want to add fetch credentials/options.
...s/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts
Show resolved
Hide resolved
...s/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createChunkLoading.ts
Show resolved
Hide resolved
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/[email protected] ### Patch Changes - Add `event.stopPropagation` and `event.stopImmediatePropagation` in MTS, to help with event propagation control ([#1835](#1835)) ```tsx function App() { function handleInnerTap(event: MainThread.TouchEvent) { "main thread"; event.stopPropagation(); // Or stop immediate propagation with // event.stopImmediatePropagation(); } // OuterTap will not be triggered return ( <view main-thread:bindtap={handleOuterTap}> <view main-thread:bindtap={handleInnerTap}> <text>Hello, world</text> </view> </view> ); } ``` Note, if this feature is used in [Lazy Loading Standalone Project](https://lynxjs.org/react/code-splitting.html#lazy-loading-standalone-project), both the Producer and the Consumer should update to latest version of `@lynx-js/react` to make sure the feature is available. - Fix the "ReferenceError: Node is not defined" error. ([#1850](#1850)) This error would happen when upgrading to `@testing-library/jest-dom` [v6.9.0](https://github.com/testing-library/jest-dom/releases/tag/v6.9.0). - fix: optimize main thread event error message ([#1838](#1838)) ## @lynx-js/[email protected] ### Patch Changes - Bump Rsbuild v1.5.13 with Rspack v1.5.8. ([#1849](#1849)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`19f823a`](19f823a)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix the "ReferenceError: Node is not defined" error. ([#1850](#1850)) This error would happen when upgrading to `@testing-library/jest-dom` [v6.9.0](https://github.com/testing-library/jest-dom/releases/tag/v6.9.0). ## @lynx-js/[email protected] ### Patch Changes - feat: support load bts chunk from remote address ([#1834](#1834)) - re-support chunk splitting - support lynx.requireModule with a json file - support lynx.requireModule, lynx.requireModuleAsync with a remote url - support to add a breakpoint in chrome after reloading the web page - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support load bts chunk from remote address ([#1834](#1834)) - re-support chunk splitting - support lynx.requireModule with a json file - support lynx.requireModule, lynx.requireModuleAsync with a remote url - support to add a breakpoint in chrome after reloading the web page - Updated dependencies \[[`a35a245`](a35a245)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`a35a245`](a35a245)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support load bts chunk from remote address ([#1834](#1834)) - re-support chunk splitting - support lynx.requireModule with a json file - support lynx.requireModule, lynx.requireModuleAsync with a remote url - support to add a breakpoint in chrome after reloading the web page - Updated dependencies \[[`a35a245`](a35a245)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Avoid generating `.css.hot-update.json` when HMR is disabled. ([#1811](#1811)) ## [email protected] ## @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
#1806
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Checklist