fix(web): resolve mts freeze issue after reload by ensuring iframe is…#1892
fix(web): resolve mts freeze issue after reload by ensuring iframe is…#1892PupilTong merged 2 commits intolynx-family:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 8e7dc20 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughImplements an async iframe-ready handshake for the UI-thread JS realm, makes iframe realm creation return a Promise, updates main-thread API boot to accept/await a promised realm, removes deferred update queuing, unskips reload-related tests across browsers, and adds a changeset entry documenting the MTS reload fix. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.changeset/loose-crews-slide.md (1)
6-10: Polish wording and capitalization in the ChangesetImprove clarity and fix grammar/casing (“freezed” → “frozen”, “Javascript” → “JavaScript”), and optionally note the async realm init.
Apply:
-fix: mts freeze after reload() +fix(web): prevent MTS freeze after reload() -The mts may be freezed after reload() called. +MTS could freeze after calling reload(). -We fixed it by waiting until the all-on-ui Javascript realm implementation, an iframe, to be fully loaded. +We fixed this by waiting until the all‑on‑UI JavaScript realm implementation (an iframe) is fully loaded before proceeding. +Additionally, @lynx-js/web-mainthread-apis now accepts a Promise<JSRealm> to support async initialization.Also consider capitalizing “MTS” consistently throughout.
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
46-46: Normalize mtsRealmPromise once to avoid repeated awaitsMinor optimization/readability: coerce to a single Promise and reuse, rather than awaiting the union at each site.
Inside prepareMainThreadAPIs (near the top):
const resolvedMtsRealm = Promise.resolve(mtsRealmPromise);Then:
-const mtsRealm = await mtsRealmPromise; +const mtsRealm = await resolvedMtsRealm;Repeat the same reuse in other call sites (see Line 262 below).
Also applies to: 100-100
packages/web-platform/web-tests/tests/react.spec.ts (3)
79-92: Make assertions deterministic; avoid string contains and fixed waitsUse Playwright’s auto-waiting CSS assertions instead of style substring checks and arbitrary sleeps.
- await wait(100); - const target = page.locator('#target'); - await target.click(); - await wait(100); - await expect(await target.getAttribute('style')).toContain('green'); + const target = page.locator('#target'); + await target.click(); + await expect(target).toHaveCSS('background-color', 'rgb(0, 128, 0)'); // green await page.evaluate(() => { // @ts-expect-error globalThis.lynxView.reload(); }); - await wait(100); - await expect(await target.getAttribute('style')).toContain('pink'); + await expect(target).toHaveCSS('background-color', 'rgb(255, 192, 203)'); // pink
93-109: Stabilize page-count assertion after reloadRely on a condition wait instead of fixed sleeps to avoid flakes across browsers.
- await wait(100); - expect( - await page.evaluate(() => - Array.from( - document.querySelector('lynx-view')?.shadowRoot?.children || [], - ) - .filter(i => i.getAttribute('lynx-tag') === 'page').length - ), - ).toBe(1); + await page.waitForFunction(() => { + const children = Array.from( + document.querySelector('lynx-view')?.shadowRoot?.children || [], + ); + return children.filter(i => i.getAttribute('lynx-tag') === 'page').length === 1; + });
176-195: Prefer auto-waiting CSS checks over fixed delays in globalProps reload testReduce flakiness by asserting computed CSS instead of sleeping.
- await wait(500); + await expect(page.locator('#target')).toHaveCSS('background-color', 'rgb(0, 128, 0)'); // green await page.evaluate(() => { (document.querySelector('lynx-view') as LynxView)?.reload(); }); - await wait(500); - expect(await page.locator('#target').getAttribute('style')).toContain('green'); + await expect(page.locator('#target')).toHaveCSS('background-color', 'rgb(0, 128, 0)'); // greenpackages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (1)
196-199: Confirm removal of update deferral is acceptableupdateDataMainThread now directly calls handleUpdatedData with no buffering. If updates arrive before MTS is ready (or after reload, pre-start), they won’t be replayed on the UI. Verify this aligns with product semantics.
If not intended, add a minimal buffer of the latest update until the realm exposes updatePage, then flush and clear.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/loose-crews-slide.md(1 hunks)packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(2 hunks)packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts(3 hunks)packages/web-platform/web-tests/tests/react.spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/loose-crews-slide.md
🧠 Learnings (2)
📚 Learning: 2025-10-10T08:22:12.033Z
Learnt from: Sherry-hue
PR: lynx-family/lynx-stack#1837
File: packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts:266-266
Timestamp: 2025-10-10T08:22:12.033Z
Learning: In packages/web-platform/web-mainthread-apis, the handleUpdatedData function returned from prepareMainThreadAPIs is internal-only, used to serve web-core. It does not require public documentation, type exports, or SSR support.
Applied to files:
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
📚 Learning: 2025-10-11T06:16:12.507Z
Learnt from: Sherry-hue
PR: lynx-family/lynx-stack#1820
File: packages/web-platform/web-tests/tests/react.spec.ts:834-856
Timestamp: 2025-10-11T06:16:12.507Z
Learning: In packages/web-platform/web-tests/tests/react.spec.ts, the tests `basic-bindmouse` and `basic-mts-bindtouchstart` are NOT duplicates despite having similar test structures. They test different event types: `basic-bindmouse` validates mouse events (mousedown, mouseup, mousemove) with mouse-specific properties (button, buttons, x, y, pageX, pageY, clientX, clientY), while `basic-mts-bindtouchstart` validates touch events (touchstart) with touch arrays (touches, targetTouches, changedTouches). The similar test structure is coincidental and follows testing conventions.
Applied to files:
packages/web-platform/web-tests/tests/react.spec.ts
🧬 Code graph analysis (3)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (3)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
JSRealm(1-5)packages/web-platform/web-constants/src/types/Cloneable.ts (1)
Cloneable(4-8)packages/web-platform/web-constants/src/types/UpdateDataOptions.ts (1)
UpdateDataOptions(6-9)
packages/web-platform/web-tests/tests/react.spec.ts (1)
packages/web-platform/web-tests/tests/coverage-fixture.ts (1)
test(11-60)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (1)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
JSRealm(1-5)
🪛 LanguageTool
.changeset/loose-crews-slide.md
[grammar] ~10-~10: There might be a mistake here.
Context: ...entation, an iframe, to be fully loaded.
(QB_NEW_EN)
🔇 Additional comments (1)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
258-271: No buffering needed: updatePage runs after runtime is ready
handleUpdatedData awaits mtsRealmPromise before calling runtime.updatePage, so updates cannot execute prematurely.Likely an incorrect or invalid review comment.
React Example#5924 Bundle Size — 237.5KiB (0%).8e7dc20(current) vs e9e7c09 main#5910(baseline) Bundle metrics
|
| Current #5924 |
Baseline #5910 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
165 |
165 |
| Duplicate Modules |
67 |
67 |
| Duplicate Code |
46.77% |
46.77% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5924 |
Baseline #5910 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.74KiB |
91.74KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-iframe-reload Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5920 Bundle Size — 364.68KiB (+0.03%).8e7dc20(current) vs e9e7c09 main#5906(baseline) Bundle metrics
|
| Current #5920 |
Baseline #5906 |
|
|---|---|---|
 Initial JS |
144.63KiB(+0.06%) |
144.54KiB |
| Initial CSS |
32.11KiB |
32.11KiB |
 Cache Invalidation |
46.6% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
| Modules |
220(+1.38%) |
217 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.23% |
3.23% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type 1 change 1 regression
| Current #5920 |
Baseline #5906 |
|
|---|---|---|
| JS |
238.77KiB (+0.05%) |
238.65KiB |
| Other |
93.8KiB |
93.8KiB |
| CSS |
32.11KiB |
32.11KiB |
Bundle analysis report Branch PupilTong:p/hw/fix-iframe-reload Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1892 will degrade performances by 7.59%Comparing Summary
Benchmarks breakdown
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (1)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
JSRealm(1-5)
🔇 Additional comments (2)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (2)
129-129: LGTM! Correct async realm initialization.Passing the
Promise<JSRealm>directly toprepareMainThreadAPIsis correct. Based on the AI summary, the main thread APIs have been updated to accept and await a promised realm, which aligns with the async initialization approach to ensure the iframe is fully loaded before use.
194-202: LGTM! Removal of deferred queuing aligns with the fix.The direct forwarding of updates to
handleUpdatedDatawithout the previouspendingUpdateCallsqueue is correct. This change, combined with the async realm initialization, ensures updates are processed only after the iframe is fully ready, which addresses the root cause of the MTS freeze issue.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/react@0.114.3 ### Patch Changes - Initialize `ctxNotFoundEventListener` before each test in testing library ([#1888](#1888)) - fix: main thread functions created during the initial render cannot correctly modify `MainThreadRef`s after hydration ([#1884](#1884)) ## @lynx-js/rspeedy@0.11.7 ### Patch Changes - Bump Rsbuild v1.5.17. ([#1889](#1889)) - feat: support web preview in rspeedy dev ([#1893](#1893)) - support web preview in rspeedy dev (experimental) - Updated dependencies \[]: - @lynx-js/web-rsbuild-server-middleware@0.18.1 ## @lynx-js/web-constants@0.18.1 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-worker-rpc@0.18.1 ## @lynx-js/web-core@0.18.1 ### Patch Changes - fix: mts freeze after reload() ([#1892](#1892)) The mts may be freezed after reload() called. We fixed it by waiting until the all-on-ui Javascript realm implementation, an iframe, to be fully loaded. - Updated dependencies \[[`70a18fc`](70a18fc)]: - @lynx-js/web-mainthread-apis@0.18.1 - @lynx-js/web-worker-runtime@0.18.1 - @lynx-js/web-constants@0.18.1 - @lynx-js/web-worker-rpc@0.18.1 ## @lynx-js/web-elements@0.8.9 ### Patch Changes - fix: layoutchange event result `detail.top` and `detail.left` was `0` incorrectly ([#1887](#1887)) - textarea placeholder inherits font-size by default ([#1874](#1874)) - feat: support <x-text text="content"></x-text> ([#1881](#1881)) - Updated dependencies \[]: - @lynx-js/web-elements-template@0.8.9 ## @lynx-js/web-mainthread-apis@0.18.1 ### Patch Changes - fix: mts freeze after reload() ([#1892](#1892)) The mts may be freezed after reload() called. We fixed it by waiting until the all-on-ui Javascript realm implementation, an iframe, to be fully loaded. - Updated dependencies \[]: - @lynx-js/web-constants@0.18.1 - @lynx-js/web-style-transformer@0.18.1 ## @lynx-js/web-worker-runtime@0.18.1 ### Patch Changes - Updated dependencies \[[`70a18fc`](70a18fc)]: - @lynx-js/web-mainthread-apis@0.18.1 - @lynx-js/web-constants@0.18.1 - @lynx-js/web-worker-rpc@0.18.1 ## create-rspeedy@0.11.7 ## upgrade-rspeedy@0.11.7 ## @lynx-js/web-core-server@0.18.1 ## @lynx-js/web-elements-template@0.8.9 ## @lynx-js/web-rsbuild-server-middleware@0.18.1 ## @lynx-js/web-style-transformer@0.18.1 ## @lynx-js/web-worker-rpc@0.18.1 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
… fully loaded
Summary by CodeRabbit
Checklist