feat: support SSR for all-on-ui Implement SSR for Web #1029
Conversation
🦋 Changeset detectedLatest commit: 41b0e40 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
PupilTong
force-pushed
the
p/hw/ssr-hydrate-3.2
branch
2 times, most recently
from
31f4e23 to
06dc59f
Compare
CodSpeed Performance ReportMerging #1029 will not alter performanceComparing Summary
|
React Example#2997 Bundle Size — 234.12KiB (0%).41b0e40(current) vs 79d1825 main#2980(baseline) Bundle metrics
|
| Current #2997 |
Baseline #2980 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
153 |
153 |
| Duplicate Modules |
61 |
61 |
| Duplicate Code |
45.85% |
45.85% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #2997 |
Baseline #2980 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
88.37KiB |
88.37KiB |
Bundle analysis report Branch PupilTong:p/hw/ssr-hydrate-3.2 Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#2988 Bundle Size — 304.54KiB (+0.31%).41b0e40(current) vs 79d1825 main#2971(baseline) Bundle metrics
|
| Current #2988 |
Baseline #2971 |
|
|---|---|---|
 Initial JS |
140.55KiB(+0.44%) |
139.94KiB |
| Initial CSS |
31.88KiB |
31.88KiB |
 Cache Invalidation |
53.4% |
0% |
| Chunks |
4 |
4 |
| Assets |
5 |
5 |
| Modules |
205(+0.49%) |
204 |
| Duplicate Modules |
17 |
17 |
| Duplicate Code |
4.23%(-0.24%) |
4.24% |
| Packages |
3 |
3 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #2988 |
Baseline #2971 |
|
|---|---|---|
| JS |
221.78KiB (+0.42%) |
220.85KiB |
| Other |
50.89KiB |
50.89KiB |
| CSS |
31.88KiB |
31.88KiB |
Bundle analysis report Branch PupilTong:p/hw/ssr-hydrate-3.2 Project dashboard
Generated by RelativeCI Documentation Report issue
PupilTong
force-pushed
the
p/hw/ssr-hydrate-3.2
branch
2 times, most recently
from
f628bea to
6f4a98e
Compare
PupilTong
force-pushed
the
p/hw/ssr-hydrate-3.2
branch
from
63aa658 to
2dc52e7
Compare
PupilTong
force-pushed
the
p/hw/ssr-hydrate-3.2
branch
from
2dc52e7 to
3783ecd
Compare
WalkthroughThis change introduces server-side rendering (SSR) hydration support for the "all-on-ui" feature across several packages. It modifies type definitions, rendering logic, test configurations, and build scripts to enable SSR data handling, conditional style injection, hydration event replay, and SSR-specific test execution and routing. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant LynxView (Custom Element)
participant createLynxView
participant startUIThread
participant createRenderAllOnUI
participant prepareMainThreadAPIs
participant createMainThreadGlobalThis
Browser->>LynxView (Custom Element): Attach lynx-view element (with optional ssr attribute)
LynxView (Custom Element)->>createLynxView: Call with configs (including ssr if present)
createLynxView->>startUIThread: Pass configs and optional ssr
startUIThread->>createRenderAllOnUI: Pass shadowRoot and ssrDumpInfo
createRenderAllOnUI->>prepareMainThreadAPIs: Pass SSR hydrate info if present
prepareMainThreadAPIs->>createMainThreadGlobalThis: Initialize with SSR hydrate info
Note over prepareMainThreadAPIs,createMainThreadGlobalThis: If SSR, replay hydration events and hydrate page<br/>Else, perform normal rendering and flushing
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ERROR Cannot resolve version $@rspack/core in overrides. The direct dependencies don't have dependency "@rspack/core". 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
packages/web-platform/web-tests/tests/react.spec.ts (2)
65-65: Consider making the wait time configurableThe 300ms wait time for SSR is hardcoded. Consider making this configurable through an environment variable or constant to allow for different timing requirements across environments.
+const SSR_WAIT_TIME = parseInt(process.env['SSR_WAIT_TIME'] || '300', 10); + const goto = async ( page: Page, testname: string, testname2?: string, hasDir?: boolean, ) => { let url = isSSR ? `/ssr?casename=${testname}` : `/?casename=${testname}`; if (hasDir) { url += '&hasdir=true'; } if (testname2) { url += `&casename2=${testname2}`; } await page.goto(url, { waitUntil: 'load', }); await page.evaluate(() => document.fonts.ready); - if (isSSR) await wait(300); + if (isSSR) await wait(SSR_WAIT_TIME); };
2520-2520: Consider adding more specific flaky test documentationWhile the test is marked as fixme for being flaky in SSR, it would be helpful to add more specific details about what makes it flaky to aid in future debugging.
-test.fixme(isSSR, 'flaky'); +test.fixme(isSSR, 'flaky - timing issues with overlay rendering in SSR');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/web-platform/web-tests/tests/__snapshots__/server.vitest.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (23)
.changeset/cool-geckos-exist.md(1 hunks).changeset/yellow-nights-fetch.md(1 hunks).github/workflows/test.yml(1 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(1 hunks)packages/web-platform/web-constants/src/types/SSR.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(1 hunks)packages/web-platform/web-core-server/src/createLynxView.ts(5 hunks)packages/web-platform/web-core-server/src/dumpHTMLString.ts(2 hunks)packages/web-platform/web-core/src/apis/LynxView.ts(4 hunks)packages/web-platform/web-core/src/apis/createLynxView.ts(4 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(6 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(5 hunks)packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts(1 hunks)packages/web-platform/web-tests/playwright.config.ts(1 hunks)packages/web-platform/web-tests/rspack.config.js(2 hunks)packages/web-platform/web-tests/server.js(1 hunks)packages/web-platform/web-tests/shell-project/index.ts(3 hunks)packages/web-platform/web-tests/shell-project/lynx-view.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(14 hunks)packages/web-platform/web-tests/tests/server.vitest.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (8)
packages/web-platform/web-core-server/src/dumpHTMLString.ts (3)
packages/web-platform/web-core-server/src/utils/escapeHtml.ts (1)
escapeHtml(30-73)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts (1)
_attributes(13-13)packages/web-platform/web-constants/src/constants.ts (2)
lynxPartIdAttribute(23-23)lynxUniqueIdAttribute(5-5)
packages/web-platform/web-tests/shell-project/lynx-view.ts (1)
packages/web-platform/web-core/src/apis/LynxView.ts (1)
LynxView(81-511)
packages/web-platform/web-tests/shell-project/index.ts (2)
packages/web-platform/web-tests/server.js (1)
lynxView(16-33)packages/web-platform/web-tests/shell-project/lynx-view.ts (1)
lynxViewTests(12-33)
packages/web-platform/web-core/src/uiThread/startUIThread.ts (1)
packages/web-platform/web-constants/src/types/SSR.ts (1)
SSRDumpInfo(11-14)
packages/web-platform/web-core/src/apis/createLynxView.ts (1)
packages/web-platform/web-constants/src/types/SSR.ts (1)
SSRDumpInfo(11-14)
packages/web-platform/web-core/src/apis/LynxView.ts (2)
packages/web-platform/web-constants/src/types/SSR.ts (1)
SSRDumpInfo(11-14)packages/web-platform/web-constants/src/constants.ts (1)
inShadowRootStyles(45-53)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (2)
packages/web-platform/web-constants/src/types/SSR.ts (2)
SSRDehydrateHooks(24-26)SSRHydrateInfo(16-22)packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts (1)
uniqueId(17-17)
packages/web-platform/web-core-server/src/createLynxView.ts (3)
packages/web-platform/offscreen-document/src/webworker/OffscreenElement.ts (1)
_attributes(13-13)packages/web-platform/web-constants/src/constants.ts (2)
lynxPartIdAttribute(23-23)lynxUniqueIdAttribute(5-5)packages/web-platform/web-constants/src/types/SSR.ts (2)
SSREventReplayInfo(4-9)SSRDumpInfo(11-14)
🔇 Additional comments (40)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
71-71: LGTM! Clean addition for SSR support.The addition of
'__GetPageElement'to the main thread injection variables follows the existing pattern and aligns with the SSR hydration workflow implementation..changeset/cool-geckos-exist.md (1)
1-8: LGTM! Properly formatted changeset for SSR support.The changeset correctly documents patch-level updates for the three core packages affected by the SSR implementation. The version level and description are appropriate.
packages/web-platform/web-constants/src/types/index.ts (1)
21-21: LGTM! Essential export addition for SSR types.The export addition makes SSR-related types available to consumers and follows the existing pattern. This is a necessary foundation for the SSR implementation across other packages.
packages/web-platform/web-core-server/src/dumpHTMLString.ts (1)
32-36: Good optimization for attribute serialization.The change to only serialize attributes with non-empty values is a sensible optimization that reduces HTML size and improves performance.
.github/workflows/test.yml (1)
98-100: LGTM! Essential test configuration for SSR validation.The addition of the "SSR-ALL-ON-UI" test configuration is crucial for ensuring SSR functionality works correctly. The configuration follows the existing pattern and properly sets the
ENABLE_SSR=trueenvironment variable.Note: Based on the past review comment, this aligns well with the matrix improvements mentioned in PR #1032.
packages/web-platform/web-core/src/apis/createLynxView.ts (3)
7-7: LGTM: Clean integration of SSR type import.The import of
SSRDumpInfotype properly integrates SSR support into the view creation pipeline.
36-36: LGTM: Well-structured optional SSR property.The optional
ssrproperty in theLynxViewConfigsinterface follows TypeScript best practices and maintains backward compatibility.
67-67: LGTM: Proper parameter destructuring and forwarding.The destructuring of the
ssrparameter and its forwarding tostartUIThreadmaintains the existing function signature while cleanly integrating SSR support.Also applies to: 88-88
.changeset/yellow-nights-fetch.md (1)
1-8: LGTM: Proper changeset documentation.The changeset file correctly documents the patch-level changes to the three affected packages and provides an appropriate description of the SSR template marking feature.
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
345-345: LGTM: Improved type flexibility for SSR hydration.The updated signature
(encodeData?: string | null)properly handles cases where SSR encoded data might be null, aligning with theSSRDumpInfotype structure wheressrEncodeDatacan benull | undefined.packages/web-platform/web-tests/playwright.config.ts (1)
22-27: LGTM: Improved test targeting for SSR.The refined conditional logic properly prioritizes SSR tests by restricting to
react.{test,spec}.tsfiles when SSR is enabled, providing more focused test execution for SSR scenarios.packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts (1)
317-322: Good defensive programming for SSR compatibility.The explicit stringification ensures consistent style value handling during SSR hydration and prevents potential runtime errors from non-string values. This change aligns well with the broader SSR implementation.
packages/web-platform/web-tests/server.js (3)
28-29: LGTM: Enhanced CSS injection for testing.The additional CSS rule
.injected-style-rules{background:green}provides a visual indicator for testing SSR style injection functionality.
32-32: LGTM: Consistent with PR objectives.The
threadStrategy: 'all-on-ui'configuration aligns with the PR's focus on implementing SSR support for the all-on-ui component.
27-27: Verify SSR output path in build artifactsThe
hydrateUrlon line 27 ofpackages/web-platform/web-tests/server.jsnow points to/dist/ssr/${caseName}/index.web.json, but that directory isn’t in your source (it’s generated at build time). Please manually confirm:
- Your SSR build configuration (e.g., bundler output or build script) writes files under
dist/ssr/<caseName>/index.web.json.- The test snapshots in
packages/web-platform/web-tests/tests/__snapshots__/server.vitest.spec.ts.snapalign with this new path.packages/web-platform/web-tests/shell-project/lynx-view.ts (3)
5-5: Minor: Import reordering.The import reordering is a minor change that doesn't affect functionality.
12-16: LGTM: Enhanced function signature for SSR support.The updated function signature allows for optional lynxView parameter, enabling reuse of existing elements during SSR hydration scenarios while maintaining backward compatibility.
30-30: Good defensive check for DOM manipulation.The parentElement check prevents duplicate DOM appends, which is especially important in SSR scenarios where elements may already exist in the DOM.
packages/web-platform/web-core/src/uiThread/startUIThread.ts (3)
17-17: LGTM: Proper type import for SSR support.The SSRDumpInfo type import provides the necessary type definitions for SSR hydration data.
39-39: LGTM: Optional SSR parameter maintains API compatibility.The optional ssr parameter allows SSR data to be passed during UI thread initialization while maintaining backward compatibility.
84-84: LGTM: Conditional SSR parameter passing.The ssr parameter is correctly passed to createRenderAllOnUI only when using the 'all-on-ui' thread strategy, which aligns with the PR's focus on SSR support for this specific strategy.
packages/web-platform/web-tests/rspack.config.js (2)
61-75: LGTM: SSR HTML generation configuration.The new HtmlRspackPlugin for
ssr.htmlprovides the necessary HTML template for SSR testing, using the same configuration as the main index.html file.
177-202: LGTM: Well-structured SSR middleware.The new
/ssrmiddleware follows the same pattern as the existing/fp-onlymiddleware, providing consistent error handling and parameter validation. The implementation correctly:
- Validates the required
casenameparameter- Handles errors with appropriate HTTP status codes
- Uses the
genHtmlfunction to generate SSR contentpackages/web-platform/web-core/src/apis/LynxView.ts (1)
304-476: Well-implemented SSR data handling.The SSR implementation correctly:
- Removes the SSR attribute on reload to ensure fresh rendering
- Properly decodes and parses SSR data from the element attribute
- Passes SSR data through to the rendering pipeline
packages/web-platform/web-constants/src/types/SSR.ts (1)
1-27: Well-structured SSR type definitions.The SSR types provide a clean interface for:
- Event replay information with proper typing
- SSR dump and hydration data structures
- Dehydration hooks for event handling
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
165-179: Well-implemented SSR hydration flow.The conditional hydration logic correctly:
- Skips normal rendering when SSR data is present
- Safely replays events by checking element references with
deref()- Calls the SSR hydration method with the encoded data
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (4)
181-182: Document the significance of index 1 for pageElement.The code retrieves the page element from index 1 without explanation. Consider adding a comment or using a named constant to clarify why index 1 specifically holds the page element.
485-485: Good addition for SSR template marking.The
__MarkTemplateElement(page)call is appropriately placed after setting page attributes, ensuring the page is properly marked for SSR template handling.
688-688: Correct implementation of SSR event hook override.The conditional delegation to
ssrHooks?.__AddEventenables proper event replay during SSR hydration while maintaining backward compatibility.
183-183: Potential off-by-one error in uniqueIdInc initialization.When
lynxUniqueIdToElement.lengthis 0,uniqueIdIncwill be 0 instead of 1. This could cause issues if unique IDs are expected to start from 1.Consider using:
- let uniqueIdInc = lynxUniqueIdToElement.length || 1; + let uniqueIdInc = Math.max(lynxUniqueIdToElement.length, 1);Likely an incorrect or invalid review comment.
packages/web-platform/web-tests/tests/react.spec.ts (8)
9-9: LGTM: Clean environment variable handlingThe SSR flag is properly defined using a double negation pattern to ensure a boolean value from the environment variable.
54-54: LGTM: Proper URL routing for SSRThe conditional URL construction correctly routes to
/ssrwhen SSR is enabled while maintaining the same query parameters.
415-416: LGTM: Proper fixme marking with assigneeThe test is appropriately marked as fixme for SSR with a clear reason and assignee reference.
465-465: LGTM: Consistent test skipping with clear reasonsThe tests are appropriately skipped for SSR with clear explanations. The reasons are well-documented, making it easy to understand why certain tests don't apply to SSR.
Also applies to: 583-583, 693-693, 700-700, 728-728, 774-774
935-935: LGTM: Clear TODO for exposure API migrationThe fixme is well-documented with a clear action item for migrating the exposure API to work with SSR.
1441-1441: LGTM: Dev mode SSR implementation notedThe test is appropriately marked as fixme with a clear task to implement dev mode for SSR.
1894-1894: LGTM: Event handling limitation documentedThe test skip is properly documented, explaining that the event is ignored in SSR, which is expected behavior.
2248-2248: LGTM: Exposure feature limitation documentedThe test is appropriately marked as fixme with a clear explanation that SSR doesn't support exposure functionality.
packages/web-platform/web-core-server/src/createLynxView.ts (2)
4-4: LGTM! Necessary imports for SSR functionality.The added imports for
lynxPartIdAttribute,SSREventReplayInfo, andSSRDumpInfoare properly used in the implementation below.Also applies to: 8-9
86-87: Good prioritization of part ID for SSR stability.The change to prioritize
lynxPartIdAttributeoverlynxUniqueIdAttributefor thessrIDmakes sense in SSR context, as part IDs are likely more stable across server/client boundaries.
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
Outdated
Show resolved
Hide resolved
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
Show resolved
Hide resolved
PupilTong
force-pushed
the
p/hw/ssr-hydrate-3.2
branch
from
3783ecd to
de3c692
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/lemon-colts-hang.md (1)
8-14: Clarify the changeset description for accuracy.The changeset could be more precise:
- Browsers do have
atob()for base64 decoding - perhaps the issue is more about performance or parsing complexity?- "Also fix some ssr issues" is vague - consider specifying what issues were fixed
Consider updating the description to be more specific:
-feat: move SSR hydrate essential info to the ssr attribute - -We found that in browser there is no simple tool to decode a base64 string - -Therefore we move the data to `ssr` attribute - -Also fix some ssr issues +feat: move SSR hydrate essential info to the ssr attribute + +Moved SSR hydration data from base64-encoded comments to a URI-encoded JSON +attribute on the root element for simpler and more efficient client-side parsing. + +This change also addresses SSR hydration reliability issues by improving +the event replay mechanism and element reference handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/web-platform/web-tests/tests/__snapshots__/server.vitest.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (22)
.changeset/cool-geckos-exist.md(1 hunks).changeset/lemon-colts-hang.md(1 hunks).github/workflows/test.yml(1 hunks)packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts(1 hunks)packages/web-platform/web-constants/src/types/SSR.ts(1 hunks)packages/web-platform/web-constants/src/types/index.ts(1 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(1 hunks)packages/web-platform/web-core-server/src/createLynxView.ts(4 hunks)packages/web-platform/web-core/src/apis/LynxView.ts(4 hunks)packages/web-platform/web-core/src/apis/createLynxView.ts(4 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(3 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/pureElementPAPIs.ts(1 hunks)packages/web-platform/web-tests/playwright.config.ts(1 hunks)packages/web-platform/web-tests/rspack.config.js(2 hunks)packages/web-platform/web-tests/server.js(1 hunks)packages/web-platform/web-tests/shell-project/index.ts(3 hunks)packages/web-platform/web-tests/shell-project/lynx-view.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(14 hunks)packages/web-platform/web-tests/tests/server.vitest.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/web-platform/web-tests/tests/server.vitest.spec.ts
- packages/web-platform/web-constants/src/utils/generateTemplate.ts
- .changeset/cool-geckos-exist.md
- packages/web-platform/web-constants/src/types/index.ts
- .github/workflows/test.yml
- packages/web-platform/web-core/src/apis/createLynxView.ts
- packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
- packages/web-platform/web-mainthread-apis/src/pureElementPAPIs.ts
- packages/web-platform/web-tests/shell-project/index.ts
- packages/web-platform/web-tests/shell-project/lynx-view.ts
- packages/web-platform/web-tests/playwright.config.ts
- packages/web-platform/web-tests/server.js
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-tests/rspack.config.js
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
- packages/web-platform/web-core/src/apis/LynxView.ts
- packages/web-platform/web-constants/src/types/SSR.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-core-server/src/createLynxView.ts
- packages/web-platform/web-tests/tests/react.spec.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
packages/web-platform/web-constants/src/types/SSR.ts (2)
SSRDehydrateHooks(25-27)SSRHydrateInfo(16-23)
🔇 Additional comments (3)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (3)
27-28: SSR parameter additions look good!The optional SSR parameters maintain backward compatibility while enabling SSR hydration support. The types are properly imported and parameters are correctly forwarded to
createMainThreadGlobalThis.Also applies to: 52-52, 73-73, 122-123
170-178: SSRHydrateInfo includes required propertiesConfirmed that in
packages/web-platform/web-constants/src/types/SSR.ts:
export type SSRDumpInfo = { ssrEncodeData: string | null | undefined; events: SSREventReplayInfo[]; }export interface SSRHydrateInfo extends SSRDumpInfo { … }Since
SSRHydrateInfoinherits botheventsandssrEncodeData, the replay logic is safe—no changes needed.
165-179: Add safety checks and error handling to the SSR hydration logic.The hydration logic needs additional safety checks:
- Bounds checking for array access
- Null check for
__AddEvent- Error handling for event replay
Apply this diff to add the necessary safety checks:
if (!ssrHydrateInfo) { mtsGlobalThis.renderPage!(initData); mtsGlobalThis.__FlushElementTree(undefined, {}); } else { // replay the hydrate event for (const event of ssrHydrateInfo.events) { const uniqueId = event[0]; - const element = ssrHydrateInfo.lynxUniqueIdToElement[uniqueId] - ?.deref(); - if (element) { - mtsGlobalThis.__AddEvent(element, event[1], event[2], event[3]); + if (uniqueId >= 0 && uniqueId < ssrHydrateInfo.lynxUniqueIdToElement.length) { + const element = ssrHydrateInfo.lynxUniqueIdToElement[uniqueId]?.deref(); + if (element && mtsGlobalThis.__AddEvent) { + try { + mtsGlobalThis.__AddEvent(element, event[1], event[2], event[3]); + } catch (error) { + reportError('SSR hydration event replay failed', error); + } + } } } mtsGlobalThis.ssrHydrate?.(ssrHydrateInfo.ssrEncodeData); }⛔ Skipped due to learnings
Learnt from: PupilTong PR: lynx-family/lynx-stack#1292 File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151 Timestamp: 2025-07-15T10:00:56.101Z Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the `element.getAttribute(lynxUniqueIdAttribute)!` call in SSR event capture where the attribute is expected to always be present.
Implement SSR for Web #52
PupilTong
force-pushed
the
p/hw/ssr-hydrate-3.2
branch
from
de3c692 to
856f5cd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/web-platform/web-tests/tests/react.spec.ts (2)
415-416: Consider providing more context for the fixme comment.The comment "reactlynx jsready bug" could be more descriptive about the specific issue and expected resolution.
- // TODO: @Yradex - test.fixme(isSSR, 'reactlynx jsready bug'); + // TODO: @Yradex - JSReady event handling issue in SSR mode + test.fixme(isSSR, 'reactlynx jsready bug - investigate JSReady event timing in SSR');
2520-2520: Consider providing more specific information about the flakiness.The generic "flaky" message could be more descriptive about the specific issue in SSR mode.
- test.fixme(isSSR, 'flaky'); + test.fixme(isSSR, 'flaky - timing issues with overlay events in SSR mode');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/web-platform/web-tests/tests/__snapshots__/server.vitest.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
.changeset/cool-geckos-exist.md(1 hunks).github/workflows/test.yml(1 hunks)packages/web-platform/web-constants/src/types/SSR.ts(2 hunks)packages/web-platform/web-constants/src/utils/generateTemplate.ts(1 hunks)packages/web-platform/web-core/src/apis/LynxView.ts(4 hunks)packages/web-platform/web-core/src/apis/createLynxView.ts(4 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(3 hunks)packages/web-platform/web-core/src/uiThread/startUIThread.ts(3 hunks)packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts(4 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(4 hunks)packages/web-platform/web-tests/playwright.config.ts(1 hunks)packages/web-platform/web-tests/rspack.config.js(2 hunks)packages/web-platform/web-tests/server.js(1 hunks)packages/web-platform/web-tests/shell-project/index.ts(3 hunks)packages/web-platform/web-tests/shell-project/lynx-view.ts(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(14 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web-platform/web-core/src/uiThread/startUIThread.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/web-platform/web-constants/src/utils/generateTemplate.ts
- .changeset/cool-geckos-exist.md
- .github/workflows/test.yml
- packages/web-platform/web-tests/playwright.config.ts
- packages/web-platform/web-tests/server.js
- packages/web-platform/web-tests/shell-project/index.ts
- packages/web-platform/web-tests/shell-project/lynx-view.ts
- packages/web-platform/web-constants/src/types/SSR.ts
- packages/web-platform/web-core/src/apis/LynxView.ts
- packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
- packages/web-platform/web-tests/rspack.config.js
- packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
- packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
- packages/web-platform/web-core/src/apis/createLynxView.ts
🔇 Additional comments (9)
packages/web-platform/web-tests/tests/react.spec.ts (9)
9-9: LGTM: SSR detection constant is properly implemented.The environment variable detection follows the existing pattern used for
ENABLE_MULTI_THREADand correctly uses boolean coercion.
54-65: LGTM: URL construction and SSR-specific wait logic is well implemented.The conditional URL construction properly handles SSR routing by switching from
/?casename=...to/ssr?casename=.... The additional 300ms wait after font readiness in SSR mode is a reasonable approach to handle potential hydration delays.
465-465: LGTM: Appropriate test skipping for SSR mode.Custom template loaders are not relevant for SSR testing, so skipping this test is appropriate.
583-583: LGTM: Clear fixme message for missing SSR performance API.The message clearly indicates what needs to be implemented.
693-693: LGTM: Consistent approach to skipping error-related tests in SSR.Error handling and reporting tests are appropriately skipped for SSR mode with consistent messaging.
Also applies to: 700-700, 728-728, 774-774
935-935: LGTM: Clear technical debt annotation for exposure API migration.The message clearly indicates the required migration work from web-elements to runtime.
1441-1441: LGTM: Appropriate fixme for dev mode SSR support.Development mode functionality would need separate implementation for SSR.
1894-1894: LGTM: Clear explanation for event handling difference in SSR.The message clearly explains that certain events are ignored in SSR mode.
2248-2248: LGTM: Appropriate fixme for exposure API in SSR.Exposure tracking is not supported in SSR mode, which is expected behavior.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Patch Changes - Optimize `componentAtIndex` by a few hundreds microseconds: avoiding manipulate `__pendingListUpdates` unless SnapshotInstance tree is changed ([#1201](#1201)) - Support alog of component rendering on production for better error reporting. Enable it by using `REACT_ALOG=true rspeedy dev/build` or defining `__ALOG__` to `true` in `lynx.config.js`: ([#1164](#1164)) ```js export default defineConfig({ // ... source: { define: { __ALOG__: true, }, }, }); ``` - Make `preact/debug` work with `@lynx-js/react`. ([#1222](#1222)) - Introduce `@lynx-js/react/debug` which would include debugging warnings and error messages for common mistakes found. ([#1250](#1250)) Add the import to `@lynx-js/react/debug` at the first line of the entry: ```js import "@lynx-js/react/debug"; import { root } from "@lynx-js/react"; import { App } from "./App.jsx"; root.render(<App />); ``` - `<list-item/>` deferred now accepts an object with `unmountRecycled` property to control unmounting behavior when the item is recycled. ([#1302](#1302)) For example, you can use it like this: ```jsx <list-item defer={{ unmountRecycled: true }} item-key="1"> <WillBeUnmountIfRecycled /> </list-item> ``` Now the component will be unmounted when it is recycled, which can help with performance in certain scenarios. - Avoid some unexpected `__SetAttribute` in hydrate when `undefined` is passed as an attribute value to intrinsic elements, for example: ([#1318](#1318)) ```jsx <image async-mode={undefined} /> ``` ## @lynx-js/[email protected] ### Patch Changes - Bump Rsbuild v1.4.6 with Rspack v1.4.8. ([#1282](#1282)) ## [email protected] ### Patch Changes - Add `import '@lynx-js/react/debug'` for all templates. ([#1250](#1250)) ## @lynx-js/[email protected] ### Patch Changes - Fix "TypeError: cannot read property 'call' of undefined" error during HMR updates. ([#1304](#1304)) - Supports extractStr for large JSON ([#1230](#1230)) - Change `extractStr` to `false` when `performance.chunkSplit.strategy` is not `all-in-one`. ([#1251](#1251)) - Updated dependencies \[[`cb7feb6`](cb7feb6), [`ec7228f`](ec7228f)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Support `@lynx-js/react/debug`. ([#1250](#1250)) ## @lynx-js/[email protected] ### Patch Changes - Support alog of component rendering on production for better error reporting. Enable it by using `REACT_ALOG=true rspeedy dev/build` or defining `__ALOG__` to `true` in `lynx.config.js`: ([#1164](#1164)) ```js export default defineConfig({ // ... source: { define: { __ALOG__: true, }, }, }); ``` - Supports `console.alog` and use different `console` object in main thread and background thread. ([#1164](#1164)) ## @lynx-js/[email protected] ### Patch Changes - feat: move SSR hydrate essential info to the ssr attribute ([#1292](#1292)) We found that in browser there is no simple tool to decode a base64 string Therefore we move the data to `ssr` attribute Also fix some ssr issues - feat: support \_\_MarkTemplateElement, \_\_MarkPartElement and \_\_GetTemplateParts for all-on-ui ([#1275](#1275)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support SSR for all-on-ui ([#1029](#1029)) - feat: move SSR hydrate essential info to the ssr attribute ([#1292](#1292)) We found that in browser there is no simple tool to decode a base64 string Therefore we move the data to `ssr` attribute Also fix some ssr issues - feat: support \_\_MarkTemplateElement, \_\_MarkPartElement and \_\_GetTemplateParts for all-on-ui ([#1275](#1275)) - feat: mark template elements for SSR and update part ID handling ([#1286](#1286)) - Updated dependencies \[[`cebda59`](cebda59), [`1443e46`](1443e46), [`5062128`](5062128), [`f656b7f`](f656b7f)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support SSR for all-on-ui ([#1029](#1029)) - feat: move SSR hydrate essential info to the ssr attribute ([#1292](#1292)) We found that in browser there is no simple tool to decode a base64 string Therefore we move the data to `ssr` attribute Also fix some ssr issues - feat: dump the event info on ssr stage ([#1237](#1237)) - feat: mark template elements for SSR and update part ID handling ([#1286](#1286)) ## @lynx-js/[email protected] ### Patch Changes - fix: indicator dots' bg-color on safari 26 ([#1298](#1298)) <https://bugs.webkit.org/show_bug.cgi?id=296048> The animation name should be defined in the template - fix: list may only render only one column in ReactLynx. ([#1280](#1280)) This is because `span-count` may not be specified when `list-type` is specified, resulting in layout according to `span-count="1"`. Postponing the acquisition of `span-count` until layoutListItem can solve this problem. - Updated dependencies \[[`443f3d5`](443f3d5)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: indicator dots' bg-color on safari 26 ([#1298](#1298)) <https://bugs.webkit.org/show_bug.cgi?id=296048> The animation name should be defined in the template ## @lynx-js/[email protected] ### Patch Changes - feat: support SSR for all-on-ui ([#1029](#1029)) - feat: move SSR hydrate essential info to the ssr attribute ([#1292](#1292)) We found that in browser there is no simple tool to decode a base64 string Therefore we move the data to `ssr` attribute Also fix some ssr issues - feat: support \_\_MarkTemplateElement, \_\_MarkPartElement and \_\_GetTemplateParts for all-on-ui ([#1275](#1275)) - feat: mark template elements for SSR and update part ID handling ([#1286](#1286)) - Updated dependencies \[[`1443e46`](1443e46), [`5062128`](5062128)]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`cebda59`](cebda59), [`1443e46`](1443e46), [`5062128`](5062128), [`f656b7f`](f656b7f)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix the `Syntax Error: expecting ';'` error of chunk splitting ([#1279](#1279)) ## [email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
3 participants
#52
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores