feat(web): implement lazy bundle#1235
Conversation
🦋 Changeset detectedLatest commit: 7a46c1a The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 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 |
Web Explorer#5150 Bundle Size — 364.39KiB (+0.44%).7a46c1a(current) vs 5ad38e6 main#5149(baseline) Bundle metrics
|
| Current #5150 |
Baseline #5149 |
|
|---|---|---|
 Initial JS |
145.41KiB(+0.14%) |
145.21KiB |
 Initial CSS |
31.89KiB |
31.89KiB |
 Cache Invalidation |
64.42% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
220(+0.46%) |
219 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.37%(+0.9%) |
3.34% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #5150 |
Baseline #5149 |
|
|---|---|---|
| JS |
238.48KiB (+0.67%) |
236.88KiB |
| Other |
94.02KiB |
94.02KiB |
| CSS |
31.89KiB |
31.89KiB |
Bundle analysis report Branch Sherry-hue:feat/web-dynamic-comp... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#5157 Bundle Size — 238.9KiB (0%).7a46c1a(current) vs 5ad38e6 main#5156(baseline) Bundle metrics
|
| Current #5157 |
Baseline #5156 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
163 |
163 |
| Duplicate Modules |
67 |
67 |
| Duplicate Code |
46.79% |
46.79% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5157 |
Baseline #5156 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
93.14KiB |
93.14KiB |
Bundle analysis report Branch Sherry-hue:feat/web-dynamic-comp... Project dashboard
Generated by RelativeCI Documentation Report issue
web bundle adds loadDynamicComponent, nativeGlobal export, L4W needs this to load dynamic component doc: lynx-family/lynx-stack#1235
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
3 times, most recently
from
6952294 to
be717b2
Compare
📝 WalkthroughWalkthroughAdds lazy-bundle support: new RPC endpoints and template-loader types, appType-aware template generation, cross-thread template loading and queryComponent flow, main-thread __QueryComponent API, background template cache, entry-scoped style updates, loader/timing instrumentation, and many React lazy-component tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
158-161: Remove dead codeThe conditional block contains a statement that has no effect. This appears to be leftover from refactoring.
Remove the dead code:
- if (!templateName) { - createJsModuleUrl; - }
🧹 Nitpick comments (7)
packages/web-platform/web-tests/tests/react/config-dynamic-component/index.jsx (1)
8-10: Consider extracting the color toggle logic for better readability.The inline ternary operator works but could be extracted for improved readability and potential reusability.
- const handleTap = () => { - setColor(color === 'green' ? 'pink' : 'green'); - }; + const handleTap = () => { + const nextColor = color === 'green' ? 'pink' : 'green'; + setColor(nextColor); + };packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
346-351: Interface extensions support dynamic component functionality.The new methods integrate well with the existing interface. Consider improving type safety by replacing
anywith more specific types for theresultparameter in__QueryComponent.- __QueryComponent: (url: string, result: any) => any; + __QueryComponent: (url: string, result: { __hasReady: boolean } | { code: number; detail: { schema: string } }) => any;packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx (1)
19-27: Consider adding error boundary for robust testing.The component structure is clean and follows React best practices. Consider wrapping the Suspense component in an error boundary to handle potential loading failures gracefully.
+ import React from 'react'; + + class ErrorBoundary extends React.Component { + constructor(props) { + super(props); + this.state = { hasError: false }; + } + + static getDerivedStateFromError(error) { + return { hasError: true }; + } + + render() { + if (this.state.hasError) { + return <text>Error loading component</text>; + } + return this.props.children; + } + } export default function App() { return ( <view> + <ErrorBoundary> <Suspense fallback={<text>Loading...</text>}> <LazyComponent /> </Suspense> + </ErrorBoundary> </view> ); }packages/web-platform/web-constants/src/utils/loadTemplate.ts (1)
36-36: Useful cache accessor function.The
getTemplatefunction provides clean read-only access to the template cache. Consider adding JSDoc documentation to clarify the return type and behavior when a template is not found.+ /** + * Retrieves a cached template by URL + * @param url - The template URL + * @returns The cached template or undefined if not found + */ export const getTemplate = (url: string) => templateCache[url];packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts (2)
20-29: Consider using filter for cleaner callback management.The current while loop with splice works but could be more idiomatic.
Consider this cleaner approach:
- let i = 0; - while (i < callbacks.length) { - const { source: rawSource, callback } = callbacks[i]!; - if (source === rawSource) { - callback(template); - callbacks.splice(i, 1); - } else { - i++; - } - } + const matchingCallbacks = callbacks.filter(({ source: rawSource }) => source === rawSource); + matchingCallbacks.forEach(({ callback }) => callback(template)); + callbacks.splice(0, callbacks.length, ...callbacks.filter(({ source: rawSource }) => source !== rawSource));
38-44: Fix grammatical errors in comments.The comments contain unclear phrasing.
- // If the resource has been passed from QueryComponentResource but not from mts, callback directly. + // If the resource has already been cached from QueryComponentResource, invoke callback directly. if (templateCache[source]) { callback(templateCache[source]); return; } - // If the resource has not been passed from QueryComponentResource but not from mts, push callbacks and then it will be triggered through registerHandler + // If the resource has not been cached yet, queue the callback to be triggered when received via registerHandler callbacks.push({ source, callback });packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
125-132: Simplify manifest URL resolution logic.The ternary expression for manifest URL resolution could be clearer.
loadScript: (sourceURL: string, entryName?: string) => { - const manifestUrl = (entryName && entryName !== '__Card__') - ? templateCache[entryName]?.manifest[`/${sourceURL}`] - : template.manifest[`/${sourceURL}`]; + let manifestUrl; + if (entryName && entryName !== '__Card__') { + manifestUrl = templateCache[entryName]?.manifest[`/${sourceURL}`]; + } else { + manifestUrl = template.manifest[`/${sourceURL}`]; + } if (manifestUrl) sourceURL = manifestUrl; importScripts(sourceURL); return createBundleInitReturnObj(); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.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-constants/src/utils/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/loadTemplate.ts(3 hunks)packages/web-platform/web-core-server/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts(1 hunks)packages/web-platform/web-rsbuild-plugin/src/index.ts(1 hunks)packages/web-platform/web-tests/rspack.config.js(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic.config.ts(1 hunks)packages/web-platform/web-tests/tests/react/commonConfig.ts(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component/rspeedy.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(6 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
packages/web-platform/web-core-server/src/utils/loadTemplate.ts (1)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
generateTemplate(200-283)
packages/web-platform/web-constants/src/endpoints.ts (3)
packages/web-platform/web-worker-rpc/src/index.ts (1)
createRpcEndpoint(10-10)packages/web-platform/web-worker-rpc/src/RpcEndpoint.ts (1)
createRpcEndpoint(128-142)packages/web-platform/web-core/src/index.ts (1)
LynxTemplate(6-6)
packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
packages/web-platform/web-constants/src/utils/loadTemplate.ts (1)
loadTemplate(10-34)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (3)
packages/web-platform/web-core/src/index.ts (1)
LynxTemplate(6-6)packages/web-platform/web-constants/src/utils/index.ts (1)
generateTemplate(7-7)packages/web-platform/web-constants/src/constants.ts (1)
globalMuteableVars(29-34)
packages/web-platform/web-tests/tests/react/config-dynamic-component/rspeedy.config.ts (3)
packages/web-platform/web-tests/rspack.config.js (2)
__filename(10-10)__dirname(11-11)packages/web-platform/web-tests/tests/react/commonConfig.ts (1)
commonConfig(10-43)packages/rspeedy/core/src/index.ts (1)
defineConfig(33-33)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts (2)
packages/web-platform/web-core/src/index.ts (1)
LynxTemplate(6-6)packages/web-platform/web-constants/src/endpoints.ts (1)
queryComponentResourceEndpoint(232-235)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (6)
packages/web-platform/web-worker-rpc/src/TypeUtils.ts (1)
RpcCallType(9-18)packages/web-platform/web-constants/src/endpoints.ts (1)
queryComponentResourceEndpoint(232-235)packages/web-platform/web-constants/src/utils/index.ts (1)
loadTemplate(8-8)packages/web-platform/web-constants/src/utils/loadTemplate.ts (1)
loadTemplate(10-34)packages/web-platform/web-core/src/index.ts (1)
LynxTemplate(6-6)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts (1)
getLepusEntries(7-29)
🔇 Additional comments (19)
packages/web-platform/web-rsbuild-plugin/src/index.ts (1)
11-14: LGTM: Clean API expansion with proper export syntax.The addition of
getNativeModulesPathRuleto the exported functions follows standard ES module patterns and improves the module's public API organization. This coordinated change with the corresponding import update demonstrates good refactoring practices.packages/web-platform/web-tests/rspack.config.js (1)
9-9: LGTM: Improved import pattern following proper API usage.The change from deep import to package-level import is a good refactoring that:
- Uses the newly exposed public API instead of internal file paths
- Improves maintainability by avoiding brittle deep imports
- Aligns with the export expansion in the main module
packages/web-platform/web-tests/tests/react/basic.config.ts (1)
25-27: LGTM! Proper environment variable configuration.The use of
JSON.stringify()with a default value follows the correct pattern for webpack's DefinePlugin and ensures consistent behavior across different environments.packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts (1)
48-57: LGTM! Proper integration of dynamic component loading.The destructuring and assignment correctly expose the
loadDynamicComponentfunction to the global scope, enabling dynamic component loading capabilities in the background thread context.packages/web-platform/web-constants/src/utils/index.ts (1)
8-8: LGTM! Correct export statement.The export statement properly exposes the template loading utilities following the established pattern in the file.
packages/web-platform/web-constants/src/endpoints.ts (2)
14-18: LGTM! Proper type import.The addition of
LynxTemplateto the import statement correctly supports the new RPC endpoint definition.
232-235: LGTM! Well-defined RPC endpoint.The
queryComponentResourceEndpointis properly defined with appropriate type parameters and follows the established pattern of other endpoints in the file. The signature[string, LynxTemplate], voidwithfalse, falseparameters correctly indicates an asynchronous endpoint with no return value.packages/web-platform/web-core-server/src/utils/loadTemplate.ts (2)
27-32: LGTM: Function signature refactoring aligns with updatedgenerateTemplateAPI.The change from positional arguments to a named options object improves code clarity and maintainability. The explicit
isDynamicComponent: falseparameter correctly indicates this is for static component loading.
27-32: Verify missinggetTemplatefunction mentioned in AI summary.The AI summary mentions a new exported
getTemplatefunction for retrieving cached templates by URL, but this function is not visible in the annotated code. Please confirm if this function should be added or if the summary is incorrect.#!/bin/bash # Description: Check if getTemplate function exists in the file # Expected: Function should be present if mentioned in summary rg -A 10 "export.*getTemplate" packages/web-platform/web-core-server/src/utils/loadTemplate.tsLikely an incorrect or invalid review comment.
packages/web-platform/web-tests/tests/react/config-dynamic-component/index.jsx (1)
1-30: LGTM: Well-structured React test component for dynamic functionality.The component correctly demonstrates dynamic behavior through state management and event handling. The implementation follows React best practices with proper use of
useStateand event handlers.packages/web-platform/web-core/src/uiThread/startUIThread.ts (2)
17-17: LGTM: Import consolidation improves code organization.The change from relative import to named import from
@lynx-js/web-constantsaligns with the package restructuring and centralizes utility imports.
90-99: LGTM: Function signature update correctly aligns with newloadTemplateAPI.The addition of the
isDynamicComponent: falseparameter correctly indicates this is for static component loading in the UI thread context. The promise handling refactoring maintains the same logical flow while improving readability.packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts (1)
51-61: Clean delegation pattern implementation.The
QueryComponentmethod properly delegates to the native app's component querying functionality. Consider adding JSDoc documentation to clarify the callback parameters and expected behavior.packages/web-platform/web-constants/src/types/NativeApp.ts (1)
199-208: Consistent interface additions for dynamic component support.The new
queryComponentmethod andttproperty integrate cleanly with the existing interface. The callback type structure properly handles both success and error cases.packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx (1)
7-17: Well-structured dynamic component loading test.The test effectively demonstrates the dynamic component loading functionality using React's lazy and Suspense. The environment variable usage for port configuration is appropriate for testing scenarios.
packages/web-platform/web-constants/src/utils/loadTemplate.ts (1)
10-27: Improved API design with options object pattern.The refactoring to use an options object for
generateTemplateimproves maintainability and readability. TheisDynamicComponentparameter properly extends the functionality for dynamic component handling.packages/web-platform/web-tests/tests/react/config-dynamic-component/rspeedy.config.ts (1)
1-21: LGTM!The configuration correctly sets up the dynamic component test case with lazy bundle support.
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
27-33: Clean refactoring to support dynamic component loading.The changes effectively centralize module loading logic and add the necessary RPC infrastructure for querying component resources.
Also applies to: 65-67, 85-88, 103-103, 197-199
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
146-148: Clarify usage of globDynamicComponentEntry variableThe
globDynamicComponentEntryvariable is declared in the generated module content but never used. Is this intentional for future use or should it be removed?If this variable should be used somewhere in the generated code, please update the implementation. Otherwise, consider removing it to avoid confusion:
- isDynamicComponent - ? `;var globDynamicComponentEntry = '${source}';` - : ';var globDynamicComponentEntry = \'__Card__\';',
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
be717b2 to
b28afb5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/fluffy-masks-mate.md (1)
9-9: Fix grammar in changeset description.The description has a grammatical error. "supports" should be "support" for proper subject-verb agreement.
Apply this diff to fix the grammar:
-feat: supports dynamic component, usage is synchronized with the native. +feat: support dynamic component, usage is synchronized with the native.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.changeset/fluffy-masks-mate.md(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.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-constants/src/utils/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/loadTemplate.ts(3 hunks)packages/web-platform/web-core-server/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic.config.ts(1 hunks)packages/web-platform/web-tests/tests/react/commonConfig.ts(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component/rspeedy.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(6 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (19)
- packages/web-platform/web-tests/tests/react/commonConfig.ts
- packages/web-platform/web-constants/src/utils/index.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-tests/tests/react/basic.config.ts
- packages/web-platform/web-core-server/src/utils/loadTemplate.ts
- packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-tests/tests/react/config-dynamic-component/index.jsx
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts
- packages/web-platform/web-tests/tests/react/config-dynamic-component/rspeedy.config.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts
- packages/web-platform/web-constants/src/utils/loadTemplate.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
packages/web-platform/web-core/src/index.ts (1)
LynxTemplate(6-6)packages/web-platform/web-constants/src/utils/index.ts (1)
generateTemplate(7-7)
🪛 LanguageTool
.changeset/fluffy-masks-mate.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...ore": patch --- feat: supports dynamic component, usage is synchronized with the native. ...
(QB_NEW_EN_OTHER)
🪛 GitHub Check: code-style-check
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
[failure] 54-54:
Property 'loadDynamicComponent' does not exist on type 'typeof import("/home/runner/_work/lynx-stack/lynx-stack/node_modules/.pnpm/@lynx-js+lynx-core@0.1.2/node_modules/@lynx-js/lynx-core/lynx_core")'.
[failure] 53-53:
Property 'nativeGlobal' does not exist on type 'typeof import("/home/runner/_work/lynx-stack/lynx-stack/node_modules/.pnpm/@lynx-js+lynx-core@0.1.2/node_modules/@lynx-js/lynx-core/lynx_core")'.
⏰ 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: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (4)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (4)
84-133: Approve the function signature refactor.The refactor from multiple parameters to options objects improves API maintainability and extensibility. The overloaded signatures properly handle both sync and async
createJsModuleUrlfunctions.
68-68: Verify the new__QueryComponentinjection variable.The
__QueryComponentvariable is added to the main thread inject vars, which aligns with the dynamic component functionality mentioned in the AI summary.
183-283: Approve the consistent parameter passing.The
generateTemplatefunction properly passes all new parameters (isDynamicComponent,isMain,source) to thegenerateJavascriptUrlfunction calls, maintaining consistency between lepusCode and manifest generation.
146-148: TheglobDynamicComponentEntryinitialization logic is correctAfter reviewing usages across the codebase—from generateTemplate.ts, through the runtime-wrapper and Webpack/Rspack plugins, to various test suites—the dynamic case (setting it to
source) and the static fallback ('__Card__') are applied consistently and behave as expected. No changes are needed here.
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
b28afb5 to
559879b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/fluffy-masks-mate.md (1)
9-9: Clarify & polish the changeset summaryWording feels awkward (“supports dynamic component”) and the comma splice weakens readability. Consider pluralising and explicitly naming the native target.
-feat: supports dynamic component, usage is synchronized with the native. +feat: support dynamic components; usage is synchronized with the native implementation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.changeset/fluffy-masks-mate.md(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.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-constants/src/utils/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/loadTemplate.ts(3 hunks)packages/web-platform/web-core-server/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic.config.ts(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component/rspeedy.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(6 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
- packages/web-platform/web-constants/src/utils/index.ts
- packages/web-platform/web-tests/tests/react/basic.config.ts
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-core-server/src/utils/loadTemplate.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-tests/tests/react/config-dynamic-component/index.jsx
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts
- packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx
- packages/web-platform/web-constants/src/utils/loadTemplate.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-tests/tests/react/config-dynamic-component/rspeedy.config.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts
- packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-constants/src/utils/generateTemplate.ts
🧰 Additional context used
🪛 LanguageTool
.changeset/fluffy-masks-mate.md
[grammar] ~9-~9: There might be a mistake here.
Context: ...ore": patch --- feat: supports dynamic component, usage is synchronized with the native. ...
(QB_NEW_EN_OTHER)
⏰ 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). (5)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: CodeQL Analyze (javascript-typescript)
- GitHub Check: zizmor
- GitHub Check: CodeQL Analyze (actions)
web bundle adds loadDynamicComponent, nativeGlobal export, L4W needs this to load dynamic component doc: lynx-family/lynx-stack#1235
web bundle adds loadDynamicComponent, nativeGlobal export, L4W needs this to load dynamic component doc: lynx-family/lynx-stack#1235 (cherry picked from commit f835ad585ffdc2fbcca79d6ca6e8c79d2edd68a4)
web bundle adds loadDynamicComponent, nativeGlobal export, L4W needs this to load dynamic component doc: lynx-family/lynx-stack#1235
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
559879b to
ccc44bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/web-platform/web-tests/tests/react/config-dynamic-component-use-effect/index.jsx (1)
6-16: Consider optimizing the useEffect pattern for cleaner test logic.While the current implementation works, the immediate state change in useEffect on mount could be simplified for better test clarity and performance.
Consider this alternative approach that achieves the same result more directly:
export default function App() { - const [color, setColor] = useState('green'); - useEffect(() => { - setColor('red'); - }, []); + const [color, setColor] = useState('red');However, if this test specifically validates that useEffect executes correctly in dynamic components (which seems likely given the file name), the current implementation is appropriate for that testing purpose.
packages/web-platform/web-tests/tests/react/basic-dynamic-multi/index.jsx (2)
22-26: Consider adding a fallback prop for consistency.The
Suspensecomponent is missing afallbackprop, which differs from other test files likebasic-dynamic-fail/index.jsxthat include fallback UI. While this might be intentional for testing purposes, adding a fallback would provide better consistency across tests and help identify loading states.return ( - <Suspense> + <Suspense fallback={<text id='fallback'>Loading...</text>}> <LazyComponent /> <LazyComponent /> </Suspense> );
9-18: Simplify the lazy component function.The function syntax can be simplified to match the pattern used in other test files for better consistency and readability.
const LazyComponent = lazy( - () => { - return import( - importPath, - { - with: { type: 'component' }, - } - ); - }, + () => + import( + importPath, + { + with: { type: 'component' }, + } + ), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.changeset/fluffy-masks-mate.md(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.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-constants/src/utils/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/loadTemplate.ts(3 hunks)packages/web-platform/web-core-server/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic-fail/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic.config.ts(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component-use-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(6 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx
- packages/web-platform/web-tests/tests/react/config-dynamic-component.config.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/web-platform/web-tests/tests/react/basic.config.ts
- packages/web-platform/web-constants/src/utils/index.ts
- .changeset/fluffy-masks-mate.md
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts
- packages/web-platform/web-core-server/src/utils/loadTemplate.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-constants/src/utils/loadTemplate.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-constants/src/utils/generateTemplate.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/web-platform/web-tests/tests/react/basic-dynamic-multi/index.jsx (3)
packages/web-platform/web-tests/tests/react/basic-dynamic-fail/index.jsx (3)
importPath(8-9)LazyComponent(10-18)App(20-28)packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx (3)
importPath(7-8)LazyComponent(9-17)App(19-27)packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx (1)
App(6-27)
packages/web-platform/web-tests/tests/react/basic-dynamic-fail/index.jsx (2)
packages/web-platform/web-tests/tests/react/basic-dynamic-multi/index.jsx (3)
importPath(7-8)LazyComponent(9-18)App(20-27)packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx (3)
importPath(7-8)LazyComponent(9-17)App(19-27)
🔇 Additional comments (3)
packages/web-platform/web-tests/tests/react/config-dynamic-component-use-effect/index.jsx (2)
4-4: LGTM! Imports are appropriate for the test.The imports correctly include the necessary React hooks and root renderer from the Lynx React package for this dynamic component test.
18-18: LGTM! Proper component rendering.The root.render call correctly renders the App component, following the expected pattern for the Lynx React framework.
packages/web-platform/web-tests/tests/react/basic-dynamic-fail/index.jsx (1)
1-31: LGTM! Well-structured failure test.This test file correctly implements failure scenario testing for dynamic component loading. The deliberate use of a nonexistent URL, proper fallback UI, and clear commenting make this an effective test case for error handling.
colinaaa
left a comment
colinaaa
left a comment
There was a problem hiding this comment.
Please avoid using term "dynamic component". Use "lazy bundle" instead.
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
2 times, most recently
from
914df87 to
701d4de
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
118-118: Clarify intentional early return for dynamic ESM modulesThis emits an early return for dynamic components on the main thread to bypass the final return of module.exports. Make this explicit to future readers.
Apply this diff:
- (isDynamicComponent && isESM) ? `return ${content}` : content, + // For dynamic components evaluated on the main thread (ESM), return the component + // directly to skip the final "return module.exports" fall-through. + (isDynamicComponent && isESM) ? `return ${content}` : content,packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (1)
835-852: Handle errors and remove incorrect type assertion in __QueryComponentTwo issues:
- Missing error reporting can mask failures in loadTemplate/getLepusEntries/evaluation.
- Type cast as (schema: string) => void is inconsistent with actual usage; entry!(mtsGlobalThis) returns whatever the module exports.
Apply this diff:
- __QueryComponent: (source: string) => { - loadTemplate(source, true).then( - async (template: LynxTemplate) => { - callbacks.queryComponentResource(source, template); - const { lepusCode } = template; - const { entry } = await getLepusEntries( - lepusCode, - moduleCache, - ); - const lepusVal = entry!(mtsGlobalThis) as (schema: string) => void; - mtsGlobalThis.globalThis?.processEvalResult?.(lepusVal, source); - }, - ).catch(() => { - callbacks.queryComponentResource(source, undefined); - }); - - return undefined; - }, + __QueryComponent: (source: string) => { + loadTemplate(source, true) + .then(async (template: LynxTemplate) => { + try { + callbacks.queryComponentResource(source, template); + const { lepusCode } = template; + const { entry } = await getLepusEntries(lepusCode, moduleCache); + const lepusVal = entry!(mtsGlobalThis); + mtsGlobalThis.globalThis?.processEvalResult?.(lepusVal, source); + } catch (error) { + callbacks._ReportError(error, 'QueryComponent', release); + callbacks.queryComponentResource(source, undefined); + } + }) + .catch((error) => { + callbacks._ReportError(error, 'QueryComponent', release); + callbacks.queryComponentResource(source, undefined); + }); + return undefined; + },
🧹 Nitpick comments (7)
packages/web-platform/web-tests/tests/react/basic-dynamic-effect/index.jsx (1)
1-1: Update header year to 2025 to stay consistent with new test files.Other newly added test files use 2025. Align the header for consistency.
-// Copyright 2023 The Lynx Authors. All rights reserved. +// Copyright 2025 The Lynx Authors. All rights reserved.packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx (1)
8-10: Use functional state update to prevent stale closures on rapid taps.Relies on prior state; functional update is more robust under concurrent/batched updates.
- const handleTap = () => { - setColor(color === 'green' ? 'pink' : 'green'); - }; + const handleTap = () => { + setColor(prev => (prev === 'green' ? 'pink' : 'green')); + };packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts (3)
51-61: Derive parameter types from NativeApp to avoid type driftAvoid duplicating the callback union here. If the contract evolves on
NativeApp, this spot won’t have to be updated manually.Apply this diff:
- QueryComponent: ( - source: string, - callback: ( - ret: { __hasReady: boolean } | { - code: number; - detail?: { schema: string }; - }, - ) => void, - ) => { - nativeApp.queryComponent(source, callback); - }, + QueryComponent: ( + ...args: Parameters<NativeApp['queryComponent']> + ) => { + return nativeApp.queryComponent(...args); + },
51-61: Align API casing with surrounding methods (lowerCamelCase) and keep a PascalCase alias for back-compatAll other APIs in this object are lowerCamelCase. Consider making
queryComponentthe canonical entry point and leavingQueryComponentas a temporary alias.Apply this diff:
- QueryComponent: ( - ...args: Parameters<NativeApp['queryComponent']> - ) => { - return nativeApp.queryComponent(...args); - }, + // Canonical lowerCamelCase API + queryComponent: ( + ...args: Parameters<NativeApp['queryComponent']> + ) => { + return nativeApp.queryComponent(...args); + }, + // Back-compat alias; remove once callers migrate + QueryComponent: ( + ...args: Parameters<NativeApp['queryComponent']> + ) => { + return nativeApp.queryComponent(...args); + },
60-61: Harden against runtime absence or exceptions from host NativeAppIf an older host doesn’t implement
queryComponentor if it throws synchronously, this will propagate and potentially crash the worker. Guard it and surface a consistent error to the callback.Apply this diff:
- nativeApp.queryComponent(source, callback); + if (typeof nativeApp.queryComponent !== 'function') { + callback({ code: -1 }); + return; + } + try { + nativeApp.queryComponent(source, callback); + } catch { + callback({ code: -1 }); + }packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
176-191: Note on manifest path: minor overhead onlyPassing isDynamicComponent/source into manifest generation results in an extra globDynamicComponentEntry var in background modules too. It’s harmless but slightly bloats output. Optional to gate the var injection by isESM if you want to keep background bundles minimal.
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
39-39: Consider lifecycle/size controls for moduleCachemoduleCache is module-scoped and will grow over the app lifetime. If dynamic components are numerous, consider an LRU cap, a clearCache hook (e.g., for tests or page teardown), or scoping by session.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
.changeset/fluffy-masks-mate.md(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(8 hunks)packages/web-platform/web-constants/src/utils/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/loadTemplate.ts(3 hunks)packages/web-platform/web-core-server/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic-fail/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component-use-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-dynamic-component.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(6 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/web-platform/web-constants/src/utils/index.ts
- .changeset/fluffy-masks-mate.md
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-tests/tests/react/basic-dynamic-fail/index.jsx
- packages/web-platform/web-tests/tests/react/basic-dynamic-multi/index.jsx
- packages/web-platform/web-tests/tests/react/config-dynamic-component.config.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/crossThreadHandlers/createRegisterQueryComponentResourceHandler.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-core-server/src/utils/loadTemplate.ts
- packages/web-platform/web-constants/src/utils/loadTemplate.ts
- packages/web-platform/web-tests/tests/react/config-dynamic-component-use-effect/index.jsx
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-06T13:28:57.139Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.139Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx
📚 Learning: 2025-08-12T16:09:32.381Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.381Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/web-platform/web-constants/src/utils/generateTemplate.ts
🧬 Code Graph Analysis (6)
packages/web-platform/web-tests/tests/react/basic-dynamic-effect/index.jsx (2)
packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx (3)
importPath(6-6)LazyComponent(7-15)App(17-25)packages/web-platform/web-tests/tests/react/config-dynamic-component-use-effect/index.jsx (1)
App(6-19)
packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx (2)
packages/web-platform/web-tests/tests/react/basic-dynamic-multi/index.jsx (3)
importPath(6-6)LazyComponent(7-16)App(18-25)packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx (1)
App(6-27)
packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx (3)
packages/web-platform/web-tests/tests/react/config-dynamic-component-use-effect/index.jsx (2)
App(6-19)color(7-7)packages/web-platform/web-tests/tests/react/basic-mts-bindtap-change-element-background/index.jsx (2)
App(5-20)App(5-20)packages/web-platform/web-tests/tests/react/basic-mts-bindtap/index.jsx (2)
App(5-21)App(5-21)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
LynxTemplate(20-39)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (4)
packages/web-platform/web-constants/src/endpoints.ts (1)
queryComponentResourceEndpoint(248-251)packages/web-platform/web-constants/src/types/LynxModule.ts (2)
LynxJSModule(41-43)LynxTemplate(20-39)packages/web-platform/web-constants/src/utils/loadTemplate.ts (1)
loadTemplate(10-34)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts (1)
getLepusEntries(7-31)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (2)
packages/web-platform/web-constants/src/endpoints.ts (1)
queryComponentResourceEndpoint(248-251)packages/web-platform/web-mainthread-apis/src/utils/getLepusEntries.ts (1)
getLepusEntries(7-31)
🔇 Additional comments (15)
packages/web-platform/web-tests/tests/react/basic-dynamic-effect/index.jsx (2)
6-15: Lazy import pattern for dynamic component looks correct.The import with with: { type: 'component' } matches the dynamic component flow introduced in this PR.
20-22: Suspense fallback is appropriately set for loading state.Including id='fallback' is helpful for tests to assert loading before resolution.
packages/web-platform/web-tests/tests/react/config-dynamic-component-bindtap/index.jsx (1)
20-23: No changes needed: lowercasebindtapis correct for dynamic componentsThe dynamic-component-bindtap test uses the standard lowercase
bindtapattribute, matching hundreds of other tests and documentation examples. CamelCasebindTapappears only in dev-mode or main-thread contexts (e.g. hot-reload and MTS tests), but for UI-thread bindings on dynamic components, lowercase is the intended convention and is handled correctly by the event system.packages/web-platform/web-tests/tests/react/basic-dynamic/index.jsx (2)
6-15: Dynamic component lazy import wiring looks good.Correct path to the dynamic component JSON with proper component typing.
20-22: Suspense fallback provides deterministic loading state for assertions.Good practice for test reliability.
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts (1)
51-61: LGTM: Background bridge method added and correctly delegates to NativeAppThe addition is aligned with the new NativeApp capability and preserves the expected callback contract by delegating directly to
nativeApp.queryComponent.packages/web-platform/web-constants/src/utils/generateTemplate.ts (4)
73-74: Good addition: expose __QueryComponent to templates on main threadIncluding __QueryComponent in mainThreadInjectVars is consistent with the new main-thread global API and enables templates to invoke it when needed.
95-97: Signature extension looks goodAdding isDynamicComponent and source to generateModuleContent gives the content generator the context it needs without leaking concerns to callers.
129-132: Propagating dynamic-component context is correctThreading isDynamicComponent and source through generateJavascriptUrl ensures both lepusCode and manifest generation have the same context. No issues found.
Also applies to: 142-144
158-166: No legacy positional calls togenerateTemplateremainAll invocations in the codebase now pass a single options object matching the updated signature:
- packages/web-platform/web-core-server/src/utils/loadTemplate.ts (lines 30–33)
- packages/web-platform/web-constants/src/utils/loadTemplate.ts (lines 22–25)
The private
#generateTemplatemethod in the Webpack plugin is distinct and unaffected.packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (2)
144-145: Callbacks surface updated correctlyAdding queryComponentResource to MainThreadRuntimeCallbacks matches the new RPC and is used appropriately below.
165-168: Constructor signature extension is consistent with new cache flowPassing moduleCache into createMainThreadGlobalThis aligns with getLepusEntries and avoids duplicating cache logic.
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (3)
36-37: Good abstraction: centralize Lepus load via getLepusEntriesReplacing bespoke dynamic imports with getLepusEntries simplifies control flow and consolidates caching.
72-74: RPC wiring for queryComponentResource looks correctThe endpoint and background call are set up properly and passed through to the callbacks.
100-103: End-to-end integration with getLepusEntries and runtime init LGTM
- getLepusEntries provides { lepusEntries, entry }.
- lepusEntries is fed into createMainThreadGlobalThis as lepusCode, keeping __LoadLepusChunk working.
- entry is executed before starting the JS context.
No issues found.
Also applies to: 119-119, 236-236
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
3 times, most recently
from
281b011 to
617907b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
c78eefb to
e37fa0b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
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)
125-139: Avoid non-null assertion on appType; default safely for older templates.Prevents runtime crashes with v2 templates lacking appType.
return { ...template, - lepusCode: await generateJavascriptUrl( - template.lepusCode, - createJsModuleUrl as (content: string, name: string) => Promise<string>, - template.appType!, - templateName, - ), - manifest: await generateJavascriptUrl( - template.manifest, - createJsModuleUrl as (content: string, name: string) => Promise<string>, - template.appType!, - templateName, - ), + ...(await (async () => { + const resolvedAppType: 'card' | 'lazy' = + template.appType + ?? (typeof template.lepusCode?.root === 'string' + && template.lepusCode.root.trimStart() + .startsWith('(function (globDynamicComponentEntry') + ? 'lazy' + : 'card'); + return { + lepusCode: await generateJavascriptUrl( + template.lepusCode, + createJsModuleUrl as (content: string, name: string) => Promise<string>, + resolvedAppType, + templateName, + ), + manifest: await generateJavascriptUrl( + template.manifest, + createJsModuleUrl as (content: string, name: string) => Promise<string>, + resolvedAppType, + templateName, + ), + }; + })()), };
♻️ Duplicate comments (2)
packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
18-43: Close timing on errors, dedupe concurrent loads, and check HTTP status.Prevents skewed metrics, redundant fetches, and silent failures on non-2xx.
import { generateTemplate, type LynxTemplate, type MarkTimingInternal, type TemplateLoader, } from '@lynx-js/web-constants'; const templateCache: Record<string, LynxTemplate> = {}; +const inFlight: Record<string, Promise<LynxTemplate>> = {}; function createJsModuleUrl(content: string): string { return URL.createObjectURL(new Blob([content], { type: 'text/javascript' })); } export function createTemplateLoader( customTemplateLoader: TemplateLoader | undefined, markTimingInternal: MarkTimingInternal, ) { - const loadTemplate: TemplateLoader = async ( - url: string, - ) => { - markTimingInternal('load_template_start'); - const cachedTemplate = templateCache[url]; - if (cachedTemplate) { - markTimingInternal('load_template_end'); - return cachedTemplate; - } - const template = customTemplateLoader - ? await customTemplateLoader(url) - : (await (await fetch(url, { - method: 'GET', - })).json()) as LynxTemplate; - const decodedTemplate = await generateTemplate( - template, - createJsModuleUrl, - ); - templateCache[url] = decodedTemplate; - /** - * This will cause a memory leak, which is expected. - * We cannot ensure that the `URL.createObjectURL` created url will never be used, therefore here we keep it for the entire lifetime of this page. - */ - markTimingInternal('load_template_end'); - return decodedTemplate; - }; + const loadTemplate: TemplateLoader = async (url: string) => { + const cached = templateCache[url]; + if (cached) { + markTimingInternal('load_template_cache_hit'); + return cached; + } + if (inFlight[url]) return inFlight[url]; + inFlight[url] = (async () => { + markTimingInternal('load_template_start'); + try { + const template = customTemplateLoader + ? await customTemplateLoader(url) + : await (async () => { + const resp = await fetch(url, { method: 'GET' }); + if (!resp.ok) { + throw new Error(`Failed to fetch template: ${url} (${resp.status})`); + } + return (await resp.json()) as LynxTemplate; + })(); + const decodedTemplate = await generateTemplate( + template, + createJsModuleUrl, + ); + templateCache[url] = decodedTemplate; + return decodedTemplate; + } finally { + markTimingInternal('load_template_end'); + delete inFlight[url]; + } + })(); + return inFlight[url]; + }; return loadTemplate; }packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
64-79: Bug: module is undefined in IIFE; use early return for lazy bundles.Using module.exports inside the IIFE throws in browsers. Emit an early return for 'lazy'.
const generateModuleContent = ( content: string, appType: 'card' | 'lazy', ) => { return [ '//# allFunctionsCalledOnLoad\n', '"use strict";\n', '(() => {const ', globalDisallowedVars.join('=void 0,'), '=void 0;\n', - appType === 'card' ? '' : 'module.exports=\n', + appType === 'card' ? '' : 'return ', content, '\n})()', ].join(''); };
🧹 Nitpick comments (2)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (1)
29-29: Guard against duplicate handler registration (HMR/re-init).If createRenderMultiThread is called more than once for the same Rpc (e.g., HMR or re-init), this will re-register the handler and may process cached messages multiple times. Gate registration per-Rpc.
+// Ensure we register the loader handler at most once per Rpc instance. +const rpcWithLoaderHandler = new WeakSet<Rpc>(); + export function createRenderMultiThread( mainThreadRpc: Rpc, shadowRoot: ShadowRoot, loadTemplate: TemplateLoader, callbacks: StartUIThreadCallbacks, ) { registerReportErrorHandler(mainThreadRpc, 'lepus.js', callbacks.onError); registerFlushElementTreeHandler(mainThreadRpc, { shadowRoot }); registerDispatchLynxViewEventHandler(mainThreadRpc, shadowRoot); createExposureMonitorForMultiThread(mainThreadRpc, shadowRoot); - mainThreadRpc.registerHandler(loadTemplateMultiThread, loadTemplate); + if (!rpcWithLoaderHandler.has(mainThreadRpc)) { + mainThreadRpc.registerHandler(loadTemplateMultiThread, loadTemplate); + rpcWithLoaderHandler.add(mainThreadRpc); + }packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
14-17: Optional: name blobs from URL for easier debugging and fewer collisions.Pass a stable templateName derived from URL to generateTemplate.
export function createTemplateLoader( customTemplateLoader: TemplateLoader | undefined, markTimingInternal: MarkTimingInternal, ) { const loadTemplate: TemplateLoader = async (url: string) => { + const templateName = + (() => { + try { return new URL(url, self.location?.href ?? 'http://localhost').pathname.replace(/[^\w.-]+/g, '_'); } + catch { return 'template'; } + })(); const cached = templateCache[url]; if (cached) { markTimingInternal('load_template_cache_hit'); return cached; } @@ - const decodedTemplate = await generateTemplate( - template, - createJsModuleUrl, - ); + const decodedTemplate = await generateTemplate( + template, + createJsModuleUrl, + templateName, + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (23)
.changeset/loose-ads-add.md(1 hunks)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/LynxModule.ts(1 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-constants/src/types/MarkTiming.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-constants/src/types/TemplateLoader.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(2 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(5 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(7 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(4 hunks)packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
- packages/web-platform/web-constants/src/types/MarkTiming.ts
- packages/web-platform/web-constants/src/types/TemplateLoader.ts
- packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-constants/src/types/index.ts
- packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts
- .changeset/loose-ads-add.md
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-constants/src/types/LynxModule.ts
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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-core/src/utils/loadTemplate.ts
🧬 Code graph analysis (2)
packages/web-platform/web-core/src/utils/loadTemplate.ts (5)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
LynxTemplate(21-42)packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-constants/src/types/MarkTiming.ts (1)
MarkTimingInternal(11-15)packages/web-platform/web-core-server/src/utils/loadTemplate.ts (1)
loadTemplate(21-41)packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
generateTemplate(105-140)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (5)
packages/web-platform/web-worker-rpc/src/Rpc.ts (1)
Rpc(38-473)packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
StartUIThreadCallbacks(26-31)packages/web-platform/web-core/src/uiThread/crossThreadHandlers/registerReportErrorHandler.ts (1)
registerReportErrorHandler(7-18)packages/web-platform/web-constants/src/endpoints.ts (1)
loadTemplateMultiThread(258-261)
⏰ 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). (5)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (javascript-typescript)
- GitHub Check: CodeQL Analyze (actions)
🔇 Additional comments (2)
packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
1-6: LGTM: type-only imports and API surface.Imports and factory signature align with the cross-thread loader design.
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
55-59: LGTM: manifest upgrader now defines module/exports before use.This fixes the prior ReferenceError risk when wrapping manifest entries.
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
e37fa0b to
f820603
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
98-105: Handle template load failures: prevent unhandled promise rejections and flush timings.Add a catch to surface errors via
onErrorand ensure timings are flushed. Prefix withvoidto avoid “floating promise” lints.- templateLoader(templateUrl).then((template) => { - flushMarkTimingInternal(); - start({ - ...configs, - template, - }); - }); + void templateLoader(templateUrl) + .then((template) => { + flushMarkTimingInternal(); + start({ + ...configs, + template, + }); + }) + .catch((err) => { + // Do not mark 'load_template_end' here to avoid double-marking; + // centralize timing end inside the loader if needed. + flushMarkTimingInternal(); + callbacks.onError?.(err as Error, '', 'templateLoader'); + });packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
65-79: Fix:moduleis undefined in the IIFE; use early-return for lazy bundlesInside the IIFE there is no
moduleobject, so emittingmodule.exports=forappType === 'lazy'throws at runtime. Return the value directly instead. Also add a brief comment to document the intentional early return for lazy bundles.const generateModuleContent = ( content: string, appType: 'card' | 'lazy', ) => { return [ '//# allFunctionsCalledOnLoad\n', '"use strict";\n', '(() => {const ', globalDisallowedVars.join('=void 0,'), '=void 0;\n', - appType === 'card' ? '' : 'module.exports=\n', + // For lazy bundles, the generated IIFE should return the component directly. + appType === 'card' ? '' : 'return ', content, '\n})()', ].join(''); };
131-139: Remove non-null assertions ontemplate.appTypeand provide a safe defaultOlder v2 templates won’t have
appType;template.appType!will crash. Default to'card'until the encoding plugin guaranteesappType, then thread that variable through both code paths.- template.appType!, + appType,- template.appType!, + appType,Add this near Line 126 (above the return) to define the fallback:
const appType: 'card' | 'lazy' = template.appType ?? 'card';packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (2)
55-55: Signature change: ensure all call sites pass loadTemplateThis is a breaking change; verify every prepareMainThreadAPIs invocation is updated.
Run:
#!/bin/bash # Show all call sites and surrounding args to confirm the new parameter is passed rg -nP -C3 --type=ts '\bprepareMainThreadAPIs\s*\('
112-122: Race: mtsGlobalThisRef may be undefined when __QueryComponent runs__QueryComponent can be invoked before Line 248 assigns mtsGlobalThisRef.mtsGlobalThis, causing a TypeError when accessing processEvalResult in createQueryComponent.
Apply this fix in createQueryComponent to guard access:
- if (mtsGlobalThisRef.mtsGlobalThis.processEvalResult) { - lepusRootChunkExport = mtsGlobalThisRef.mtsGlobalThis.processEvalResult( - lepusRootChunkExport, - url, - ); - } + const processEvalResult = mtsGlobalThisRef.mtsGlobalThis?.processEvalResult; + if (processEvalResult) { + lepusRootChunkExport = processEvalResult(lepusRootChunkExport, url); + }
🧹 Nitpick comments (7)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (1)
19-24: Consider tying the RPC handler’s lifecycle to dispose.If multiple views reuse the same Rpc, the handler may accumulate across inits. Consider removing it on teardown (e.g., via createDispose or a returned cleanup).
Also applies to: 29-29
packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
78-81: Optional: centralize success/failure timing in the loader.A try/finally around the loader’s internals guarantees a single source of truth for start/end marks and avoids duplication at call sites.
Outside this file (in createTemplateLoader):
export function createTemplateLoader(customTemplateLoader: TemplateLoader | undefined, markTimingInternal: MarkTimingInternal) { const loadTemplate: TemplateLoader = async (url: string) => { markTimingInternal('load_template_start'); try { // existing body... } finally { // ensure 'end' always recorded (success or failure) markTimingInternal('load_template_end'); } }; return loadTemplate; }packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
57-57: Preferconstovervarin emitted manifest wrapperUse block-scoped bindings to avoid accidental reassignments in user code and match modern JS style. No behavior change.
- `module.exports = {init: (lynxCoreInject) => { var {${defaultInjectStr}} = lynxCoreInject.tt; var module = {exports:{}}; var exports=module.exports; ${value}\n return module.exports; } }`, + `module.exports = {init: (lynxCoreInject) => { const {${defaultInjectStr}} = lynxCoreInject.tt; const module = {exports:{}}; const exports = module.exports; ${value}\n return module.exports; } }`,packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
248-248: Ordering note: rely on guards or defer registrationSince mtsGlobalThisRef is assigned after the PAPI is created, keep the null-guards in createQueryComponent (above). Alternatively, construct the PAPI after assignment if feasible.
packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts (3)
52-58: Report errors via _ReportError; keep console as fallbackSurface failures to the platform reporter and avoid relying solely on console.error.
Apply:
- }).catch((error) => { - console.error(`lynx web: lazy bundle load failed:`, error); + }).catch((error) => { + try { mtsGlobalThisRef.mtsGlobalThis?._ReportError?.(error as Error, undefined as any); } catch {} + if (!mtsGlobalThisRef.mtsGlobalThis?._ReportError) { + console.error(`lynx web: lazy bundle load failed:`, error); + }
61-74: Ensure RPC handler resolves even on sync throwWrap the __QueryComponentImpl call to avoid hanging the Promise if a synchronous error occurs before the callback.
Apply:
- backgroundThreadRpc.registerHandler(queryComponentEndpoint, (url: string) => { + backgroundThreadRpc.registerHandler(queryComponentEndpoint, (url: string) => { const ret: ReturnType<RpcCallType<typeof queryComponentEndpoint>> = - new Promise(resolve => { - __QueryComponentImpl(url, (result) => { - resolve({ - code: result.code, - detail: { - schema: url, - }, - }); - }); - }); + new Promise(resolve => { + try { + __QueryComponentImpl(url, (result) => { + resolve({ + code: result.code, + detail: { schema: url }, + }); + }); + } catch { + resolve({ + code: -1, + detail: { schema: url }, + }); + } + }); return ret; });
28-41: Optional: de-dup concurrent loads per URLAvoid duplicate loadTemplate/loadScript/style updates under bursty calls.
Minimal pattern:
// declare once inside factory scope const inFlight: Record<string, Promise<void>> = {}; // inside __QueryComponentImpl before starting work if (inFlight[url]) { return null; } inFlight[url] = (async () => { const template = await loadTemplate(url); const updateBTS = updateBTSTemplateCache(url, template); let exp = await mtsRealm.loadScript(template.lepusCode.root); const per = mtsGlobalThisRef.mtsGlobalThis?.processEvalResult; if (per) exp = per(exp, url); updateLazyComponentStyle(template.styleInfo, url); await updateBTS; jsContext.dispatchEvent({ type: '__OnDynamicJSSourcePrepared', data: url }); callback?.({ code: 0, data: { url, evalResult: exp } }); })() .catch((e) => { try { mtsGlobalThisRef.mtsGlobalThis?._ReportError?.(e as Error, undefined as any); } catch {} }) .finally(() => { delete inFlight[url]; });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (23)
.changeset/loose-ads-add.md(1 hunks)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/LynxModule.ts(1 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-constants/src/types/MarkTiming.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-constants/src/types/TemplateLoader.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(2 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(5 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(7 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(4 hunks)packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/loose-ads-add.md
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/web-platform/web-constants/src/types/TemplateLoader.ts
- packages/web-platform/web-constants/src/types/LynxModule.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
- packages/web-platform/web-constants/src/types/index.ts
- packages/web-platform/web-core/src/utils/loadTemplate.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-constants/src/types/MarkTiming.ts
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
- packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-29T16:59:22.009Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1235
File: packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts:33-38
Timestamp: 2025-08-29T16:59:22.009Z
Learning: The processEvalResult method in MainThreadGlobalThis interface should return the processed evaluation result rather than void, as it's used to transform lepusRootChunkExport in the lazy component loading flow.
Applied to files:
packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
📚 Learning: 2025-08-29T17:32:08.014Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1235
File: packages/web-platform/web-constants/src/endpoints.ts:248-261
Timestamp: 2025-08-29T17:32:08.014Z
Learning: In the Lynx RPC system, Promise<void> with hasReturn=true is meaningful for cross-thread operations as it allows callers to await completion of background work, even when no data is returned. The Promise<void> serves as a completion signal rather than a data carrier.
Applied to files:
packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
📚 Learning: 2025-08-29T16:57:19.204Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1235
File: packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts:24-26
Timestamp: 2025-08-29T16:57:19.204Z
Learning: In the Lynx RPC system, when createRpcEndpoint has hasReturn=true (third parameter), even if the return type is void, it returns a Promise<void> that resolves when the work on the background thread is completed. This allows proper awaiting of cross-thread operations.
Applied to files:
packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
📚 Learning: 2025-07-16T06:26:22.230Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
Applied to files:
packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
📚 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-mainthread-apis/src/prepareMainThreadAPIs.ts
📚 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-core/src/uiThread/startUIThread.ts
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-core/src/uiThread/startUIThread.ts
🧬 Code graph analysis (4)
packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts (5)
packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (2)
MainThreadGlobalThis(316-406)QueryComponentPAPI(305-314)packages/web-platform/web-constants/src/types/JSRealm.ts (1)
JSRealm(1-5)packages/web-platform/web-constants/src/endpoints.ts (2)
updateBTSTemplateCacheEndpoint(253-256)queryComponentEndpoint(248-251)packages/web-platform/web-worker-rpc/src/TypeUtils.ts (1)
RpcCallType(9-18)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (4)
packages/web-platform/web-worker-rpc/src/Rpc.ts (1)
Rpc(38-473)packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
StartUIThreadCallbacks(26-31)packages/web-platform/web-constants/src/endpoints.ts (1)
loadTemplateMultiThread(258-261)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (6)
packages/web-platform/web-core-server/src/utils/loadTemplate.ts (1)
loadTemplate(21-41)packages/web-platform/web-tests/server.js (1)
loadTemplate(7-14)packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts (1)
appendStyleElement(175-252)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
MainThreadGlobalThis(316-406)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts (1)
createQueryComponent(14-76)
packages/web-platform/web-core/src/uiThread/startUIThread.ts (3)
packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-constants/src/types/MarkTiming.ts (1)
MarkTimingInternal(11-15)packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
createTemplateLoader(14-45)
⏰ 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). (5)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (7)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (1)
29-29: Cross-thread loader registration looks correct.Registering
loadTemplateMultiThreadwith the providedTemplateLoaderis type-safe and aligns with the new endpoint contract.packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
81-96: LGTM:appTypeis correctly threaded intogenerateJavascriptUrlPlumbing the
appTypeparameter through URL generation is the right direction; once the non-null assertions are removed per above, this path looks good.Also applies to: 92-93, 84-85
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (4)
28-30: Types import looks correctImporting MainThreadGlobalThis and TemplateLoader aligns with the new API surface.
37-37: Importing createQueryComponent is appropriateWires the new PAPI into the main-thread setup.
104-111: Correct: capturing updateLazyComponentStyle from appendStyleElementThis matches the extended return contract and enables lazy-style updates.
245-245: Exposing __QueryComponent on callbacks is consistent with MainThreadGlobalThisInterface alignment looks good.
packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts (1)
59-59: Returning null matches QueryComponentPAPIThe return value adheres to the declared signature.
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
f820603 to
4ec1389
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
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-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
97-112: Fix double-callback and wrong error typing in loadScriptAsync.
import(...).catch(callback).then(...)invokes the callback on both failure and success, and passes an Error to a(message: string | null)param.- loadScriptAsync: function( + loadScriptAsync: function( sourceURL: string, callback: (message: string | null, exports?: BundleInitReturnObj) => void, entryName?: string, ): void { - entryName = entryName ?? '__Card__'; - const manifestUrl = templateCache.get(entryName!) - ?.manifest[`/${sourceURL}`]; + const effectiveEntry = entryName ?? '__Card__'; + const manifestUrl = templateCache.get(effectiveEntry) + ?.manifest[`/${sourceURL}`]; if (manifestUrl) sourceURL = manifestUrl; - import( + // Avoid stale exports between async loads + Object.assign(globalThis, { module: { exports: null } }); + import( /* webpackIgnore: true */ sourceURL - ).catch(callback).then(async () => { - callback(null, createBundleInitReturnObj()); - }); + ) + .then(() => { + callback(null, createBundleInitReturnObj()); + }) + .catch((err) => { + // Surface to UI and normalize error to string for the callback + reportError(err as Error, undefined, release); + const msg = (err && typeof err === 'object' && 'message' in (err as any)) + ? String((err as any).message) + : String(err); + callback(msg); + }); },
♻️ Duplicate comments (5)
packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx (1)
9-14: Ensure import attributes are preserved in the test bundler.
Dynamic import with “with: { type: 'component' }” requires keepImportAttributes enabled in the web-tests rspack config. This echoes an earlier suggestion on this PR.Run to confirm:
#!/bin/bash rg -nP "keepImportAttributes|builtin:swc-loader" packages/web-platform/web-tests/rspack.config.js -C2Expected: a rule using builtin:swc-loader with options.jsc.experimental.keepImportAttributes: true for JS/JSX.
packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts (1)
101-107: Run dprint to satisfy CIFormatting around this block is what CI flagged previously; re-run the repo formatter.
Command:
pnpm dprint fmtpackages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
67-79: Bug: module is undefined inside the IIFE for lazy path—use early returnInjecting
module.exports=here throws at runtime; the IIFE does not definemodule. For lazy bundles, emit an earlyreturninstead.Apply:
- appType === 'card' ? '' : 'module.exports=\n', + appType === 'card' ? '' : 'return ',packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
76-82: Notify pending queryComponent callers when template arrives.Right now the update handler populates the cache but never signals waiting callers. Track per-source pending callbacks and flush them here.
const templateCache = new Map<string, LynxTemplate>([['__Card__', template]]); + const pendingQueryComponentCallbacks: Record< + string, + Array<(ret: { __hasReady: boolean } | { code: number; detail?: { schema: string } }) => void> + > = {}; mainThreadRpc.registerHandler( updateBTSTemplateCacheEndpoint, (url, template) => { templateCache.set(url, template); + const cbs = pendingQueryComponentCallbacks[url]; + if (cbs?.length) { + for (const cb of cbs) cb({ __hasReady: true }); + delete pendingQueryComponentCallbacks[url]; + } }, );
172-180: Attach .catch to queryComponent to avoid unhandled rejections and notify caller.Currently failures bubble as unhandled rejections and the callback never fires.
queryComponent: (source, callback) => { if (templateCache.has(source)) { callback({ __hasReady: true }); } else { - queryComponent(source).then(res => { - callback?.(res); - }); + queryComponent(source) + .then(res => { + callback?.(res); + }) + .catch((err: Error) => { + reportError(err, undefined, release); + callback?.({ code: -1, detail: { schema: 'queryComponent rpc failed' } } as any); + }); } },
🧹 Nitpick comments (8)
packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx (4)
6-15: Inline the import path literal to avoid context-based bundling.
Using a variable for the import() specifier can trigger a context module in webpack/rspack, bloating bundles and slowing tests. Also drops the unnecessary template literal.-const importPath = `./dist/config-lazy-component-bindtap/index.web.json`; -const LazyComponent = lazy( - () => - import( - importPath, - { - with: { type: 'component' }, - } - ), -); +const LazyComponent = lazy(() => + import('./dist/config-lazy-component-bindtap/index.web.json', { + with: { type: 'component' }, + }), +);
7-15: Optional: name the lazy chunk for easier debugging.- import('./dist/config-lazy-component-bindtap/index.web.json', { + import(/* webpackChunkName: "react-basic-lazy-relpath" */ './dist/config-lazy-component-bindtap/index.web.json', { with: { type: 'component' }, }),
17-25: Wrap Suspense with a minimal ErrorBoundary to keep tests resilient on load failures.
If the lazy import rejects, tests may hard-crash without an error boundary.return ( <view> - <Suspense fallback={<text id='fallback'>Loading...</text>}> - <LazyComponent /> - </Suspense> + <ErrorBoundary> + <Suspense fallback={<text id='fallback'>Loading...</text>}> + <LazyComponent /> + </Suspense> + </ErrorBoundary> </view> );Add this helper near the top of the file (outside the shown range):
class ErrorBoundary extends (globalThis.React?.Component ?? (await import('@lynx-js/react')).Component) { constructor(props) { super(props); this.state = { hasError: false }; } static getDerivedStateFromError() { return { hasError: true }; } render() { return this.state.hasError ? <text id="error">Error</text> : this.props.children; } }
27-27: JSX nit: use self-closing tag.-root.render(<App></App>); +root.render(<App />);packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts (1)
101-107: Hoist TEMPLATE_VERSION = 2 and tighten appType typingAvoid magic numbers and brittle bracket lookups; align with currentSupportedTemplateVersion (2) to skip unnecessary upgrades and keep parity across tooling.
Apply:
- version: 1, - appType: - (encodeOptions['sourceContent'] as Record<string, unknown>) - ?.['appType'] - === 'DynamicComponent' - ? 'lazy' - : 'card', + version: TEMPLATE_VERSION, + appType: + ((encodeOptions.sourceContent as { appType?: unknown })?.appType === + 'DynamicComponent' + ? 'lazy' + : 'card'),Add near the top of this file (outside the selected range):
const TEMPLATE_VERSION = 2;packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
65-75: Minor: keep the disallowed globals list static and documentedConsider a short comment explaining why these globals are shadowed, to aid future maintainers.
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
71-75: Keep createBundleInitReturnObj side-effect free.Minor: resetting
globalThis.module.exportshere couples “read entry” with global mutation. Consider doing all resets in the loaders and let this helper only read/return the entry.
113-121: Avoid non-null assertion and optionally pre-warm cache before importScripts.Use a local const for
entryNameto drop!. Optionally fire-and-forget a fetch to leverage HTTP cache before the synchronousimportScripts()(matches prior worker-loading guidance).- loadScript: (sourceURL: string, entryName?: string) => { - entryName = entryName ?? '__Card__'; - const manifestUrl = templateCache.get(entryName!) - ?.manifest[`/${sourceURL}`]; + loadScript: (sourceURL: string, entryName?: string) => { + const effectiveEntry = entryName ?? '__Card__'; + const manifestUrl = templateCache.get(effectiveEntry) + ?.manifest[`/${sourceURL}`]; if (manifestUrl) sourceURL = manifestUrl; + // Optional: pre-warm HTTP cache for the impending sync load + // fetch(sourceURL).catch(() => {}); Object.assign(globalThis, { module: { exports: null } }); importScripts(sourceURL); return createBundleInitReturnObj(); },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (23)
.changeset/loose-ads-add.md(1 hunks)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/LynxModule.ts(1 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-constants/src/types/MarkTiming.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-constants/src/types/TemplateLoader.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(2 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(5 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(7 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(4 hunks)packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (19)
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- .changeset/loose-ads-add.md
- packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts
- packages/web-platform/web-constants/src/types/MarkTiming.ts
- packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
- packages/web-platform/web-constants/src/types/LynxModule.ts
- packages/web-platform/web-constants/src/types/TemplateLoader.ts
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-constants/src/types/index.ts
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
- packages/web-platform/web-core/src/utils/loadTemplate.ts
🧰 Additional context used
🧠 Learnings (2)
📚 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/createNativeApp.ts
📚 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-tests/tests/react/basic-lazy-component-relative-path/index.jsx
🧬 Code graph analysis (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
packages/web-platform/web-constants/src/types/NativeApp.ts (1)
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). (5)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (actions)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (9)
packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx (3)
1-3: LGTM: License header present and correct.
20-22: LGTM: Stable fallback selector.
id='fallback' gives the test harness a reliable hook.
27-27: Consistent self‐mounting confirmed: explicit root.render present in every React test
All tests inpackages/web-platform/web-tests/tests/reactinvokeroot.renderthemselves, indicating the harness does not auto-mount—no duplicate render risk.packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
57-58: Upgrade wrapper for manifest looks correctWrapper initializes a CJS-like module and returns
module.exports. This aligns with v1→v2 upgrade behavior.
81-86: API threading of appType through generateJavascriptUrl looks goodPassing
appTypeinto the code-generation path is consistent with the new lazy/card split.Also applies to: 90-96
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (4)
17-19: Imports LGTM.The added types and endpoints align with the new lazy-component flow.
67-69: RPC handle creation looks correct.
mainThreadRpc.createCall(queryComponentEndpoint)is the right choice for a one-shot background→UI call.
132-159: No unsafe non-null assertions detected, approving code changes
All occurrences oftt!and legacyloadScript(template.manifest['/app-service.js'])patterns were searched and none were found. Merge as-is.
172-180: The script has been queued—awaiting output.
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
4ec1389 to
e4e365b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
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)
81-104: Do not drop non-string entries; support sync creators; avoid 'undefined-' in filenamesCurrent logic filters out non-string values and loses them in the returned object. It also always prefixes with
undefined-whentemplateNameis missing, and the type forces async creators. Preserve all entries, build a safe filename, and accept both sync/async creators.-async function generateJavascriptUrl<T extends Record<string, string | {}>>( - obj: T, - createJsModuleUrl: (content: string, name: string) => Promise<string>, - appType: 'card' | 'lazy', - templateName?: string, -): Promise<T> { - const processEntry = async ([name, content]: [string, string]) => [ - name, - await createJsModuleUrl( - generateModuleContent( - content, - appType, - ), - `${templateName}-${name.replaceAll('/', '')}.js`, - ), - ]; - return Promise.all( - (Object.entries(obj).filter(([_, content]) => - typeof content === 'string' - ) as [string, string][]).map(processEntry), - ).then( - Object.fromEntries, - ); -} +async function generateJavascriptUrl<T extends Record<string, string | {}>>( + obj: T, + createJsModuleUrl: (content: string, name: string) => Promise<string> | string, + appType: 'card' | 'lazy', + templateName?: string, +): Promise<T> { + const result: Record<string, unknown> = { ...obj }; + for (const [name, content] of Object.entries(obj)) { + if (typeof content === 'string') { + const base = name.replaceAll('/', ''); + const fileName = (templateName ? `${templateName}-` : '') + `${base}.js`; + const moduleContent = generateModuleContent(content, appType); + // Normalize to Promise and assign back + result[name] = await Promise.resolve(createJsModuleUrl(moduleContent, fileName)); + } + } + return result as T; +}
♻️ Duplicate comments (8)
packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts (1)
278-281: textContent getter must aggregate descendants to match DOM semanticsCurrently returns only the element’s own text; callers will read incomplete content for nested structures.
Apply this diff:
- get textContent() { - return this[textContent]; - } + get textContent(): string { + // Own text plus all descendants, mirroring Element.textContent + let acc = this[textContent] ?? ''; + for (const child of this[_children]) { + acc += child.textContent ?? ''; + } + return acc; + }packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts (1)
48-60: Fix TS breakage: don’t destructure optional exports; assign loader idempotently.Destructuring
nativeGlobalandloadDynamicComponentbreaks type-checking against older@lynx-js/lynx-coretypings and can overwrite an existing loader. Guard via property access on the resolved module and make the assignment idempotent.- lynxCore.then( - ( - { - loadCard, - destroyCard, - callDestroyLifetimeFun, - nativeGlobal, - loadDynamicComponent, - }, - ) => { - // @lynx-js/lynx-core >= 0.1.3 will export nativeGlobal and loadDynamicComponent - if (nativeGlobal && loadDynamicComponent) { - nativeGlobal.loadDynamicComponent = loadDynamicComponent; - } + lynxCore.then((core) => { + // Destructure only stable exports + const { loadCard, destroyCard, callDestroyLifetimeFun } = + core as typeof import('@lynx-js/lynx-core/web'); + // Optional exports (newer cores only) + const maybeNativeGlobal = + (core as any).nativeGlobal as undefined | { loadDynamicComponent?: unknown }; + const maybeLoadDynamic = + (core as any).loadDynamicComponent as undefined | ((...args: any[]) => any); + // Assign once + if (maybeNativeGlobal && maybeLoadDynamic && !maybeNativeGlobal.loadDynamicComponent) { + (maybeNativeGlobal as any).loadDynamicComponent = maybeLoadDynamic; + } // ...If you prefer to keep destructuring, add the missing type members to the
lynx-coretypings as well. Do you want me to open a follow-up for the type update?packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (3)
132-159: LGTM:ttexposure and assignment.Publicly exposing
ttand setting it insetCardmatches prior guidance. Ensure theNativeApptype includestt: any | null.
76-82: Success path never notifies callers after template arrives; flush pending callbacks in the update handler.You set the cache on
updateBTSTemplateCacheEndpointbut don’t notify any waitingqueryComponentcallbacks. Track per-source pending callbacks and flush on update.- const templateCache = new Map<string, LynxTemplate>([['__Card__', template]]); + const templateCache = new Map<string, LynxTemplate>([['__Card__', template]]); + const pendingQueryComponentCallbacks: Record< + string, + Array<(ret: { __hasReady: boolean } | { code: number; detail?: { schema: string } }) => void> + > = {}; mainThreadRpc.registerHandler( updateBTSTemplateCacheEndpoint, (url, template) => { templateCache.set(url, template); + const cbs = pendingQueryComponentCallbacks[url]; + if (cbs?.length) { + for (const cb of cbs) cb({ __hasReady: true }); + delete pendingQueryComponentCallbacks[url]; + } }, );
172-180: Handle RPC rejections and integrate with pending-callbacks map.Currently, rejections are unhandled and callbacks can leak. Also, on success you forward the UI result but don’t notify when the template actually arrives. Use the pending map (above), forward errors, and clean up.
queryComponent: (source, callback) => { if (templateCache.has(source)) { callback({ __hasReady: true }); } else { - queryComponent(source).then(res => { - callback?.(res); - }); + (pendingQueryComponentCallbacks[source] ??= []).push(callback); + queryComponent(source) + .then(res => { + // If UI reports an error, surface it and clean up this callback. + if (res && typeof (res as any).code === 'number' && (res as any).code !== 0) { + const arr = pendingQueryComponentCallbacks[source]; + if (arr) { + const idx = arr.indexOf(callback); + if (idx >= 0) arr.splice(idx, 1); + if (arr.length === 0) delete pendingQueryComponentCallbacks[source]; + } + callback(res as any); + } + // Success will be signaled via updateBTSTemplateCacheEndpoint. + }) + .catch((err: Error) => { + const arr = pendingQueryComponentCallbacks[source]; + if (arr) { + const idx = arr.indexOf(callback); + if (idx >= 0) arr.splice(idx, 1); + if (arr.length === 0) delete pendingQueryComponentCallbacks[source]; + } + reportError(err, undefined, release); + callback?.({ code: -1, detail: { schema: 'queryComponent rpc failed' } } as any); + }); } },packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
99-105: Handle templateLoader rejection and flush timings on failure.Currently, a rejected load causes an unhandled promise rejection and leaves timings unflushed. Add a catch that marks load_template_end and surfaces the error; this won’t double-mark since createTemplateLoader only marks end on success.
Apply:
- templateLoader(templateUrl).then((template) => { - flushMarkTimingInternal(); - start({ - ...configs, - template, - }); - }); + templateLoader(templateUrl) + .then((template) => { + flushMarkTimingInternal(); + start({ + ...configs, + template, + }); + }) + .catch((err) => { + // Ensure timing markers are closed and flushed on failure + markTimingInternal('load_template_end'); + flushMarkTimingInternal(); + callbacks.onError?.(err as Error, '', templateUrl); + });packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
126-140: Remove non-null assertion on appType; add backward-compatible fallback and reuse for both pathsOlder v2 templates may lack
appType. The non-null assertion risks wrong behavior. Compute a safe fallback once and pass it into bothgenerateJavascriptUrlcalls.return { ...template, - lepusCode: await generateJavascriptUrl( - template.lepusCode, - createJsModuleUrl as (content: string, name: string) => Promise<string>, - template.appType!, - templateName, - ), - manifest: await generateJavascriptUrl( - template.manifest, - createJsModuleUrl as (content: string, name: string) => Promise<string>, - template.appType!, - templateName, - ), + // Fallback: infer lazy/card only when appType is absent; remove once upstream always sets appType. + lepusCode: await generateJavascriptUrl( + template.lepusCode, + createJsModuleUrl as (content: string, name: string) => Promise<string> | string, + (template.appType ?? + (typeof (template as any).lepusCode?.root === 'string' && + (template as any).lepusCode.root.trimStart() + .startsWith('(function (globDynamicComponentEntry') ? 'lazy' : 'card')), + templateName, + ), + manifest: await generateJavascriptUrl( + template.manifest, + createJsModuleUrl as (content: string, name: string) => Promise<string> | string, + (template.appType ?? + (typeof (template as any).lepusCode?.root === 'string' && + (template as any).lepusCode.root.trimStart() + .startsWith('(function (globDynamicComponentEntry') ? 'lazy' : 'card')), + templateName, + ), };To confirm impact across the repo (older templates without
appType), run:#!/bin/bash # Find templates missing appType and check for lepusCode.root usage (heuristic fallback path) rg -n --type ts -C2 '\bappType\b' packages/ | wc -l rg -n --type ts -C2 '\blepusCode\s*\.\s*root\b' packages/
72-78: Fix ReferenceError for lazy bundles: use early return instead of CommonJS exportThere is no
modulebinding inside the IIFE.appType === 'lazy' ? 'module.exports=\n' : ''will throw in browsers. Return the component from the IIFE for lazy bundles. Also add a brief comment to document the intent.- return [ + // Lazy: early-return value from IIFE; there is no CommonJS `module` here. + return [ '//# allFunctionsCalledOnLoad\n', '"use strict";\n', '(() => {const ', globalDisallowedVars.join('=void 0,'), '=void 0;\n', - appType === 'lazy' ? 'module.exports=\n' : '', + appType === 'lazy' ? 'return ' : '', content, '\n})()', ].join('');
🧹 Nitpick comments (14)
packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx (4)
24-26: Avoid eager load to truly test “when needed”.Rendering unconditionally triggers an immediate fetch; the click only adds a second instance. If the test’s intent is “load on demand,” gate the first render behind shouldDisplay too.
- <Suspense fallback={<text>Loading...</text>}> - <LazyComponent /> - </Suspense> + {shouldDisplay && ( + <Suspense fallback={<text>Loading...</text>}> + <LazyComponent /> + </Suspense> + )}
27-33: Stabilize the click target for e2e selectors.Add a data-testid to avoid coupling to id or text in automation.
- <view + <view bindtap={handleClick} - id='target' + id='target' + data-testid='lazy-toggle' style={{ width: '100px', height: '100px', backgroundColor: 'red' }} >
24-26: Deduplicate identical Suspense fallbacks.If you keep both Suspense blocks, extract the fallback to a constant for clarity.
import { useState, root, lazy, Suspense } from '@lynx-js/react'; +const Fallback = <text>Loading...</text>; @@ - <Suspense fallback={<text>Loading...</text>}> + <Suspense fallback={Fallback}> <LazyComponent /> </Suspense> @@ - <Suspense fallback={<text>Loading...</text>}> + <Suspense fallback={Fallback}> <LazyComponent /> </Suspense>Also applies to: 35-37
4-4: Remove manual root.render to let harness mount App
Inpackages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx, drop therootimport and delete theroot.render(<App></App>)call—your existingexport function Appwill be mounted by the test runner.-import { useState, root, lazy, Suspense } from '@lynx-js/react'; +import { useState, lazy, Suspense } from '@lynx-js/react'; @@ -root.render(<App></App>);packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx (2)
7-26: DRY the lazy-import wiring with a tiny factory.You duplicate the same lazy(() => import(path, { with: { type: 'component' } })) pattern twice. A small helper improves readability and future changes.
import { root, lazy, Suspense } from '@lynx-js/react'; import './index.css'; +const mkLazy = (path) => + lazy(() => import(path, { with: { type: 'component' } })); const importPath = `/dist/config-lazy-component-css/index.web.json`; -const LazyComponent = lazy( - () => - import( - importPath, - { - with: { type: 'component' }, - } - ), -); +const LazyComponent = mkLazy(importPath); const importPath2 = `/dist/config-lazy-component-css-other/index.web.json`; -const LazyComponent2 = lazy( - () => - import( - importPath2, - { - with: { type: 'component' }, - } - ), -); +const LazyComponent2 = mkLazy(importPath2);
40-40: Use self-closing JSX.Minor consistency/readability nit.
-root.render(<App></App>); +root.render(<App />);packages/web-platform/web-tests/tests/react.spec.ts (5)
444-466: Reduce flakiness: prefer assertion-driven waits over fixed sleeps.Replace the initial fixed wait with a readiness assertion; keep toHaveCSS auto-wait for state changes.
test( 'basic-lazy-component', async ({ page }, { title }) => { test.skip(isSSR, 'Lazy Component not support on SSR'); await goto(page, title); - await wait(500); - await expect(page.locator('#target1')).toHaveCSS( + // Wait until the lazy component is visible/ready instead of sleeping. + await expect(page.locator('#target1')).toBeVisible({ timeout: 10000 }); + await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green await page.locator('#target2').click(); - await wait(100); await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(255, 192, 203)', ); // pink await page.locator('#target2').click(); - await wait(100); await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green }, );If this stabilizes well, consider applying the same pattern to the other lazy tests below.
446-676: Fix wording in skip reasons.Make skip messages grammatically correct and consistent.
- test.skip(isSSR, 'Lazy Component not support on SSR'); + test.skip(isSSR, 'Lazy Component is not supported on SSR');Apply to all lazy-component tests in this block.
Also applies to: 469-491, 493-711, 590-604, 607-621, 625-643, 646-670, 673-711
495-501: Use Playwright’s locator assertions instead of innerText().Avoid fetching text manually; toHaveText auto-waits and is less flaky.
-const result = await page.locator('#fallback').first().innerText(); -expect(result).toBe('Loading...'); +await expect(page.locator('#fallback').first()).toHaveText('Loading...');
625-643: Optional: strengthen the CSS isolation check.Asserting “not orange” is permissive. If the expected color is known (e.g., transparent or a specific value), assert it directly to catch regressions.
516-550: Deduplicate repeated locators.Minor readability improvement: store locators before reuse.
- await expect(page.locator('#target1').nth(0)).toHaveCSS( + const t1_0 = page.locator('#target1').nth(0); + const t1_1 = page.locator('#target1').nth(1); + await expect(t1_0).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green - await expect(page.locator('#target1').nth(1)).toHaveCSS( + await expect(t1_1).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green - await page.locator('#target2').nth(0).click(); + const t2_0 = page.locator('#target2').nth(0); + const t2_1 = page.locator('#target2').nth(1); + await t2_0.click();packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
73-75: Avoid clobbering the global module object.
Object.assign(globalThis, { module: { exports: null } })replaces the entiremoduleobject. Prefer only resettingexportsto avoid dropping potential fields.- Object.assign(globalThis, { module: { exports: null } }); + // Keep existing module object if present; only reset exports + (globalThis as any).module = (globalThis as any).module ?? {}; + (globalThis as any).module.exports = null;
113-121: Keep loadScript consistent and avoid clobberingmodule.Mirror the safer export reset used above and consider factoring manifest resolution into a helper to avoid duplication between sync/async loaders.
loadScript: (sourceURL: string, entryName?: string) => { entryName = entryName ?? '__Card__'; - const manifestUrl = templateCache.get(entryName!) + const manifestUrl = templateCache.get(entryName) ?.manifest[`/${sourceURL}`]; if (manifestUrl) sourceURL = manifestUrl; - Object.assign(globalThis, { module: { exports: null } }); + (globalThis as any).module = (globalThis as any).module ?? {}; + (globalThis as any).module.exports = null; importScripts(sourceURL); return createBundleInitReturnObj(); },packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
57-58: Minor: add "use strict" inside manifest init for parity and predictabilityLepus code runs under
"use strict". Add it to the manifestinitwrapper for consistent semantics.- `module.exports = {init: (lynxCoreInject) => { var {${defaultInjectStr}} = lynxCoreInject.tt; var module = {exports:{}}; var exports=module.exports; ${value}\n return module.exports; } }`, + `module.exports = {init: (lynxCoreInject) => { "use strict"; var {${defaultInjectStr}} = lynxCoreInject.tt; var module = {exports:{}}; var exports=module.exports; ${value}\n return module.exports; } }`,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (52)
.changeset/fluffy-masks-mate.md(1 hunks).changeset/loose-ads-add.md(1 hunks)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/LynxModule.ts(1 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-constants/src/types/MarkTiming.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-constants/src/types/TemplateLoader.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(2 hunks)packages/web-platform/web-core-server/src/createLynxView.ts(1 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(5 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts(2 hunks)packages/web-platform/web-tests/rspack.config.js(1 hunks)packages/web-platform/web-tests/tests/performance.test.ts(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(7 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(4 hunks)packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/loose-ads-add.md
🚧 Files skipped from review as they are similar to previous changes (43)
- packages/web-platform/web-constants/src/types/LynxModule.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx
- packages/web-platform/web-constants/src/types/index.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css
- packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css
- packages/web-platform/web-constants/src/types/TemplateLoader.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css
- packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx
- packages/web-platform/web-core/src/utils/loadTemplate.ts
- .changeset/fluffy-masks-mate.md
- packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-tests/rspack.config.js
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx
- packages/web-platform/web-core-server/src/createLynxView.ts
- packages/web-platform/web-constants/src/types/MarkTiming.ts
- packages/web-platform/web-tests/tests/performance.test.ts
- packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
- packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx
🧰 Additional context used
🧠 Learnings (7)
📚 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/startBackgroundThread.ts
📚 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-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.tspackages/web-platform/web-core/src/uiThread/startUIThread.ts
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
📚 Learning: 2025-08-13T11:36:12.075Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1523
File: vitest.config.ts:52-72
Timestamp: 2025-08-13T11:36:12.075Z
Learning: The lynx-stack project requires Node.js >=22 as specified in package.json engines, so Node.js compatibility fallbacks for features introduced before v22 are unnecessary.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
📚 Learning: 2025-08-14T12:54:51.143Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: .changeset/brave-melons-add.md:1-7
Timestamp: 2025-08-14T12:54:51.143Z
Learning: In the lynx-family/lynx-stack repository, packages use 0.x.x versioning where minor version bumps indicate breaking changes (not major bumps), following pre-1.0 semantic versioning conventions.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.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/createNativeApp.ts
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-core/src/uiThread/startUIThread.ts
🧬 Code graph analysis (5)
packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx (3)
packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx (3)
importPath(6-6)LazyComponent(7-16)App(30-37)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx (5)
importPath(6-6)LazyComponent(7-15)App(17-38)shouldDisplay(18-18)handleClick(19-21)packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx (1)
App(6-27)
packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx (2)
packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx (1)
App(7-9)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx (1)
App(7-9)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (3)
packages/web-platform/web-constants/src/endpoints.ts (3)
queryComponentEndpoint(248-251)reportErrorEndpoint(85-88)updateBTSTemplateCacheEndpoint(253-256)packages/web-platform/web-constants/src/types/NativeApp.ts (1)
BundleInitReturnObj(75-85)packages/web-platform/web-constants/src/types/LynxModule.ts (2)
LynxJSModule(44-46)LynxTemplate(21-42)
packages/web-platform/web-core/src/uiThread/startUIThread.ts (4)
packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-constants/src/types/MarkTiming.ts (1)
MarkTimingInternal(11-15)packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
createTemplateLoader(14-45)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (1)
createRenderAllOnUI(87-209)
packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts (1)
packages/web-platform/offscreen-document/src/webworker/index.ts (1)
textContent(7-7)
⏰ 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). (4)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: CodeQL Analyze (javascript-typescript)
🔇 Additional comments (10)
packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx (1)
7-15: Lazy import shape looks correct for component JSON bundles.Using lazy(() => import(importPath, { with: { type: 'component' } })) aligns with the new loader. No issues spotted here.
packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx (1)
7-8: Confirm absolute /dist paths resolve under the test server.If the test server serves under a non-root basePath, leading slashes can 404. Consider relative URLs or a shared base constant if needed.
Also applies to: 17-18
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
17-20: LGTM: imports align with new lazy-component flow.No issues spotted with the added types and endpoints.
67-69: LGTM: RPC handle for queryComponent created.Follow-ups about consumption are below.
packages/web-platform/web-core/src/uiThread/startUIThread.ts (6)
17-19: Imports look correct.TemplateLoader and MarkTimingInternal usage aligns with downstream types.
20-20: Good move to centralize loading via factory.This keeps timing and decoding concerns in one place.
63-67: Timing wrapper LGTM.Typed MarkTimingInternal passthrough keeps pipelineId/timeStamp propagation intact.
78-81: Factory instantiation LGTM.Wires custom loader with timing consistently.
86-87: All-on-UI path wired to TemplateLoader.Looks good.
95-96: Multi-thread path wired to TemplateLoader.Looks good.
Sherry-hue
changed the title
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
e4e365b to
3a908dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
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-core/src/uiThread/startUIThread.ts (1)
99-105: Handle templateLoader rejection to avoid unhandled rejections and to flush timings.On network/parse errors, the promise rejects and timings aren’t flushed; error isn’t surfaced via
onError.Apply:
- templateLoader(templateUrl).then((template) => { - flushMarkTimingInternal(); - start({ - ...configs, - template, - }); - }); + templateLoader(templateUrl) + .then((template) => { + flushMarkTimingInternal(); + start({ + ...configs, + template, + }); + }) + .catch((err) => { + // Ensure timings are flushed even on failure + flushMarkTimingInternal(); + callbacks.onError?.(err as Error, '', 'loadTemplate'); + });
♻️ Duplicate comments (4)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
70-93: Fix:moduleis undefined inside the IIFE for lazy bundles—use earlyreturninstead ofmodule.exports=The generated IIFE has no
modulebinding, soappType === 'lazy' ? 'module.exports=\n' : ''throws at runtime. Lazy should return the component directly.Apply:
[ eager ? '//# allFunctionsCalledOnLoad' : '', '\n(function() { "use strict"; const ', globalDisallowedVars.join('=void 0,'), '=void 0;\n', - appType === 'lazy' ? 'module.exports=\n' : '', + appType === 'lazy' ? 'return ' : '', content, '\n})()', ].join('');
143-158: Remove non-null assertions ontemplate.appType; add backward-compatible fallbackOlder v2 templates may not carry
appType, causing a crash. Compute a safe localappTypeonce and reuse it for bothlepusCodeandmanifest.Apply:
return { ...template, + // Back-compat: infer appType when missing (will be explicit in newer encoders) + // Heuristic aligns with existing lazy-bundle detection. + // TODO: replace heuristic once all producers emit appType. + ...(template as any), + // local binding for use below + // (kept inline in this diff for minimal patch footprint) + } as any;And replace the two call sites by first introducing
const inferredAppTypejust above the return:while ( template.version! < currentSupportedTemplateVersion && (upgrader = templateUpgraders[template.version! - 1]) ) { template = upgrader(template); } - return { + const inferredAppType = + template.appType ?? + (template.lepusCode.root.trimStart().startsWith('(function (globDynamicComponentEntry') + ? 'lazy' + : 'card'); + return { ...template, lepusCode: await generateJavascriptUrl( template.lepusCode, createJsModuleUrl as (content: string, name: string) => Promise<string>, true, - template.appType!, + inferredAppType, templateName, ), manifest: await generateJavascriptUrl( template.manifest, createJsModuleUrl as (content: string, name: string) => Promise<string>, false, - template.appType!, + inferredAppType, templateName, ), };Also applies to: 148-149, 155-156
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts (1)
49-55: Fix TS breakage: don’t destructure optional exports; keep compat with lynx-core ≤0.1.2 (or bump peerDep).CI shows TS errors for
nativeGlobal/loadDynamicComponentwhen building against 0.1.2. Read these via guarded access instead of destructuring, or mandate 0.1.3+. Idempotent inject to avoid overwriting an existing loader.- lynxCore.then( - ( - { - loadCard, - destroyCard, - callDestroyLifetimeFun, - nativeGlobal, - loadDynamicComponent, - }, - ) => { - // @lynx-js/lynx-core >= 0.1.3 will export nativeGlobal and loadDynamicComponent - if (nativeGlobal && loadDynamicComponent) { - nativeGlobal.loadDynamicComponent = loadDynamicComponent; - } + lynxCore.then((core) => { + // Destructure only stable exports + const { loadCard, destroyCard, callDestroyLifetimeFun } = + core as typeof import('@lynx-js/lynx-core/web'); + // Optional exports on >=0.1.3 + const maybeNativeGlobal = + (core as any).nativeGlobal as undefined | { loadDynamicComponent?: unknown }; + const maybeLoadDynamic = + (core as any).loadDynamicComponent as undefined | ((...args: any[]) => any); + // Wire once + if (maybeNativeGlobal && maybeLoadDynamic && !maybeNativeGlobal.loadDynamicComponent) { + (maybeNativeGlobal as any).loadDynamicComponent = maybeLoadDynamic; + }packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
26-31: Ensure all customTemplateLoader implementations match the TemplateLoader signature.Implementations must be
(url: string) => Promise<LynxTemplate>. Past commits showed zero-arg loaders.Run:
#!/bin/bash set -euo pipefail # Find implementations with 0 params or wrong arity rg -nP --type=ts '\bcustomTemplateLoader\s*:\s*async\s*\(\s*\)\s*=>' -C2 || true rg -nP --type=ts '\bcustomTemplateLoader\s*:\s*\([^)]*\)\s*=>\s*Promise<[^>]*>' -C2 || true # Show all occurrences for manual scan rg -nP --type=ts '\bcustomTemplateLoader\b' -n -C2
🧹 Nitpick comments (7)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (1)
778-782: Bind the callback to preservethisand avoid subtle context bugsIf the producer implementation relies on
this, passing the bare function can break it. Bind at wiring.- __QueryComponent: callbacks.__QueryComponent, + __QueryComponent: callbacks.__QueryComponent.bind(callbacks),packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
95-112: Propagation ofappTypeis OK, but consider documenting expectations
generateJavascriptUrlnow depends onappTypeto decide wrapper semantics. Add a short JSDoc on how callers should choose it to avoid misuse.Also applies to: 99-109
packages/web-platform/web-tests/tests/react.spec.ts (3)
446-446: Grammar nit: fix skip reasonUse “are not supported” for clarity and consistency.
- test.skip(isSSR, 'Lazy Component not support on SSR'); + test.skip(isSSR, 'Lazy components are not supported on SSR');Apply the same edit to the other lazy tests in this block.
Also applies to: 471-471, 495-495, 505-505, 519-519, 555-555, 591-591, 609-609, 627-627, 648-648, 675-675
447-466: Reduce flakiness by removing fixed waits; rely on Playwright auto-waittoHaveCSS already polls; the extra waits after goto/clicks can slow tests and add timing sensitivity.
await goto(page, title); - await wait(500); await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green await page.locator('#target2').click(); - await wait(100); await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(255, 192, 203)', ); // pink await page.locator('#target2').click(); - await wait(100); await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green(Consider the same cleanup in the neighboring lazy tests.)
498-500: Prefer assertion auto-wait over manual innerText readUse toHaveText to get built-in retries and reduce brittle timing.
- const result = await page.locator('#fallback').first().innerText(); - expect(result).toBe('Loading...'); + await expect(page.locator('#fallback').first()).toHaveText('Loading...');packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts (2)
58-60: Make the injection idempotent (don’t overwrite an existing loader).Avoid clobbering if something already set it earlier or in another thread.
- if (nativeGlobal && loadDynamicComponent) { - nativeGlobal.loadDynamicComponent = loadDynamicComponent; - } + if (nativeGlobal && loadDynamicComponent && !nativeGlobal.loadDynamicComponent) { + (nativeGlobal as any).loadDynamicComponent = loadDynamicComponent; + }
48-56: Nit: remove redundant parens around the parameter.The extra wrapping parens reduce readability.
- lynxCore.then( - ( - { + lynxCore.then( + { /* … */ - }, - ) => { + } => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
.changeset/fluffy-masks-mate.md(1 hunks).changeset/loose-ads-add.md(1 hunks)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-constants/src/types/MarkTiming.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-constants/src/types/TemplateLoader.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(6 hunks)packages/web-platform/web-core-server/src/createLynxView.ts(1 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(5 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts(2 hunks)packages/web-platform/web-tests/rspack.config.js(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(7 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(4 hunks)packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (44)
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css
- packages/web-platform/web-constants/src/types/index.ts
- packages/web-platform/web-tests/rspack.config.js
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css
- .changeset/loose-ads-add.md
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css
- packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts
- packages/web-platform/web-constants/src/types/MarkTiming.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx
- packages/web-platform/web-constants/src/types/TemplateLoader.ts
- .changeset/fluffy-masks-mate.md
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css
- packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx
- packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
- packages/webpack/template-webpack-plugin/src/WebEncodePlugin.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-core-server/src/createLynxView.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-core/src/utils/loadTemplate.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
🧰 Additional context used
📓 Path-based instructions (1)
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.ts
🧠 Learnings (6)
📚 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/startBackgroundThread.ts
📚 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-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.tspackages/web-platform/web-core/src/uiThread/startUIThread.ts
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
📚 Learning: 2025-08-13T11:36:12.075Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1523
File: vitest.config.ts:52-72
Timestamp: 2025-08-13T11:36:12.075Z
Learning: The lynx-stack project requires Node.js >=22 as specified in package.json engines, so Node.js compatibility fallbacks for features introduced before v22 are unnecessary.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
📚 Learning: 2025-08-14T12:54:51.143Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: .changeset/brave-melons-add.md:1-7
Timestamp: 2025-08-14T12:54:51.143Z
Learning: In the lynx-family/lynx-stack repository, packages use 0.x.x versioning where minor version bumps indicate breaking changes (not major bumps), following pre-1.0 semantic versioning conventions.
Applied to files:
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
📚 Learning: 2025-08-13T11:46:43.737Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1523
File: vitest.config.ts:5-6
Timestamp: 2025-08-13T11:46:43.737Z
Learning: In the lynx-stack codebase, default imports are consistently used for Node.js built-in modules (e.g., `import os from 'node:os'`, `import fs from 'node:fs'`). The TypeScript configuration supports esModuleInterop and allowSyntheticDefaultImports, making default imports the preferred pattern over namespace imports for Node.js built-ins.
Applied to files:
packages/web-platform/web-core/src/uiThread/startUIThread.ts
🧬 Code graph analysis (3)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (4)
packages/web-platform/web-worker-rpc/src/Rpc.ts (1)
Rpc(38-473)packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
StartUIThreadCallbacks(26-31)packages/web-platform/web-constants/src/endpoints.ts (1)
loadTemplateMultiThread(258-261)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (1)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
QueryComponentPAPI(305-314)
packages/web-platform/web-core/src/uiThread/startUIThread.ts (3)
packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-constants/src/types/MarkTiming.ts (1)
MarkTimingInternal(11-15)packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
createTemplateLoader(14-45)
⏰ 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). (4)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / clippy
- GitHub Check: test-rust / Test (Ubuntu)
- GitHub Check: code-style-check
🔇 Additional comments (10)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (2)
60-63: LGTM: type import is correct and aligns contractsImporting
QueryComponentPAPIhere keeps the surface in lockstep with web-constants.
136-139: Surface contract extension verified — signatures matchQueryComponentPAPI is declared as (source: string, resultCallback?: ...) => string | null in packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts; createQueryComponent returns an implementation typed as QueryComponentPAPI (packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts) and is wired into the main-thread global in prepareMainThreadAPIs; no arity or return-type mismatches found.
packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
62-62: Manifest wrapper looks correct and safer than v1Initializing
module = {exports:{}}and aliasingexportsinsideinitis good for isolating per-entry state.
73-74: Type narrowing for appType is fineExplicit union
'card' | 'lazy'here is clear and matches downstream use.packages/web-platform/web-tests/tests/react.spec.ts (1)
442-711: LGTM: solid lazy-component coverage with proper awaits and SSR gatingThe added suite thoroughly exercises lazy loading paths and uses awaited Playwright assertions correctly. Location is compliant with the web-tests guideline.
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts (1)
48-60: Optionally report init/import wiring failures to main thread (lightweight)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts:48-60
- package.jsons show @lynx-js/lynx-core = 0.1.3 (includes web-worker-runtime) — export-availability claim for nativeGlobal/loadDynamicComponent is consistent.
- ripgrep search for nativeGlobal/loadDynamicComponent returned "No files were searched" — I could not locate other injection sites; re-run a global search or confirm runtime injectors.
- Action: on import() or wiring failure send a minimal mainThreadRpc error (short code + origin) to aid debugging; keep payload tiny to avoid perf impact.
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (2)
19-24: Signature + type wiring LGTM.Adding
loadTemplate: TemplateLoaderandcallbacks: StartUIThreadCallbacksaligns with the new loader abstraction and keeps error reporting plumbed.
29-29: OK to keep structured-clone — LynxTemplate is cloneableI inspected packages/web-platform/web-constants/src/types/LynxModule.ts and the sample templates: LynxTemplate fields (styleInfo, pageConfig, customSections, lepusCode, manifest, etc.) are plain JSON-serializable values (strings/objects/arrays — lepusCode and inlined images are strings). I found no Function, MessagePort, ArrayBuffer/Uint8Array/Blob/ImageData or other non-cloneable fields, so returning the template via registerHandler(loadTemplateMultiThread, loadTemplate) is safe. Change to the transfer variant only if you later add binary/transferable fields.
packages/web-platform/web-core/src/uiThread/startUIThread.ts (2)
78-81: Loader integration LGTM.Using
createTemplateLoader(customTemplateLoader, markTimingInternal)centralizes timing and caching; matches the newTemplateLoadercontract.
92-97: SSR is only wired for all-on-ui; confirm this is intentional.
ssris passed tocreateRenderAllOnUIbut not tocreateRenderMultiThread. If SSR should also apply in multi-thread mode, wire it similarly.
Sherry-hue
force-pushed
the
feat/web-dynamic-component
branch
from
3a908dc to
1587f4f
Compare
PupilTong
left a comment
PupilTong
left a comment
There was a problem hiding this comment.
LGTM, please double check some chunking loading behaviors
There was a problem hiding this comment.
Actionable comments posted: 0
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-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
120-124: Initialize globalThis.module before importScripts to avoid crashes when undefined.Directly writing
globalThis.module.exportscan throw ifmoduleisn’t initialized.- globalThis.module.exports = null; - globalThis.__bundle__holder = null; - importScripts(sourceURL); - return createBundleInitReturnObj(); + (globalThis as any).module = (globalThis as any).module ?? {}; + (globalThis as any).module.exports = null; + (globalThis as any).__bundle__holder = null; + importScripts(sourceURL); + return createBundleInitReturnObj();
♻️ Duplicate comments (5)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (3)
86-93: Use early return for lazy bundles;moduleis undefined hereThe wrapper should return the component for
lazy; assigning tomodule.exportsinside the IIFE causesReferenceError.- appType === 'lazy' ? 'module.exports=\n' : '', + appType === 'lazy' ? 'return ' : '',
142-158: Remove non-null assertions onappType; add backward-compatible fallbackOlder v2 templates may lack
appType. Compute once and reuse.- return { - ...template, - lepusCode: await generateJavascriptUrl( + const appType = + template.appType ?? + (template.lepusCode.root.trimStart().startsWith('(function (globDynamicComponentEntry') ? 'lazy' : 'card'); + + return { + ...template, + lepusCode: await generateJavascriptUrl( template.lepusCode, createJsModuleUrl as (content: string, name: string) => Promise<string>, true, - template.appType!, + appType, templateName, ), manifest: await generateJavascriptUrl( template.manifest, createJsModuleUrl as (content: string, name: string) => Promise<string>, false, - template.appType!, + appType, templateName, ), };
62-63: Top-levelmodule.exportshere will throw inside the IIFE
moduleisn’t defined in the wrapper; export a plain object literal and let the wrapper handle return semantics.- `module.exports={init: (lynxCoreInject) => { var {${defaultInjectStr}} = lynxCoreInject.tt; var module = {exports:{}}; var exports=module.exports; ${value}\n return module.exports; } }`, + `({init: (lynxCoreInject) => { var {${defaultInjectStr}} = lynxCoreInject.tt; var module = {exports:{}}; var exports=module.exports; ${value}\n return module.exports; } })`,packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
106-113: Fix double-callback and ensure single-invocation in loadScriptAsync; also report and stringify errors.Current
.catch(callback).then(...)fires the success path after failures, causing double-callbacks. Also, pass a string tocallbackand report viareportError.- globalThis.module.exports = null; - globalThis.__bundle__holder = null; - import( - /* webpackIgnore: true */ - sourceURL - ).catch(callback).then(async () => { - callback(null, createBundleInitReturnObj()); - }); + (globalThis as any).module = (globalThis as any).module ?? {}; + (globalThis as any).module.exports = null; + (globalThis as any).__bundle__holder = null; + import( + /* webpackIgnore: true */ + sourceURL + ) + .then(() => { + callback(null, createBundleInitReturnObj()); + }) + .catch((err) => { + reportError(err as Error, undefined, release); + callback(err instanceof Error ? err.message : String(err)); + });
76-82: No success-notification path for queryComponent; callbacks aren’t flushed when template arrives, and failures aren’t handled.Track pending callbacks per source, flush them in the update handler, and handle RPC errors.
const templateCache = new Map<string, LynxTemplate>([['__Card__', template]]); + const pendingQueryComponentCallbacks: Record< + string, + Array<(ret: { __hasReady: boolean } | { code: number; detail?: { schema: string } }) => void> + > = {}; mainThreadRpc.registerHandler( updateBTSTemplateCacheEndpoint, (url, template) => { templateCache.set(url, template); + const cbs = pendingQueryComponentCallbacks[url]; + if (cbs?.length) { + for (const cb of cbs) cb({ __hasReady: true }); + delete pendingQueryComponentCallbacks[url]; + } }, ); @@ queryComponent: (source, callback) => { if (templateCache.has(source)) { callback({ __hasReady: true }); } else { - queryComponent(source).then(res => { - callback?.(res); - }); + (pendingQueryComponentCallbacks[source] ??= []).push(callback); + queryComponent(source) + .then((res: any) => { + // If UI reports an error via { code }, surface it and clean up this callback. + if (res && typeof res.code === 'number' && res.code !== 0) { + const arr = pendingQueryComponentCallbacks[source]; + if (arr) { + const idx = arr.indexOf(callback); + if (idx >= 0) arr.splice(idx, 1); + if (arr.length === 0) delete pendingQueryComponentCallbacks[source]; + } + callback(res); + } + // Success path will be signaled by updateBTSTemplateCacheEndpoint. + }) + .catch((err: Error) => { + const arr = pendingQueryComponentCallbacks[source]; + if (arr) { + const idx = arr.indexOf(callback); + if (idx >= 0) arr.splice(idx, 1); + if (arr.length === 0) delete pendingQueryComponentCallbacks[source]; + } + reportError(err, undefined, release); + callback({ code: -1, detail: { schema: 'queryComponent rpc failed' } } as any); + }); } },Also applies to: 175-183
🧹 Nitpick comments (6)
packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx (1)
27-27: Nit: use self-closing JSX.Minor readability tweak.
-root.render(<App></App>); +root.render(<App />);packages/web-platform/web-tests/tests/react.spec.ts (4)
442-711: Trim fixed waits; rely on Playwright’s auto-wait for stability.Hard sleeps (500/100ms) make these lazy tests flaky. Use assertion auto-wait (and event-driven waits) instead. Example for one case—apply similarly to others:
test( 'basic-lazy-component', async ({ page }, { title }) => { test.skip(isSSR, 'Lazy Component not support on SSR'); await goto(page, title); - await wait(500); - await expect(page.locator('#target1')).toHaveCSS( + await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green await page.locator('#target2').click(); - await wait(100); - await expect(page.locator('#target1')).toHaveCSS( + await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(255, 192, 203)', ); // pink await page.locator('#target2').click(); - await wait(100); - await expect(page.locator('#target1')).toHaveCSS( + await expect(page.locator('#target1')).toHaveCSS( 'background-color', 'rgb(0, 128, 0)', ); // green }, );
493-501: Prefer assertion auto-wait for fallback text.This removes the need for a prior sleep and is more robust.
- await wait(500); - const result = await page.locator('#fallback').first().innerText(); - expect(result).toBe('Loading...'); + await expect(page.locator('#fallback').first()).toHaveText('Loading...');
446-446: Fix skip message grammar.Use “not supported on SSR”.
- test.skip(isSSR, 'Lazy Component not support on SSR'); + test.skip(isSSR, 'Lazy Component not supported on SSR');Also applies to: 471-471, 495-495, 505-505, 518-518, 555-555, 592-592, 609-609, 627-627, 648-648, 675-675
442-711: Mark block as slow to reduce CI flakes.Optional: set a slower timeout for the whole lazy block.
- // lazy component - test( + // lazy component + test.slow(); + test( 'basic-lazy-component',packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
103-105: Remove unnecessary non-null assertions on entryName.
entryNameis defaulted; the!is redundant.- const manifestUrl = templateCache.get(entryName!) + const manifestUrl = templateCache.get(entryName) ?.manifest[`/${sourceURL}`]; @@ - const manifestUrl = templateCache.get(entryName!) + const manifestUrl = templateCache.get(entryName) ?.manifest[`/${sourceURL}`];Also applies to: 117-119
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
.changeset/fluffy-masks-mate.md(1 hunks)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-constants/src/types/MarkTiming.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-constants/src/types/TemplateLoader.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(6 hunks)packages/web-platform/web-core-server/src/createLynxView.ts(1 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(5 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts(2 hunks)packages/web-platform/web-tests/rspack.config.js(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(7 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css
🚧 Files skipped from review as they are similar to previous changes (43)
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css
- packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx
- packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
- .changeset/fluffy-masks-mate.md
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx
- packages/web-platform/web-constants/src/types/MarkTiming.ts
- packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts
- packages/web-platform/web-core-server/src/createLynxView.ts
- packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx
- packages/web-platform/web-constants/src/types/TemplateLoader.ts
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
- packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx
- packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts
- packages/web-platform/web-tests/rspack.config.js
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx
- packages/web-platform/web-core/src/utils/loadTemplate.ts
- packages/web-platform/web-constants/src/types/index.ts
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx
🧰 Additional context used
📓 Path-based instructions (1)
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/basic-lazy-component-effect/index.jsxpackages/web-platform/web-tests/tests/react.spec.ts
🧠 Learnings (1)
📚 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/createNativeApp.ts
🧬 Code graph analysis (2)
packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx (1)
packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx (1)
App(6-19)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (3)
packages/web-platform/web-constants/src/endpoints.ts (3)
queryComponentEndpoint(248-251)reportErrorEndpoint(85-88)updateBTSTemplateCacheEndpoint(253-256)packages/web-platform/web-constants/src/types/NativeApp.ts (1)
BundleInitReturnObj(75-85)packages/web-platform/web-constants/src/types/LynxModule.ts (1)
LynxTemplate(21-42)
⏰ 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 (6)
packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx (1)
6-15: Lazy import wiring looks correct.Path and import attribute match the lazy bundle pattern; good test fixture for the spec.
packages/web-platform/web-tests/tests/react.spec.ts (2)
442-711: Coverage and placement LGTM.Tests live under packages/web-platform/web-tests/** and exercise the lazy-bundle paths thoroughly.
442-711: Verified fixtures exist for each case name.
All referenced test case directories under packages/web-platform/web-tests/tests/react contain an index file (jsx/tsx/js/ts).packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
73-74: ThreadingappTypeinto the wrapper — LGTM
95-112: PropagatingappTypeintogenerateJavascriptUrl— LGTMSignature and threading look consistent.
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)
76-82: Confirm cache key semantics: unify entryName vs URL used for templateCacheupdateBTSTemplateCacheEndpoint handler does templateCache.set(url, template) (packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts) but lookups use entryName/source (templateCache.get(entryName) in loadScript/loadScriptAsync and templateCache.has(source) in queryComponent). Ensure the string passed as "url" from createQueryComponent.ts (updateBTSTemplateCache(url, template)) is the same key used for lookups — otherwise store the template under the entryName (or store both keys / normalize keys) to avoid manifest-resolution cache misses.
feat(web): all queryComponents of mts and bts are fetched in the ui thread feat(web): impl lazy component when needed && add tests patch & rebase fix: remove lazy component judge by content
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
21-43: Always close timing on errors and validate HTTP status.On exceptions, load_template_end is skipped; non-2xx responses aren’t checked. Wrap miss-path in try/finally and guard resp.ok.
Apply:
markTimingInternal('load_template_start'); const cachedTemplate = templateCache[url]; if (cachedTemplate) { markTimingInternal('load_template_end'); return cachedTemplate; } - const template = customTemplateLoader - ? await customTemplateLoader(url) - : (await (await fetch(url, { - method: 'GET', - })).json()) as LynxTemplate; - const decodedTemplate = await generateTemplate( - template, - createJsModuleUrl, - ); - templateCache[url] = decodedTemplate; - /** - * This will cause a memory leak, which is expected. - * We cannot ensure that the `URL.createObjectURL` created url will never be used, therefore here we keep it for the entire lifetime of this page. - */ - markTimingInternal('load_template_end'); - return decodedTemplate; + try { + const template = customTemplateLoader + ? await customTemplateLoader(url) + : await (async () => { + const resp = await fetch(url, { method: 'GET' }); + if (!resp.ok) { + throw new Error(`Failed to fetch template: ${url} (${resp.status})`); + } + return (await resp.json()) as LynxTemplate; + })(); + const decodedTemplate = await generateTemplate( + template, + createJsModuleUrl, + ); + templateCache[url] = decodedTemplate; + /** + * This will cause a memory leak, which is expected. + * We cannot ensure that the `URL.createObjectURL` created url will never be used, therefore here we keep it for the entire lifetime of this page. + */ + return decodedTemplate; + } finally { + markTimingInternal('load_template_end'); + }packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)
90-93: Bug: module is undefined inside the IIFE; use early return for lazy.Using
module.exports=here throws at runtime. Return the component for lazy bundles.Apply:
- appType === 'lazy' ? 'module.exports=\n' : '', + appType === 'lazy' ? 'return ' : '',
148-156: Avoid non-null assertion on appType; add safe fallback.v2 templates may lack appType; pass a derived default to prevent crashes/mis-generation.
Apply:
- template.appType!, + (template.appType + ?? (template.lepusCode.root.trimStart() + .startsWith('(function (globDynamicComponentEntry') ? 'lazy' : 'card')),and
- template.appType!, + (template.appType + ?? (template.lepusCode.root.trimStart() + .startsWith('(function (globDynamicComponentEntry') ? 'lazy' : 'card')),Optionally, hoist this into a local const
appTypeonce and reuse.
🧹 Nitpick comments (6)
packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css (2)
1-5: Ensure width/height apply to custom elements; set display explicitly.Unknown/custom tags default to inline; width/height won't apply unless another stylesheet sets display. Make the test self-contained to avoid flakiness.
.container { + display: block; width: 100px; height: 100px; background-color: red; }
1-5: Reduce cross-test CSS collisions by namespacing the class."container" is generic and may clash with other test assets. Consider renaming to something test-specific (e.g., ".lazy-css-container") and updating the JSX accordingly.
packages/web-platform/web-constants/src/types/MarkTiming.ts (1)
11-15: Type shape looks good; consider documenting units and monotonic clock.Add a short JSDoc noting whether timeStamp is epoch ms or performance.now() and whether callers should pass a monotonic value.
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (1)
29-29: Consider unregister/teardown on disposal.If the UI thread is torn down/restarted, we should unregister the handler to avoid leaks or stale handlers.
packages/web-platform/web-core/src/utils/loadTemplate.ts (1)
18-43: Deduplicate concurrent loads to avoid double fetch/duplicate decode.Cache an in-flight promise per URL and share it across callers.
Apply:
export function createTemplateLoader( customTemplateLoader: TemplateLoader | undefined, markTimingInternal: MarkTimingInternal, ) { + const inFlight: Record<string, Promise<LynxTemplate>> = {}; const loadTemplate: TemplateLoader = async ( url: string, ) => { - markTimingInternal('load_template_start'); + if (inFlight[url]) return inFlight[url]; + markTimingInternal('load_template_start'); const cachedTemplate = templateCache[url]; if (cachedTemplate) { markTimingInternal('load_template_end'); return cachedTemplate; } - try { - const template = customTemplateLoader - ? await customTemplateLoader(url) - : await (async () => { - const resp = await fetch(url, { method: 'GET' }); - if (!resp.ok) { - throw new Error(`Failed to fetch template: ${url} (${resp.status})`); - } - return (await resp.json()) as LynxTemplate; - })(); - const decodedTemplate = await generateTemplate( - template, - createJsModuleUrl, - ); - templateCache[url] = decodedTemplate; - /** - * This will cause a memory leak, which is expected. - * We cannot ensure that the `URL.createObjectURL` created url will never be used, therefore here we keep it for the entire lifetime of this page. - */ - return decodedTemplate; - } finally { - markTimingInternal('load_template_end'); - } + inFlight[url] = (async () => { + try { + const template = customTemplateLoader + ? await customTemplateLoader(url) + : await (async () => { + const resp = await fetch(url, { method: 'GET' }); + if (!resp.ok) { + throw new Error(`Failed to fetch template: ${url} (${resp.status})`); + } + return (await resp.json()) as LynxTemplate; + })(); + const decodedTemplate = await generateTemplate( + template, + createJsModuleUrl, + ); + templateCache[url] = decodedTemplate; + return decodedTemplate; + } finally { + markTimingInternal('load_template_end'); + delete inFlight[url]; + } + })(); + return inFlight[url]; }; return loadTemplate; }packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
51-55: Heuristic for appType is brittle; prefer explicit metadata when available.If newer encoders add appType explicitly, read that first and only fall back to the startsWith check.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
.changeset/fluffy-masks-mate.md(1 hunks)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts(1 hunks)packages/web-platform/web-constants/src/endpoints.ts(2 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-constants/src/types/MarkTiming.ts(1 hunks)packages/web-platform/web-constants/src/types/NativeApp.ts(2 hunks)packages/web-platform/web-constants/src/types/TemplateLoader.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(6 hunks)packages/web-platform/web-core-server/src/createLynxView.ts(1 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts(1 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(5 hunks)packages/web-platform/web-core/src/utils/loadTemplate.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts(1 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts(2 hunks)packages/web-platform/web-tests/rspack.config.js(1 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts(1 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts(7 hunks)packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts(1 hunks)packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (43)
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.css
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.css
- packages/web-platform/web-tests/tests/react/config-lazy-component-bindtap/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-fail/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-css/index.css
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.jsx
- packages/web-platform/web-tests/rspack.config.js
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.css
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-blank/index.jsx
- packages/web-platform/web-constants/src/endpoints.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-when-needed/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-use-effect/index.jsx
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-effect/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component-css-other/index.jsx
- packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
- packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createBackgroundLynx.ts
- packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/startBackgroundThread.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi/index.jsx
- .changeset/fluffy-masks-mate.md
- packages/web-platform/web-tests/tests/react/basic-lazy-component-relative-path/index.jsx
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-constants/src/types/NativeApp.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-blank/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-when-need-with-itself/index.jsx
- packages/web-platform/web-tests/tests/react/basic-lazy-component-css-multi/index.css
- packages/web-platform/web-core-server/src/createLynxView.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component/index.jsx
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-tests/tests/react/basic-lazy-component-multi-import/index.jsx
- packages/web-platform/web-tests/tests/react/config-lazy-component.config.ts
- packages/web-platform/web-mainthread-apis/src/crossThreadHandlers/createQueryComponent.ts
- packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts
- packages/web-platform/web-constants/src/types/TemplateLoader.ts
- packages/web-platform/web-constants/src/types/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
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/basic-lazy-component-css/index.css
🧠 Learnings (1)
📚 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-core/src/utils/loadTemplate.ts
🧬 Code graph analysis (2)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (4)
packages/web-platform/web-worker-rpc/src/Rpc.ts (1)
Rpc(38-473)packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
StartUIThreadCallbacks(26-31)packages/web-platform/web-constants/src/endpoints.ts (1)
loadTemplateMultiThread(258-261)
packages/web-platform/web-core/src/utils/loadTemplate.ts (3)
packages/web-platform/web-constants/src/types/TemplateLoader.ts (1)
TemplateLoader(3-3)packages/web-platform/web-constants/src/types/MarkTiming.ts (1)
MarkTimingInternal(11-15)packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
generateTemplate(122-159)
⏰ 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). (1)
- GitHub Check: build / Build (Windows)
🔇 Additional comments (2)
packages/web-platform/web-core/src/uiThread/createRenderMultiThread.ts (1)
19-24: Signature + handler wiring LGTM.Type-only import for StartUIThreadCallbacks avoids runtime cycles; registering loadTemplate as handler matches endpoint params/return.
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
95-112: Confirm that non-string entries are intentionally dropped.generateJavascriptUrl filters out non-string values; verify no manifest/lepusCode entries legitimately carry objects that must be preserved.
4 participants
Summary
Checklist
Summary by CodeRabbit
New Features
Refactor
Tests
Chores
Other