Skip to content

Conversation

@PupilTong
Copy link
Collaborator

@PupilTong PupilTong commented Aug 23, 2025

Summary by CodeRabbit

  • New Features

    • Main-thread now runs in an isolated JS realm (iframe/VM) providing a real globalThis.
    • Automatic template versioning with on-load upgrade path.
    • Standard timer APIs exposed on the main-thread: setTimeout/clearTimeout/setInterval/clearInterval.
  • Refactor

    • Replaced proxy-based global handling with a realm-backed globalWindow and simplified module/script loading.
  • Bug Fixes

    • Removed reliance on legacy global variable lists and tightened global scoping.
  • Tests

    • Added/updated tests validating globalThis and cross-thread behavior.
  • Chores

    • Minor package version bumps.

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).
  • Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).

@changeset-bot
Copy link

changeset-bot bot commented Aug 23, 2025

🦋 Changeset detected

Latest commit: 8ce6e87

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@lynx-js/web-mainthread-apis Minor
@lynx-js/web-worker-runtime Minor
@lynx-js/web-core-server Minor
@lynx-js/web-worker-rpc Minor
@lynx-js/web-constants Minor
@lynx-js/web-core Minor
@lynx-js/web-style-transformer Minor

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 23, 2025

📝 Walkthrough

Walkthrough

Replace proxy/fake globalThis with isolated real JS realms (iframe on UI, VM on server/worker); add JSRealm type and template versioning/upgraders; remove proxy/global-binding symbols and disallowed/global lists; expose timer APIs; update prepare/start wiring, template generation, fixtures, and tests to use realm-based flows.

Changes

Cohort / File(s) Summary
Types & Constants
packages/web-platform/web-constants/src/constants.ts, packages/web-platform/web-constants/src/types/JSRealm.ts, packages/web-platform/web-constants/src/types/LynxModule.ts, packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts, packages/web-platform/web-constants/src/types/MainThreadLynx.ts, packages/web-platform/web-constants/src/types/index.ts
Remove exported globalMuteableVars and globalDisallowedVars. Add JSRealm type and re-export it. Add optional version?: number to LynxTemplate. Remove _updateVars and __lynxGlobalBindingValues from MainThreadGlobalThis. Add timer API types to MainThreadLynx.
Template Generation
packages/web-platform/web-constants/src/utils/generateTemplate.ts
Add template versioning (currentSupportedTemplateVersion=2) and an upgrader v1→v2; replace previous injection/wrapper logic with versioned module wrappers, local disallowed-global handling, and simplified module/content URL generation; remove reliance on removed global lists.
Main-thread API Refactor
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts, packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts, packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
Replace proxy-based globalThis with realm-based mtsRealm.globalWindow; remove createElement callback; swap config fields to include lynxTemplate, document, mtsRealm; prepareMainThreadAPIs now accepts document and mtsRealm and returns Promise<void>; load scripts via realm loaders; expose timer functions on MainThreadLynx.
UI & Server Realm Creation
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts, packages/web-platform/web-core-server/src/createLynxView.ts
Create isolated JS realms: hidden iframe in UI (createIFrameRealm) and Node VM-backed context on server (mtsVMContext), each providing globalWindow, loadScript, loadScriptSync. Wire realms into prepare/start flows and SSR extraction.
Worker / Runtime Startup
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts, packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
Remove Proxy-based sandbox and import/use of globalDisallowedVars; return bundle entry directly. Introduce/consume JSRealm, attach document, create/consume mtsRealm, pass realm to prepareMainThreadAPIs, and adjust event/commit wiring and update handler typing.
Tests, Fixtures & Examples
packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json, .../web-core.main-thread.json, packages/web-platform/web-tests/shell-project/mainthread-test.ts, packages/web-platform/web-tests/tests/web-core.test.ts, packages/web-platform/web-tests/tests/react.spec.ts, packages/web-platform/web-tests/tests/react/api-global-disallowed-vars/index.jsx, packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx, packages/web-platform/web-tests/tests/react/api-updateGlobalProps/index.jsx, packages/web-platform/web-tests/tests/react/basic-globalProps/index.jsx
Update fixtures to use globalThis runtime and safe self alias; change imports to lynxTemplate; pass document/mtsRealm and lynxTemplate in runtime initialization; add api-globalThis test and update tests to new runtime/global paths and global props access.
Changeset / Version Bumps
.changeset/odd-cooks-play.md
Bump package minors and note switch to real globalThis via iframe/VM and removal of prior function wrapper/fake global.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Possibly related PRs

Suggested reviewers

  • Yradex
  • hzy
  • Sherry-hue
  • colinaaa

Poem

"I'm a rabbit in the code, I hop and play 🐇
I dug an iframe burrow where globals stay.
VM hums on the server, windows snug and bright,
Timers tick, templates lift — I bound from byte to byte.
Patch sealed, I twitch my whiskers and take flight."

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Aug 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@relativeci
Copy link

relativeci bot commented Aug 23, 2025

React Example

#4673 Bundle Size — 237.06KiB (0%).

8ce6e87(current) vs bc310a8 main#4649(baseline)

Bundle metrics  no changes
                 Current
#4673
     Baseline
#4649
No change  Initial JS 0B 0B
No change  Initial CSS 0B 0B
No change  Cache Invalidation 0% 0%
No change  Chunks 0 0
No change  Assets 4 4
No change  Modules 158 158
No change  Duplicate Modules 64 64
No change  Duplicate Code 45.83% 45.83%
No change  Packages 2 2
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#4673
     Baseline
#4649
No change  IMG 145.76KiB 145.76KiB
No change  Other 91.3KiB 91.3KiB

Bundle analysis reportBranch PupilTong:p/hw/real-global-thisProject dashboard


Generated by RelativeCIDocumentationReport issue

@relativeci
Copy link

relativeci bot commented Aug 23, 2025

Web Explorer

#4666 Bundle Size — 367.24KiB (-0.27%).

8ce6e87(current) vs bc310a8 main#4642(baseline)

Bundle metrics  Change 4 changes Improvement 2 improvements
                 Current
#4666
     Baseline
#4642
Improvement  Initial JS 144.21KiB(-0.07%) 144.31KiB
No change  Initial CSS 31.84KiB 31.84KiB
Change  Cache Invalidation 63.32% 0%
No change  Chunks 8 8
No change  Assets 8 8
No change  Modules 220 220
Improvement  Duplicate Modules 16(-5.88%) 17
Change  Duplicate Code 3.32%(-13.77%) 3.85%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  Change 1 change Improvement 1 improvement
                 Current
#4666
     Baseline
#4642
Improvement  JS 235.24KiB (-0.42%) 236.23KiB
No change  Other 100.16KiB 100.16KiB
No change  CSS 31.84KiB 31.84KiB

Bundle analysis reportBranch PupilTong:p/hw/real-global-thisProject dashboard


Generated by RelativeCIDocumentationReport issue

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 23, 2025

CodSpeed Performance Report

Merging #1589 will degrade performances by 51.34%

Comparing PupilTong:p/hw/real-global-this (8ce6e87) with main (bc310a8)1

Summary

❌ 8 regressions
✅ 11 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
002-hello-reactLynx__main-thread-renderToString 1.7 ms 3.5 ms -51.34%
basic-performance-div-100 4.2 ms 5.7 ms -26.88%
basic-performance-image-100 7.9 ms 9.7 ms -18.1%
basic-performance-large-css 8.4 ms 10.4 ms -19.25%
basic-performance-nest-level-100 4.9 ms 6 ms -18.46%
basic-performance-scroll-view-100 7.3 ms 9.1 ms -20.25%
basic-performance-small-css 4.9 ms 6.9 ms -28.41%
basic-performance-text-200 13.1 ms 15.4 ms -15.48%

Footnotes

  1. No successful run was found on main (60c81a1) during the generation of this report, so bc310a8 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@PupilTong PupilTong force-pushed the p/hw/real-global-this branch 5 times, most recently from 7a572f2 to 24afad6 Compare August 25, 2025 08:54
@PupilTong PupilTong self-assigned this Aug 26, 2025
@PupilTong PupilTong force-pushed the p/hw/real-global-this branch 4 times, most recently from cbcd5dd to 54983df Compare August 27, 2025 08:02
@PupilTong
Copy link
Collaborator Author

CodSpeed Performance Report

Merging #1589 will degrade performances by 28.21%

Comparing PupilTong:p/hw/real-global-this (cbcd5dd) with main (518013a)

Summary

❌ 7 regressions ✅ 3 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
basic-performance-div-100 4.5 ms 5.9 ms -23.46%
basic-performance-image-100 7.9 ms 9.7 ms -18.47%
basic-performance-large-css 8.4 ms 10.1 ms -17.18%
basic-performance-nest-level-100 4.9 ms 6.5 ms -25.11%
basic-performance-scroll-view-100 7.3 ms 9.1 ms -19.6%
basic-performance-small-css 5 ms 6.9 ms -28.21%
basic-performance-text-200 13.1 ms 15.7 ms -16.84%

constantly +2ms igonre it.

@PupilTong PupilTong moved this to In Progress in Lynx for Web Aug 27, 2025
@PupilTong PupilTong force-pushed the p/hw/real-global-this branch from 54983df to db83fa8 Compare August 27, 2025 09:54
@PupilTong PupilTong changed the title wip: provide mts a real globalthis feat: provide mts a real globalthis Aug 27, 2025
@PupilTong PupilTong force-pushed the p/hw/real-global-this branch from db83fa8 to b679c9f Compare August 27, 2025 11:02
@PupilTong PupilTong marked this pull request as ready for review August 27, 2025 11:13
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
packages/web-platform/web-tests/tests/react.spec.ts (1)

1148-1164: Fix flakiness causing the pipeline failure: wait for the exact console event instead of sleeping.

The 100 ms waits are racy; console logs can arrive later, yielding an empty messages array (as seen in CI). Wait for the 'pass:dataset2' console event deterministically.

Apply this diff:

-      const message: string[] = [];
-      await page.on('console', (msg) => {
-        const text = msg.text();
-        if (text.startsWith('pass')) {
-          message.push(msg.text());
-        }
-      });
+      const message: string[] = [];
+      page.on('console', (msg) => {
+        const text = msg.text();
+        if (text.startsWith('pass')) message.push(text);
+      });
@@
-      await wait(100);
-      await page.locator('#button').click();
-      await wait(100);
-      expect(message).toContain('pass:dataset2');
+      await wait(100);
+      await page.locator('#button').click();
+      await page.waitForEvent('console', (m) => m.text() === 'pass:dataset2');
+      expect(message).toContain('pass:dataset2');
packages/web-platform/web-core-server/src/createLynxView.ts (1)

221-223: Bug: attribute encoding should use encodeURIComponent.

encodeURI does not escape quotes and can break the HTML attribute. Use encodeURIComponent:

-      '" ssr ="',
-      encodeURI(JSON.stringify(ssrDumpInfo)),
+      '" ssr ="',
+      encodeURIComponent(JSON.stringify(ssrDumpInfo)),
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (1)

182-187: Dropped updates before init: queue when updatePage is not ready, not when mtsGlobalThis is falsy

mtsGlobalThis is always truthy now (it’s the iframe window), so the else branch is dead and updates get silently dropped if updatePage isn’t set yet.

-  // Process any pending update calls that were queued while mtsGlobalThis was undefined
+  // Process any pending update calls that were queued while updatePage wasn't ready
   for (const args of pendingUpdateCalls) {
     mtsGlobalThis.updatePage?.(...args);
   }
   pendingUpdateCalls.length = 0;
-  if (mtsGlobalThis) {
-    mtsGlobalThis.updatePage?.(...args);
-  } else {
-    // Cache the call if mtsGlobalThis is not yet initialized
-    pendingUpdateCalls.push(args);
-  }
+  if (typeof mtsGlobalThis.updatePage === 'function') {
+    mtsGlobalThis.updatePage(...args);
+  } else {
+    // Cache the call if updatePage is not yet initialized
+    pendingUpdateCalls.push(args);
+  }

Also applies to: 188-197

packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)

89-95: Fix double-callback bug in loadScriptAsync promise chain.

catch(callback).then(...) invokes the callback on error and then again in then, causing duplicate invocations. Also normalize the error to the declared string type.

Apply this diff:

-      import(
-        /* webpackIgnore: true */
-        sourceURL
-      ).catch(callback).then(async () => {
-        callback(null, createBundleInitReturnObj());
-      });
+      import(
+        /* webpackIgnore: true */
+        sourceURL
+      )
+        .then(() => {
+          callback(null, createBundleInitReturnObj());
+        })
+        .catch((e) => {
+          const msg = e instanceof Error ? e.message : String(e);
+          callback(msg);
+        });
🧹 Nitpick comments (25)
packages/web-platform/web-tests/tests/react.spec.ts (2)

1165-1179: Make assertions deterministic by awaiting both expected console logs.

Remove fixed sleeps; await the two specific console messages to avoid timing issues.

-      const message: string[] = [];
-      page.on('console', (msg) => {
-        const text = msg.text();
-        if (text.startsWith('pass')) {
-          message.push(msg.text());
-        }
-      });
+      const message: string[] = [];
+      page.on('console', (msg) => {
+        const text = msg.text();
+        if (text.startsWith('pass')) message.push(text);
+      });
@@
-      await goto(page, title);
-      await page.locator('#button').click();
-      await wait(100);
-      expect(message).toContain('pass:dataset1');
-      expect(message).toContain('pass:dataset2');
+      await goto(page, title);
+      await page.locator('#button').click();
+      await Promise.all([
+        page.waitForEvent('console', (m) => m.text() === 'pass:dataset1'),
+        page.waitForEvent('console', (m) => m.text() === 'pass:dataset2'),
+      ]);
+      expect(message).toContain('pass:dataset1');
+      expect(message).toContain('pass:dataset2');

1287-1305: Harden api-globalThis: await console events instead of sleeping.

Sleep-based checks can flake across realms; wait for the exact logs.

-    test(
-      'api-globalThis',
-      async ({ page }, { title }) => {
-        let mts = false;
-        let bts = false;
-        page.on('console', (message) => {
-          if (message.text() === 'mtsFoo 123') {
-            mts = true;
-          }
-          if (message.text() === 'btsFoo 123') {
-            bts = true;
-          }
-        });
-        await goto(page, title);
-        await wait(200);
-        !isSSR && expect(mts).toBe(true);
-        expect(bts).toBe(true);
-      },
-    );
+    test(
+      'api-globalThis',
+      async ({ page }, { title }) => {
+        await goto(page, title);
+        const mtsPromise = isSSR
+          ? Promise.resolve()
+          : page.waitForEvent('console', (m) => m.text() === 'mtsFoo 123');
+        const btsPromise = page.waitForEvent('console', (m) => m.text() === 'btsFoo 123');
+        await Promise.all([mtsPromise, btsPromise]);
+      },
+    );
packages/web-platform/web-tests/tests/react/api-updateGlobalProps/index.jsx (1)

12-12: Prefer runtime-scoped access for consistency.

Elsewhere tests use globalThis.runtime.lynx. Consider aligning to avoid dual globals.

-        backgroundColor: lynx.__globalProps.backgroundColor,
+        backgroundColor: globalThis.runtime.lynx.__globalProps.backgroundColor,
packages/web-platform/web-constants/src/types/LynxModule.ts (1)

40-41: Narrow the template version type and document semantics.

Constrain allowed versions and add a brief JSDoc to prevent accidental bumps and ease upgrades.

 export interface LynxTemplate {
@@
   elementTemplate: Record<string, ElementTemplateData[]>;
-  version?: number;
+  /**
+   * Template version. Defaults to 1 when omitted.
+   * Supported values are incrementally upgraded by the runtime.
+   */
+  version?: TemplateVersion;
 }
 
 export interface LynxJSModule {
   exports?: (lynx_runtime: any) => unknown;
 }
+
+// Keep in sync with generateTemplate/currentSupportedTemplateVersion.
+export type TemplateVersion = 1 | 2;
packages/web-platform/web-tests/tests/react/basic-globalProps/index.jsx (1)

12-12: Use the runtime-scoped lynx for uniformity with other tests.

Minor consistency tweak only.

-        backgroundColor: lynx.__globalProps.backgroundColor,
+        backgroundColor: globalThis.runtime.lynx.__globalProps.backgroundColor,
packages/web-platform/web-tests/tests/react/api-global-disallowed-vars/index.jsx (1)

8-13: Ensure globals are recognized by linting in both window and worker contexts.

Using bare navigator/postMessage can trip no-undef depending on config.

+/* eslint-env browser, worker */
 // Copyright 2023 The Lynx Authors. All rights reserved.
 // Licensed under the Apache License Version 2.0 that can be found in the
 // LICENSE file in the root directory of this source tree.
 import { root } from '@lynx-js/react';
 function App() {
   if (__MAIN_THREAD__) {
     console.log(
       `main thread: ${navigator}, ${postMessage}`,
     );
   } else {
     console.log(
       `background thread: ${navigator}, ${postMessage}`,
     );
   }
packages/web-platform/web-constants/src/utils/generateTemplate.ts (5)

71-82: Safer wrapper termination and minor emission tweak

Terminate the IIFE with a semicolon to avoid ASI hazards when concatenated, keeping the rest unchanged.

Apply:

-    }=void 0;module.exports = 
+    }=void 0;module.exports = 
     content,
-    '\n})()',
+    '\n})();',

95-97: Avoid undefined- prefixes when templateName is not provided

Default the prefix to a stable value to improve cacheability and readability.

Apply:

-      `${templateName}-${name.replaceAll('/', '')}.js`,
+      `${templateName ?? 'lynx-template'}-${name.replaceAll('/', '')}.js`,

114-119: More actionable error: include versions in the message

Helps users immediately understand the mismatch.

Apply:

-    throw new Error(
-      `Unsupported template, please upgrade your web-platform dependencies`,
-    );
+    throw new Error(
+      `Unsupported template version ${template.version}; supported <= ${currentSupportedTemplateVersion}. Please upgrade your web-platform dependencies.`,
+    );

12-51: Perf heads-up: large per-module destructure in upgrader

Destructuring ~30+ globals for every manifest entry adds parse/bind overhead and likely contributes to the reported CodSpeed regressions. Consider trimming the list to only what your v1 manifests actually use, or generate the list per template via static analysis.

I can prototype a small analyzer that scans manifest strings for free identifiers and emits a minimal destructure list if helpful.


7-9: Naming/style nit: type aliases are typically PascalCase

templateUpgrader -> TemplateUpgrader. Keeps consistency with the rest of the codebase.

Apply:

-type templateUpgrader = (template: LynxTemplate) => LynxTemplate;
-const templateUpgraders: templateUpgrader[] = [
+type TemplateUpgrader = (template: LynxTemplate) => LynxTemplate;
+const templateUpgraders: TemplateUpgrader[] = [
.changeset/odd-cooks-play.md (1)

10-18: Polish changeset wording; fix typos and clarify intent.

Apply this diff to improve clarity and correctness:

-refactor: provide the mts a real globalThis
-
-Before this change, We create a function wrapper and a fake globalThis for Javascript code.
-
-This caused some issues.
-
-After this change, we will create an iframe for createing an isolated Javascript context.
-
-This means the globalThis will be the real one.
+refactor: use a real globalThis for Main Thread Scripts (MTS)
+
+Before: we wrapped user code and proxied a fake globalThis for JavaScript.
+This led to observable differences from a real window context.
+
+After: we create an iframe to host an isolated JavaScript realm.
+Within this realm, globalThis is the real one (the iframe’s window).
packages/web-platform/web-constants/src/types/JSRealm.ts (1)

1-5: Add brief docs to define realm guarantees.

Consider documenting expectations for implementers (browser iframe vs Node VM), especially that:

  • code executed via loadScript/Sync runs in the realm so that this === globalWindow,
  • globalWindow should expose timers/DOM as appropriate for the environment.

Example:

+/**
+ * Abstraction over an isolated JS realm hosting MTS code.
+ * Implementations should ensure scripts execute with `this === globalWindow`.
+ */
 export type JSRealm = {
   globalWindow: typeof globalThis;
   loadScript: (url: string) => Promise<unknown>;
   loadScriptSync: (url: string) => unknown;
 };
packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx (2)

11-14: Stabilize effect to run once.

Prevent multiple invocations across re-renders:

-  useEffect(() => {
-    runOnMainThread(mtsFoo)();
-    console.log('btsFoo', foo);
-  });
+  useEffect(() => {
+    runOnMainThread(mtsFoo)();
+    console.log('btsFoo', globalThis.foo);
+  }, []);

9-13: Be explicit with global access across realms.

To avoid accidental shadowing and clarify intent across realms:

-    console.log('mtsFoo', foo);
+    console.log('mtsFoo', globalThis.foo);

If MTS globalThis is fully isolated, ensure the intended bridging for foo (e.g., via globalProps) is in place; otherwise this test may rely on undefined behavior.

packages/web-platform/web-core-server/src/createLynxView.ts (2)

131-134: Nit: avoid Buffer→string roundtrip.

-    const scriptContent = fs.readFileSync(url);
-    const script = new vm.Script(scriptContent.toString(), { filename: url });
+    const scriptContent = fs.readFileSync(url, 'utf8');
+    const script = new vm.Script(scriptContent, { filename: url });
     return script.runInContext(mtsVMContext);

135-151: Prefer fs.promises and simplify loadScript.

-  const loadScript = async (url: string) => {
-    return new Promise((resolve, reject) => {
-      fs.readFile(url, (err, data) => {
-        if (err) {
-          reject(err);
-          return;
-        }
-        try {
-          const script = new vm.Script(data.toString(), { filename: url });
-          const result = script.runInContext(mtsVMContext);
-          resolve(result);
-        } catch (e) {
-          reject(e);
-        }
-      });
-    });
-  };
+  const loadScript = async (url: string) => {
+    const data = await fs.promises.readFile(url, 'utf8');
+    const script = new vm.Script(data, { filename: url });
+    return script.runInContext(mtsVMContext);
+  };
packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts (1)

11-16: Source timers from the realm’s window to honor “real globalThis”.

Bind timer APIs to the iframe realm when available, falling back to ambient global:

-  const requestAnimationFrameBrowserImpl = requestAnimationFrame;
-  const cancelAnimationFrameBrowserImpl = cancelAnimationFrame;
-  const setTimeoutBrowserImpl = setTimeout;
-  const clearTimeoutBrowserImpl = clearTimeout;
-  const setIntervalBrowserImpl = setInterval;
-  const clearIntervalBrowserImpl = clearInterval;
+  const gw = (config.mtsRealm?.globalWindow ?? globalThis) as Window & typeof globalThis;
+  const requestAnimationFrameBrowserImpl = gw.requestAnimationFrame.bind(gw);
+  const cancelAnimationFrameBrowserImpl = gw.cancelAnimationFrame.bind(gw);
+  const setTimeoutBrowserImpl = gw.setTimeout.bind(gw);
+  const clearTimeoutBrowserImpl = gw.clearTimeout.bind(gw);
+  const setIntervalBrowserImpl = gw.setInterval.bind(gw);
+  const clearIntervalBrowserImpl = gw.clearInterval.bind(gw);

This ensures timers/RAF align with the realm hosting MTS code.

Also applies to: 21-26

packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (1)

54-67: Script injection robustness

  • Dynamically inserted scripts ignore defer; setting it is a no-op here.
  • Use head || body fallback to avoid NPEs.
-      script.fetchPriority = 'high';
-      script.defer = true;
-      script.onload = () => resolve(iframeWindow?.module?.exports);
+      script.fetchPriority = 'high';
+      script.onload = () => resolve(iframeWindow?.module?.exports);
       script.onerror = () => reject(new Error(`Failed to load script: ${url}`));
-      iframe.contentDocument!.head.appendChild(script);
+      (iframe.contentDocument!.head ?? iframe.contentDocument!.body)!.appendChild(script);
packages/web-platform/web-tests/shell-project/mainthread-test.ts (1)

37-43: Casting OffscreenDocument to Document

This cast is pragmatic for tests. Just a note: docu.commit is then hidden behind ts-expect-error at call sites. Consider narrowing a local type that includes commit() to avoid ts-expect-error.

-const docu: Document = ENABLE_MULTI_THREAD
+type TestDocument = Document & { commit?: () => void } & Record<string, any>;
+const docu: TestDocument = ENABLE_MULTI_THREAD
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (3)

644-652: Make __LoadLepusChunk resilient to missing mapping and surface the original path

Good direction using mtsRealm.loadScriptSync. To aid debugging, keep the original path in logs and avoid mutating the path var before logging.

-  const __LoadLepusChunk: (path: string) => boolean = (path) => {
+  const __LoadLepusChunk: (path: string) => boolean = (path) => {
     try {
-      path = lepusCode?.[path] ?? path;
-      mtsRealm.loadScriptSync(path);
+      const resolved = lepusCode?.[path] ?? path;
+      mtsRealm.loadScriptSync(resolved);
       return true;
     } catch (e) {
-      console.error(`failed to load lepus chunk ${path}`, e);
+      console.error(`failed to load lepus chunk ${path}`, e);
       return false;
     }
   };

586-596: __SwapElement typing: remove ts-expect-error by casting through WebFiberElementImpl

This keeps the code type-safe across both DOM and OffscreenDocument implementations.

-    const temp = document.createElement('div');
-    // @ts-expect-error fixme
-    childA.replaceWith(temp);
+    const temp = document.createElement('div') as unknown as WebFiberElementImpl;
+    childA.replaceWith(temp);
     childB.replaceWith(childA);
-    // @ts-expect-error fixme
-    temp.replaceWith(childB);
+    temp.replaceWith(childB);

289-291: Hot path: avoid cross-realm property lookup per event

Reading runWorklet off mtsRealm.globalWindow on every event can be slightly slower. Cache a ref once when it’s set.

-        (mtsRealm.globalWindow as typeof globalThis & MainThreadGlobalThis)
-          .runWorklet?.(hname.value, [crossThreadEvent]);
+        // Cache once at init time:
+        // let runWorkletRef: MainThreadGlobalThis['runWorklet'] | undefined;
+        // Object.defineProperty(mtsRealm.globalWindow, 'runWorklet', { set(v){ runWorkletRef = v; }, get(){ return runWorkletRef; }});
+        // Then:
+        (mtsRealm.globalWindow as typeof globalThis & MainThreadGlobalThis).runWorklet?.(hname.value, [crossThreadEvent]);

If you want, I can send a small follow-up to wire runWorkletRef similarly to renderPage.

packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)

85-85: Callback error param type is misaligned with usage.

The callback is typed as (message: string | null, ...) but the previous code passed an Error. Either keep the message-only approach (as per the previous diff) or adopt Error | null for stronger typing.

Apply this diff if you prefer error objects:

-      callback: (message: string | null, exports?: BundleInitReturnObj) => void,
+      callback: (err: Error | null, exports?: BundleInitReturnObj) => void,

Follow up: update the catch to pass the Error directly and successful path to pass null.


83-101: Reduce reliance on globalThis.module for async path.

The async path resolves an ESM namespace via import() but ignores it and relies on globalThis.module side effects. Prefer passing the resolved module to a helper (e.g., createBundleInitReturnObjFrom(mod)) to avoid ambient globals and bundler coupling. Keep importScripts path as-is for legacy bundles.

If desired, I can draft a minimal adapter that handles both ESM namespace objects and legacy UMD/CommonJS globals.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between d70efba and b679c9f.

📒 Files selected for processing (24)
  • .changeset/odd-cooks-play.md (1 hunks)
  • packages/web-platform/web-constants/src/constants.ts (0 hunks)
  • packages/web-platform/web-constants/src/types/JSRealm.ts (1 hunks)
  • packages/web-platform/web-constants/src/types/LynxModule.ts (1 hunks)
  • packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (0 hunks)
  • packages/web-platform/web-constants/src/types/MainThreadLynx.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 (5 hunks)
  • packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (4 hunks)
  • packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (11 hunks)
  • packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts (1 hunks)
  • packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (5 hunks)
  • packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json (1 hunks)
  • packages/web-platform/web-tests/resources/web-core.main-thread.json (1 hunks)
  • packages/web-platform/web-tests/shell-project/mainthread-test.ts (3 hunks)
  • packages/web-platform/web-tests/tests/react.spec.ts (1 hunks)
  • packages/web-platform/web-tests/tests/react/api-global-disallowed-vars/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/react/api-updateGlobalProps/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/react/basic-globalProps/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/web-core.test.ts (1 hunks)
  • packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1 hunks)
  • packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts (4 hunks)
💤 Files with no reviewable changes (2)
  • packages/web-platform/web-constants/src/constants.ts
  • packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-21T08:46:54.456Z
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.456Z
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-tests/resources/web-core.main-thread.json
  • packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json
  • packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts
  • packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts
  • packages/web-platform/web-tests/shell-project/mainthread-test.ts
📚 Learning: 2025-08-14T06:51:39.827Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/template-webpack-plugin/src/LynxEncodePlugin.ts:160-171
Timestamp: 2025-08-14T06:51:39.827Z
Learning: In the LynxEncodePlugin for events-cache.js functionality, when iterating through entrypoints to find background scripts, any entry .js file that exists in the manifest is guaranteed to be the background.js needed by design. The manifest generation logic ensures only background scripts from entrypoints are included, making the simple "first .js file in manifest" selection approach correct and reliable.

Applied to files:

  • packages/web-platform/web-tests/resources/web-core.main-thread.json
  • packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json
📚 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-tests/resources/web-core.main-thread.json
  • packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx
  • packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json
  • packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts
  • packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
  • packages/web-platform/web-tests/shell-project/mainthread-test.ts
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.

Applied to files:

  • packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx
  • packages/web-platform/web-tests/tests/react.spec.ts
  • packages/web-platform/web-tests/shell-project/mainthread-test.ts
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
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/api-globalThis/index.jsx
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.

Applied to files:

  • packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx
📚 Learning: 2025-08-27T08:10:09.899Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1612
File: packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/src/tsconfig.json:3-13
Timestamp: 2025-08-27T08:10:09.899Z
Learning: In the lynx-family/lynx-stack repository, Rspeedy templates use `lynx-js/rspeedy/client` types via `rspeedy-env.d.ts` instead of `vite/client` types. Rspeedy provides its own client-side environment type definitions and doesn't require direct Vite type references.

Applied to files:

  • packages/web-platform/web-tests/shell-project/mainthread-test.ts
🧬 Code graph analysis (11)
packages/web-platform/web-tests/tests/react/api-global-disallowed-vars/index.jsx (1)
packages/web-platform/web-constants/src/utils/LynxCrossThreadContext.ts (1)
  • postMessage (23-25)
packages/web-platform/web-tests/tests/react.spec.ts (1)
packages/web-platform/web-tests/tests/coverage-fixture.ts (2)
  • test (11-61)
  • expect (63-63)
packages/web-platform/web-core-server/src/createLynxView.ts (2)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
  • prepareMainThreadAPIs (36-227)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
  • MainThreadGlobalThis (304-388)
packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts (1)
packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (2)
  • requestAnimationFrame (102-104)
  • cancelAnimationFrame (105-107)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
  • LynxTemplate (21-41)
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)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (3)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
  • JSRealm (1-5)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
  • MainThreadGlobalThis (304-388)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
  • prepareMainThreadAPIs (36-227)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
  • JSRealm (1-5)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (4)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
  • LynxTemplate (21-41)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
  • JSRealm (1-5)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
  • MainThreadGlobalThis (304-388)
packages/web-platform/web-constants/src/types/Element.ts (1)
  • WebFiberElementImpl (51-96)
packages/web-platform/web-tests/shell-project/mainthread-test.ts (1)
packages/web-platform/offscreen-document/src/webworker/OffscreenDocument.ts (2)
  • OffscreenDocument (23-128)
  • operations (17-17)
packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts (7)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
  • JSRealm (1-5)
packages/web-platform/offscreen-document/src/webworker/index.ts (2)
  • OffscreenDocument (4-4)
  • _onEvent (4-4)
packages/web-platform/offscreen-document/src/webworker/OffscreenDocument.ts (2)
  • OffscreenDocument (23-128)
  • _onEvent (20-20)
packages/web-platform/web-constants/src/types/I18n.ts (1)
  • I18nResources (31-39)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
  • prepareMainThreadAPIs (36-227)
packages/web-platform/web-worker-runtime/src/mainThread/crossThreadHandlers/registerUpdateDataHandler.ts (1)
  • registerUpdateDataHandler (10-20)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
  • MainThreadGlobalThis (304-388)
🪛 GitHub Actions: Test
packages/web-platform/web-tests/tests/react.spec.ts

[error] 1162-1162: Test failed: expected 'message' to contain 'pass:dataset2', but received [] (api-exposure-stop-events-has-complex-dataset).

🔇 Additional comments (18)
packages/web-platform/web-tests/tests/react.spec.ts (1)

1287-1305: I couldn’t find any dual or chained console method assignments in the current testing-environment/src/index.ts. The snippet around lines 240–300 also shows no such code. Please verify that the console-hooking logic still exists (it may have been refactored or moved) and that both target.console and the global console methods are reassigned as required by rstest.

packages/web-platform/web-tests/tests/web-core.test.ts (1)

204-204: LGTM — runtime-scoped processor access matches the new global model.

This aligns with the realm-based globalThis.runtime.* surface.

packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json (1)

10-10: Clarify load order for runtime/__lynx_worker_type writes

root sets self.runtime = globalThis and __lynx_worker_type = 'main', while manifest sets globalThis.runtime = lynxCoreInject.tt and __lynx_worker_type = 'background'. Confirm intended execution order so the effective runtime/worker type is deterministic across tests.

packages/web-platform/web-constants/src/types/MainThreadLynx.ts (1)

12-15: Expose timer APIs: implementation wired correctly; confirm handle type

Verified that createMainThreadLynx aliases the global timer functions and exposes them on the returned MainThreadLynx object:

  • Aliases created at lines 13–16 in createMainThreadLynx.ts:
    const setTimeoutBrowserImpl = setTimeout;
    const clearTimeoutBrowserImpl = clearTimeout;
    const setIntervalBrowserImpl = setInterval;
    const clearIntervalBrowserImpl = clearInterval;
  • Exposed on the returned object at lines 33–36:
    setTimeout: setTimeoutBrowserImpl,
    clearTimeout: clearTimeoutBrowserImpl,
    setInterval: setIntervalBrowserImpl,
    clearInterval: clearIntervalBrowserImpl,

These all match the typeof setTimeout etc. declarations in MainThreadLynx.ts. Please ensure your tsconfig.json includes the correct lib (e.g. "dom") so that the timer handle return type resolves to the expected number rather than NodeJS.Timeout.

packages/web-platform/web-tests/resources/web-core.main-thread.json (1)

9-9: Double-write of runtime/worker type

As in the other test resource, confirm the intended winner between root and /app-service.js assignments to avoid test flakes.

packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)

71-82: Verify disallowed globals won’t break existing v2 lepusCode

Masking navigator and postMessage globally is intentional, but older lepusCode that accessed navigator without injection will now see undefined. Ensure tests cover this or expand the upgrader to inject these for lepusCode where needed.

packages/web-platform/web-constants/src/types/index.ts (1)

22-22: LGTM: re-exporting JSRealm

Type surface expansion is straightforward and aligns with the realm migration.

.changeset/odd-cooks-play.md (1)

2-7: SemVer: confirm minor vs. breaking.

Switching to a real globalThis can change observable behavior (e.g., cross-realm globals, prototype identity). Please confirm this is not breaking for consumers; otherwise mark with BREAKING and bump major for the affected packages. Do you want me to draft migration notes?

packages/web-platform/web-core-server/src/createLynxView.ts (2)

152-160: Confirm prepareMainThreadAPIs arg order.

Signature is (rpc, rootDom, document, mtsRealm, ...). Passing offscreenDocument for both is intentional, correct?


112-114: Portability: Promise.withResolvers requires modern Node.

If your server Node version < 20.12, this API may be missing. Consider a small helper fallback or confirm CI/runtime version.

packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts (1)

28-30: LGTM: custom sections now read from lynxTemplate.

packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts (2)

77-83: Attach document onto globalThis: LGTM

Assigning OffscreenDocument to globalThis.document early keeps downstream code simple and is consistent with previous patterns.


112-119: Register update handler after start: LGTM

Deferring registerUpdateDataHandler until after await startMainThread ensures updatePage is present before updates arrive.

packages/web-platform/web-tests/shell-project/mainthread-test.ts (1)

102-116: Stubbed realm script loaders in tests

Throwing in loadScript/loadScriptSync is fine here since tests call createMainThreadGlobalThis directly. If any test starts exercising __LoadLepusChunk, it will break.

If you expect lazy chunks in these tests soon, I can wire a minimal in-memory loader that returns canned module.exports to keep tests hermetic. Want me to add it?

packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (2)

98-111: Realm-first initialization: LGTM

Passing document and mtsRealm into createMainThreadGlobalThis cleanly separates DOM ops from code execution realm.


139-151: Eager backgroundStart payload filtering

The lazy customSections filtering is correct and avoids shipping lazy content to the BG thread.

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (1)

848-859: Global projection into realm: LGTM

Assigning mtsGlobalThis into mtsRealm.globalWindow and intercepting renderPage ensures mainChunkReady runs exactly once when the app sets it.

Also applies to: 861-862

packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1)

65-68: BundleInitReturnObj.init is still present; no core runtime calls to .init(…) detected

  • The BundleInitReturnObj type in packages/web-platform/web-constants/src/types/NativeApp.ts still includes an init(opt: { tt: NativeTTObject }) method.
  • A project-wide search found no .init( invocations in the web-worker-runtime or web-mainthread-apis production code.
  • The only references to .init( are in:
    packages/webpack/runtime-wrapper-webpack-plugin/test/cases.test.js:26–28 (plugin tests)
    packages/rspeedy/plugin-react/src/css.ts:160–162 (plugin-side API)
    • Various i18next examples in the website/docs folders
  • Therefore, the background-thread helper change does not break any core runtime consumers. All existing .init usages are isolated to plugin tests and documentation.

@PupilTong PupilTong requested a review from Sherry-hue August 27, 2025 13:01
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (3)

135-137: Pending updates can be dropped: gate on runtime readiness and updatePage presence

mtsGlobalThis is always truthy; early calls no-op via optional chaining. Queue until the runtime is started and updatePage is available, then flush.

   const pendingUpdateCalls: Parameters<
     RpcCallType<typeof updateDataEndpoint>
   >[] = [];
+  let runtimeStarted = false;
-      await startMainThread(configs, {
+      await startMainThread(configs, {
         // @ts-expect-error
         lynxUniqueIdToElement: lynxUniqueIdToElement,
         lynxUniqueIdToStyleRulesIndex,
         ...ssrDumpInfo,
         cardStyleElement: hydrateStyleElement,
       });
+      runtimeStarted = true;
-      await startMainThread(configs);
+      await startMainThread(configs);
+      runtimeStarted = true;
-    // Process any pending update calls that were queued while mtsGlobalThis was undefined
+    // Flush any updates queued before the runtime was ready
   const updateDataMainThread: RpcCallType<typeof updateDataEndpoint> = async (
     ...args
   ) => {
-    if (mtsGlobalThis) {
-      mtsGlobalThis.updatePage?.(...args);
-    } else {
-      // Cache the call if mtsGlobalThis is not yet initialized
-      pendingUpdateCalls.push(args);
-    }
+    if (!runtimeStarted || typeof mtsGlobalThis.updatePage !== 'function') {
+      pendingUpdateCalls.push(args);
+      return;
+    }
+    mtsGlobalThis.updatePage(...args);
   };

Also applies to: 173-181, 184-198


54-69: Deterministic, CSP-safe dynamic loading: resolve base URL, set async before src, head/body fallback

Preserve insertion order, avoid about:blank-relative resolution, and prevent head null deref. Defer has no effect for dynamically inserted scripts.

-  const loadScript: (url: string) => Promise<unknown> = (url) => {
+  const loadScript: (url: string) => Promise<unknown> = (url) => {
     return new Promise(async (resolve, reject) => {
       if (iframeDocument.readyState !== 'complete') {
         await iframeLoaded;
       }
-      const script = iframeDocument.createElement('script');
-      script.src = url;
-      script.fetchPriority = 'high';
-      script.defer = true;
-      script.async = false;
-      script.onload = () => resolve(iframeWindow?.module?.exports);
-      script.onerror = (err) =>
-        reject(new Error(`Failed to load script: ${url}`, { cause: err }));
-      iframe.contentDocument!.head.appendChild(script);
+      const doc = iframe.contentDocument!;
+      const target = doc.head ?? doc.body ?? doc.documentElement;
+      if (!target) return reject(new Error('Realm document not ready'));
+      // Reset exports per load to avoid staleness between chunks.
+      iframeWindow.module = { exports: undefined };
+      const resolved = new URL(url, document.baseURI).toString();
+      const script = doc.createElement('script');
+      script.async = false; // set before src to preserve order across browsers
+      script.fetchPriority = 'high';
+      script.src = resolved;
+      script.onload = () => resolve(iframeWindow?.module?.exports);
+      script.onerror = (err) =>
+        reject(new Error(`Failed to load script: ${resolved}`, { cause: err as Error }));
+      target.appendChild(script);
     });
   };

70-82: Blocker: synchronous XHR on the UI thread causes perf regressions and CSP breakage

XMLHttpRequest(..., false) blocks the main thread and inline textContent is often blocked by CSP without unsafe-inline. This aligns with the reported 16–28% regressions. Remove sync loading on UI realms or make it throw to force async usage. Keep sync only for Worker realms.

-  const loadScriptSync: (url: string) => unknown = (url) => {
-    const xhr = new XMLHttpRequest();
-    xhr.open('GET', url, false); // Synchronous request
-    xhr.send(null);
-    if (xhr.status === 200) {
-      const script = iframe.contentDocument!.createElement('script');
-      script.textContent = xhr.responseText;
-      iframe.contentDocument!.head.appendChild(script);
-      return iframeWindow?.module?.exports;
-    } else {
-      throw new Error(`Failed to load script: ${url}`, { cause: xhr });
-    }
-  };
+  const loadScriptSync: (url: string) => unknown = (_url) => {
+    throw new Error('loadScriptSync is not supported on UI realms; use loadScript() instead.');
+  };

Run to find and fix UI-thread callsites relying on sync:

#!/bin/bash
rg -nP --type=ts -C2 '\bloadScriptSync\s*\('

Note: For Worker-only paths, keep the fetch()+importScripts() pattern (as per retrieved learnings) to leverage HTTP caching.

🧹 Nitpick comments (1)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (1)

40-47: Sandbox the iframe realm and plan for cleanup

The realm iframe is unsandboxed and never disposed. Add minimal sandboxing and expose a dispose hook to avoid leaks.

-  const iframe = document.createElement('iframe');
+  const iframe = document.createElement('iframe');
+  // Minimal sandbox; keep same-origin so parent can manage the realm.
+  iframe.sandbox.add('allow-scripts');
+  iframe.sandbox.add('allow-same-origin');
   const iframeLoaded = new Promise<void>((resolve) => {
     iframe.onload = () => resolve();
   });
   iframe.style.display = 'none';
   iframe.src = 'about:blank';
   parent.appendChild(iframe);

Optionally return a disposer and wire it up at call sites:

-  return { globalWindow: iframeWindow, loadScript, loadScriptSync };
+  const dispose = () => iframe.remove();
+  return { globalWindow: iframeWindow, loadScript, loadScriptSync, /* @future */ dispose };

Outside this file, consider extending JSRealm with an optional dispose(): void to formalize teardown.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2facfa9 and 9aa3f11.

📒 Files selected for processing (1)
  • packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (4 hunks)
🧰 Additional context used
🧠 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-core/src/uiThread/createRenderAllOnUI.ts
🧬 Code graph analysis (1)
packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (3)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
  • JSRealm (1-5)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
  • MainThreadGlobalThis (304-388)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
  • prepareMainThreadAPIs (36-227)
⏰ 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 (javascript-typescript)
  • GitHub Check: CodeQL Analyze (actions)

@PupilTong PupilTong force-pushed the p/hw/real-global-this branch from 8c5c477 to 07a84bf Compare August 27, 2025 13:35
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

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)

98-105: Non-string manifest/lepus entries are dropped. Preserve pass-through entries.

The current filter removes non-string values from the result.

-  return Promise.all(
-    (Object.entries(obj).filter(([_, content]) =>
-      typeof content === 'string'
-    ) as [string, string][]).map(processEntry),
-  ).then(
-    Object.fromEntries,
-  );
+  const entries = Object.entries(obj) as [string, string | {}][];
+  const processed = await Promise.all(
+    entries.map(async ([name, content]) =>
+      typeof content === 'string'
+        ? await processEntry([name, content])
+        : [name, content],
+    ),
+  );
+  return Object.fromEntries(processed) as T;
♻️ Duplicate comments (1)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)

39-41: Document template version semantics (scope, bump rules, upgrade path).

Add JSDoc clarifying valid values (e.g., 1|2), bump policy, and that generateTemplate auto-upgrades to currentSupportedTemplateVersion.

   elementTemplate: Record<string, ElementTemplateData[]>;
-  version?: number;
+  /**
+   * Template schema version.
+   * - 1: legacy (no module wrapper); will be auto-upgraded.
+   * - 2: current (realm-ready).
+   * Omit to default to 1. Auto-upgraded by generateTemplate() up to the supported version.
+   */
+  version?: number; // consider narrowing to 1 | 2 when v3 lands
🧹 Nitpick comments (13)
packages/web-platform/web-tests/tests/react/basic-globalProps/index.jsx (1)

12-12: Declare lynx as a known global to satisfy linters.
If eslint/no-undef is enforced in tests, add a file-level declaration to avoid false positives.

Add near the top of the file (above imports):

/* global lynx */
// or, if needed by your rules:
// /* eslint-disable no-underscore-dangle */
packages/web-platform/web-constants/src/types/MainThreadLynx.ts (1)

12-15: Bind timers to their originating realm; consider making them readonly to prevent accidental mutation

When these are passed across realms (iframe/VM), ensure the functions are bound to the source realm to avoid illegal invocation or scheduling into the wrong event loop. Making them readonly helps avoid accidental reassignment post-construction.

Example (in createMainThreadLynx.ts or equivalent):

const realmGlobal = realm.globalThis as Window & typeof globalThis;
return {
  // ...
  setTimeout: realmGlobal.setTimeout.bind(realmGlobal),
  clearTimeout: realmGlobal.clearTimeout.bind(realmGlobal),
  setInterval: realmGlobal.setInterval.bind(realmGlobal),
  clearInterval: realmGlobal.clearInterval.bind(realmGlobal),
};

Optional diff to mark properties readonly here:

-  setTimeout: typeof setTimeout;
-  clearTimeout: typeof clearTimeout;
-  setInterval: typeof setInterval;
-  clearInterval: typeof clearInterval;
+  readonly setTimeout: typeof setTimeout;
+  readonly clearTimeout: typeof clearTimeout;
+  readonly setInterval: typeof setInterval;
+  readonly clearInterval: typeof clearInterval;
packages/web-platform/web-core-server/src/createLynxView.ts (5)

36-37: Imports look fine; consider promise-based fs only if you refactor below.
You can keep node:fs for sync needs; async bits below can use fs.promises to drop the manual Promise wrapper.


130-151: Simplify script loading; improve diagnostics.
Use fs.promises and pass displayErrors to surface better stack traces; also specify utf8 once (minor perf/clarity). Consider caching compiled vm.Script by filename if the same script is loaded across renders.

-const loadScriptSync = (url: string) => {
-  const scriptContent = fs.readFileSync(url);
-  const script = new vm.Script(scriptContent.toString(), { filename: url });
-  return script.runInContext(mtsVMContext);
-};
+const loadScriptSync = (url: string) => {
+  const scriptContent = fs.readFileSync(url, 'utf8');
+  const script = new vm.Script(scriptContent, { filename: url });
+  return script.runInContext(mtsVMContext, { displayErrors: true });
+};
 
-const loadScript = async (url: string) => {
-  return new Promise((resolve, reject) => {
-    fs.readFile(url, (err, data) => {
-      if (err) {
-        reject(err);
-        return;
-      }
-      try {
-        const script = new vm.Script(data.toString(), { filename: url });
-        const result = script.runInContext(mtsVMContext);
-        resolve(result);
-      } catch (e) {
-        reject(e);
-      }
-    });
-  });
-};
+const loadScript = async (url: string) => {
+  const data = await fs.promises.readFile(url, 'utf8');
+  const script = new vm.Script(data, { filename: url });
+  return script.runInContext(mtsVMContext, { displayErrors: true });
+};

If you want optional compile caching (helps with repeated renders), I can provide a small Map<string, vm.Script> wrapper.


191-204: Start-up flow: OK; consider early/cached load to mitigate perf report.
await startMainThread({...}) after one loadScript is fine. Given CodSpeed shows regressions, consider pre-compiling/caching the main chunk (see note above) to shave cold-start overhead.


212-212: Access pattern for ssrEncode is fine; add a null-safe string fallback if needed.
If downstream expects a string, coerce to string | undefined safely.

-const ssrEncodeData = (mtsVMContext as MainThreadGlobalThis)?.ssrEncode?.();
+const ssrEncodeData = (mtsVMContext as MainThreadGlobalThis)?.ssrEncode?.() ?? undefined;

125-129: Seed browser-like globals & improve CommonJS interop

Node.js v24 (per .nvmrc) and the project’s "engines": "^22 || ^24" guarantee support for vm.constants.DONT_CONTEXTIFY, so it’s safe to leave that as-is. To reduce surprises when loading UMD bundles or libraries that probe for window/self/globalThis, and to avoid module.exports = null–style gotchas, you may optionally update the context assignment as follows:

- Object.assign(mtsVMContext, {
-   document: offscreenDocument,
-   module: { exports: null },
- });
+ Object.assign(mtsVMContext as any, {
+   // DOM API stub
+   document: offscreenDocument,
+   // Mirror common global aliases:
+   globalThis: mtsVMContext as any,
+   window: mtsVMContext as any,
+   self: mtsVMContext as any,
+   // Initialize CommonJS exports safely:
+   module: { exports: {} },
+   exports: {},
+});

This change is purely opt-in—there are no mandatory fixes required here.

packages/web-platform/web-constants/src/utils/generateTemplate.ts (4)

95-96: Avoid "undefined-" in file names when templateName is not provided.

-      `${templateName}-${name.replaceAll('/', '')}.js`,
+      `${templateName ? templateName + '-' : ''}${name.replaceAll('/', '')}.js`,

115-119: Error message lacks actionable version info. Include actual and supported versions.

-    throw new Error(
-      `Unsupported template, please upgrade your web-platform dependencies`,
-    );
+    throw new Error(
+      `Unsupported template version: ${template.version}. Supported up to ${currentSupportedTemplateVersion}. Please upgrade @lynx-js/web-platform dependencies.`,
+    );

120-126: Upgrader mutates the input template; consider defensive copy.

Mutation can surprise callers reusing the same object.

-  while (
+  // Optionally: clone before upgrading to avoid in-place mutation
+  // template = structuredClone?.(template) ?? JSON.parse(JSON.stringify(template));
+  while (

8-8: Disallowed globals set is incomplete vs injected BOM list; confirm policy and align.

Only 'navigator' and 'postMessage' are shadowed, while the upgrader injects many BOM symbols. Either document the intended minimal set or expand globalDisallowedVars to match policy to avoid leakage.

Would you like me to generate a PR-local audit script to diff the effective denylist against usages inside transformed modules?

Also applies to: 26-51

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (2)

731-769: Template caching path does extra DOM append and recursion; reduce work.

Avoid appending the template element into rootDom and re-entering __ElementFromBinary. Cache off-DOM and clone directly to cut allocations and event loop churn (benchmarks show regressions).

-        if (rootDom.cloneNode) {
-          const template = document.createElement(
-            'template',
-          ) as unknown as HTMLTemplateElement;
-          template.content.append(...clonedElements as unknown as Node[]);
-          templateIdToTemplate[templateId] = template;
-          rootDom.append(template);
-          return __ElementFromBinary(templateId, parentComponentUniId);
-        }
+        if (rootDom.cloneNode) {
+          const template = document.createElement('template') as unknown as HTMLTemplateElement;
+          template.content.append(...(clonedElements as unknown as Node[]));
+          templateIdToTemplate[templateId] = template;
+          // no DOM append; clone from cache right away
+          clonedElements = Array.from(template.content.cloneNode(true).children) as unknown as WebFiberElementImpl[];
+        }

165-177: API shifts to lynxTemplate/mtsRealm/document look consistent; add minimal tests.

Add tests covering:

  • __LoadLepusChunk mapping works when lepusCode[path] is undefined.
  • Event dispatch path with runWorklet called via realm.

I can add tests under packages/web-platform/web-tests for these cases.

Also applies to: 178-179

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 9aa3f11 and 07a84bf.

📒 Files selected for processing (24)
  • .changeset/odd-cooks-play.md (1 hunks)
  • packages/web-platform/web-constants/src/constants.ts (0 hunks)
  • packages/web-platform/web-constants/src/types/JSRealm.ts (1 hunks)
  • packages/web-platform/web-constants/src/types/LynxModule.ts (1 hunks)
  • packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (0 hunks)
  • packages/web-platform/web-constants/src/types/MainThreadLynx.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 (5 hunks)
  • packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts (4 hunks)
  • packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (11 hunks)
  • packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts (1 hunks)
  • packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (5 hunks)
  • packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json (1 hunks)
  • packages/web-platform/web-tests/resources/web-core.main-thread.json (1 hunks)
  • packages/web-platform/web-tests/shell-project/mainthread-test.ts (3 hunks)
  • packages/web-platform/web-tests/tests/react.spec.ts (1 hunks)
  • packages/web-platform/web-tests/tests/react/api-global-disallowed-vars/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/react/api-updateGlobalProps/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/react/basic-globalProps/index.jsx (1 hunks)
  • packages/web-platform/web-tests/tests/web-core.test.ts (1 hunks)
  • packages/web-platform/web-worker-runtime/src/backgroundThread/background-apis/createNativeApp.ts (1 hunks)
  • packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts (4 hunks)
💤 Files with no reviewable changes (2)
  • packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts
  • packages/web-platform/web-constants/src/constants.ts
🚧 Files skipped from review as they are similar to previous changes (16)
  • packages/web-platform/web-constants/src/types/index.ts
  • packages/web-platform/web-tests/tests/web-core.test.ts
  • packages/web-platform/web-tests/tests/react/api-globalThis/index.jsx
  • packages/web-platform/web-tests/tests/react/api-updateGlobalProps/index.jsx
  • packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.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-tests/shell-project/mainthread-test.ts
  • .changeset/odd-cooks-play.md
  • packages/web-platform/web-tests/tests/react/api-global-disallowed-vars/index.jsx
  • packages/web-platform/web-constants/src/types/JSRealm.ts
  • packages/web-platform/web-tests/resources/web-core.main-thread.json
  • packages/web-platform/web-worker-runtime/src/mainThread/startMainThread.ts
  • packages/web-platform/web-tests/resources/web-core.enable-css-selector-false.json
  • packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts
🧰 Additional context used
🧠 Learnings (7)
📚 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-constants/src/utils/generateTemplate.ts
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.

Applied to files:

  • packages/web-platform/web-constants/src/utils/generateTemplate.ts
📚 Learning: 2025-08-20T04:56:36.011Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1566
File: package.json:53-53
Timestamp: 2025-08-20T04:56:36.011Z
Learning: In lynx-stack, Node.js v24 is the preferred/default version for development (established in PR #1557), but Node.js v22 compatibility is maintained specifically for external CI systems like rspack-ecosystem-ci. The engines.node specification uses "^22 || ^24" to support both versions while keeping v24 as the primary target.

Applied to files:

  • packages/web-platform/web-constants/src/utils/generateTemplate.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-mainthread-apis/src/createMainThreadGlobalThis.ts
📚 Learning: 2025-08-27T11:34:29.404Z
Learnt from: PupilTong
PR: lynx-family/lynx-stack#1589
File: packages/web-platform/web-core-server/src/createLynxView.ts:125-129
Timestamp: 2025-08-27T11:34:29.404Z
Learning: vm.createContext(vm.constants.DONT_CONTEXTIFY) is a valid usage documented in Node.js official documentation. vm.constants.DONT_CONTEXTIFY creates a context without contextifying its global object, making globalThis behave more closely to an ordinary one and allowing the context to be frozen if needed.

Applied to files:

  • packages/web-platform/web-core-server/src/createLynxView.ts
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.

Applied to files:

  • packages/web-platform/web-core-server/src/createLynxView.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-server/src/createLynxView.ts
🧬 Code graph analysis (3)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
  • LynxTemplate (21-41)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (4)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
  • LynxTemplate (21-41)
packages/web-platform/web-constants/src/types/JSRealm.ts (1)
  • JSRealm (1-5)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
  • MainThreadGlobalThis (304-388)
packages/web-platform/web-constants/src/types/Element.ts (1)
  • WebFiberElementImpl (51-96)
packages/web-platform/web-core-server/src/createLynxView.ts (2)
packages/web-platform/web-mainthread-apis/src/prepareMainThreadAPIs.ts (1)
  • prepareMainThreadAPIs (36-227)
packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts (1)
  • MainThreadGlobalThis (304-388)
⏰ 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). (7)
  • GitHub Check: build / Build (Windows)
  • GitHub Check: build / Build (Ubuntu)
  • GitHub Check: test-rust / Test (Ubuntu)
  • GitHub Check: code-style-check
  • GitHub Check: CodeQL Analyze (javascript-typescript)
  • GitHub Check: zizmor
  • GitHub Check: CodeQL Analyze (actions)
🔇 Additional comments (8)
packages/web-platform/web-tests/tests/react/basic-globalProps/index.jsx (1)

12-12: LGTM: switch to lynx.__globalProps aligns with the new global access pattern.
Change is consistent with the realm-based global strategy and keeps the test intent intact.

packages/web-platform/web-constants/src/types/MainThreadLynx.ts (1)

12-15: Confirm breaking change flagged & implementation coverage

I searched the repo for all references to MainThreadLynx and found:

  • The interface in packages/web-platform/web-constants/src/types/MainThreadLynx.ts now adds the four timer APIs.
  • The default implementation in packages/web-platform/web-mainthread-apis/src/createMainThreadLynx.ts returns all four (setTimeout, clearTimeout, setInterval, clearInterval), so the built-in provider is up-to-date.
  • No other classes or objects implement or assert MainThreadLynx elsewhere in the codebase.

Next steps:

  • Ensure your changeset is marked as a breaking change so downstream consumers are alerted.
  • If you have any custom implementers outside of this repo, update them to include the new timer methods.
packages/web-platform/web-core-server/src/createLynxView.ts (2)

10-11: LGTM: type-only import is correct and used precisely where needed.
No issues with importing type MainThreadGlobalThis for the narrow cast at render time.


155-160: prepareMainThreadAPIs signature alignment looks correct.
Passing document rather than a bound creator and providing { globalWindow, loadScript, loadScriptSync } matches the new JSRealm-based shape.

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (4)

434-437: Switch to document.createElement for element creation is correct.


848-859: renderPage getter/setter overlay is fine; ensure mainChunkReady semantics are preserved.

If renderPage is reassigned frequently, this will queue many microtasks; confirm intended behavior.

Would you like a small harness to assert that mainChunkReady fires exactly once when SSR boot sets renderPage?


644-651: Good: lepus chunk resolution and error logging in realm.


211-219: Good: style element creation now uses real document from realm.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (2)

71-82: Define module/exports in wrapper and return exports to avoid ReferenceError and enable CJS interop.

module is not defined in the IIFE; assigning to module.exports will throw. Also the wrapper does not return the export, which breaks evaluators relying on the IIFE’s return value.

Apply:

-    '"use strict";\n',
-    `(() => {const ${
-      globalDisallowedVars.join('=void 0,')
-    }=void 0;module.exports = `,
+    '"use strict";\n',
+    `(() => {const ${
+      globalDisallowedVars.join('=void 0,')
+    }=void 0;var module={exports:{}};var exports=module.exports;module.exports = `,
     content,
-    '\n})()',
+    '\n;return module.exports;\n})()',

54-59: Initialize CommonJS shims correctly in v1→v2 manifest upgrader.

module = {exports:null} breaks modules that mutate/extend exports; exports alias is also missing.

Apply:

-        `{init: (lynxCoreInject) => { var {${defaultInjectStr}} = lynxCoreInject.tt; var module = {exports:null}; ${value}\n return module.exports; } }`,
+        `{init: (lynxCoreInject) => { var {${defaultInjectStr}} = lynxCoreInject.tt; var module = {exports:{}}; var exports = module.exports; ${value}\n return module.exports; } }`,
🧹 Nitpick comments (6)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (6)

95-96: Avoid “undefined-” prefix in generated filenames when templateName is absent.

Apply:

-      `${templateName}-${name.replaceAll('/', '')}.js`,
+      `${templateName ? `${templateName}-` : ''}${name.replaceAll('/', '')}.js`,

114-119: Improve unsupported-version error message with actual numbers.

Apply:

-    throw new Error(
-      `Unsupported template, please upgrade your web-platform dependencies`,
-    );
+    throw new Error(
+      `Unsupported template version ${template.version}; supported up to ${currentSupportedTemplateVersion}. Please upgrade your web-platform dependencies`,
+    );

7-10: Document template schema versioning policy (not tied to package 0.x semver).

Add a brief comment so future bumps are clear and discoverable.

Apply:

-const currentSupportedTemplateVersion = 2;
+/** Template schema version. Increment on breaking template-shape changes; not tied to package 0.x semver. */
+const currentSupportedTemplateVersion = 2;

11-50: Verify injected identifiers and casing.

Check that every name in defaultInjectStr matches what lynxCoreInject.tt actually provides (e.g., Caches vs the standard caches). Mismatch will cause ReferenceError during init.


84-105: Generic helper drops non-string entries; either constrain the type or preserve them.

Given the generic signature, entries with non-string values are filtered out. If this helper is only for string maps, narrow the type; otherwise, pass non-string entries through unchanged.

Apply (to preserve pass-through):

-  return Promise.all(
-    (Object.entries(obj).filter(([_, content]) =>
-      typeof content === 'string'
-    ) as [string, string][]).map(processEntry),
-  ).then(
-    Object.fromEntries,
-  );
+  const pairs = await Promise.all(
+    Object.entries(obj).map(async ([name, content]) =>
+      typeof content === 'string'
+        ? await processEntry([name, content])
+        : [name, content]
+    ),
+  );
+  return Object.fromEntries(pairs) as T;

60-67: Confirm lepus IIFE semantics are intended.

lepusCode exports become the IIFE’s return (often undefined) after the outer wrapper assigns to module.exports. If consumers expect side-effects only, this is fine; otherwise, consider returning a value.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 07a84bf and 8ce6e87.

📒 Files selected for processing (1)
  • packages/web-platform/web-constants/src/utils/generateTemplate.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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-constants/src/utils/generateTemplate.ts
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.

Applied to files:

  • packages/web-platform/web-constants/src/utils/generateTemplate.ts
📚 Learning: 2025-08-20T04:56:36.011Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1566
File: package.json:53-53
Timestamp: 2025-08-20T04:56:36.011Z
Learning: In lynx-stack, Node.js v24 is the preferred/default version for development (established in PR #1557), but Node.js v22 compatibility is maintained specifically for external CI systems like rspack-ecosystem-ci. The engines.node specification uses "^22 || ^24" to support both versions while keeping v24 as the primary target.

Applied to files:

  • packages/web-platform/web-constants/src/utils/generateTemplate.ts
🧬 Code graph analysis (1)
packages/web-platform/web-constants/src/utils/generateTemplate.ts (1)
packages/web-platform/web-constants/src/types/LynxModule.ts (1)
  • LynxTemplate (21-41)
⏰ 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: build / Build (Ubuntu)
  • GitHub Check: test-rust / Test (Ubuntu)
  • GitHub Check: CodeQL Analyze (javascript-typescript)

@PupilTong PupilTong merged commit c1f8715 into lynx-family:main Aug 28, 2025
189 of 202 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Lynx for Web Aug 28, 2025
colinaaa pushed a commit that referenced this pull request Sep 1, 2025
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]

### Minor Changes

- Deprecate `source.alias`, use `resolve.alias` instead.
([#1610](#1610))

Note that `resolve.alias` has **lower** priority than the deprecated
`source.alias`.

- Bump Rsbuild v1.5.0 with Rspack v1.5.0.
([#1591](#1591))

- **BREAKING CHANGE**: Remove the `./register` exports from
`@lynx-js/rspeedy`.
([#1547](#1547))

    This should not affect most users.

### Patch Changes

- Support `resolve.alias`.
([#1610](#1610))

- Support `rspeedy build --watch`
([#1579](#1579))

- Updated dependencies
\[[`d7d0b9b`](d7d0b9b),
[`1952fc1`](1952fc1)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Minor Changes

- refactor: provide the mts a real globalThis
([#1589](#1589))

Before this change, We create a function wrapper and a fake globalThis
for Javascript code.

    This caused some issues.

After this change, we will create an iframe for createing an isolated
Javascript context.

    This means the globalThis will be the real one.

### Patch Changes

- refactor: add `:not([l-e-name])` at the end of selector for lazy
component ([#1622](#1622))

- fix: the SystemInfo in bts should be assigned to the globalThis
([#1599](#1599))

- Updated dependencies
\[[`c1f8715`](c1f8715)]:
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Minor Changes

- refactor: provide the mts a real globalThis
([#1589](#1589))

Before this change, We create a function wrapper and a fake globalThis
for Javascript code.

    This caused some issues.

After this change, we will create an iframe for createing an isolated
Javascript context.

    This means the globalThis will be the real one.

### Patch Changes

- refactor: add `:not([l-e-name])` at the end of selector for lazy
component ([#1622](#1622))

- feat: remove multi-thread mts heating
([#1597](#1597))

The default rendering mode is "all-on-ui". Therefore the preheating for
"multi-thread" will be removed.

- fix: the SystemInfo in bts should be assigned to the globalThis
([#1599](#1599))

- Updated dependencies
\[[`1a32dd8`](1a32dd8),
[`bb53d9a`](bb53d9a),
[`1a32dd8`](1a32dd8),
[`c1f8715`](c1f8715)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Minor Changes

- refactor: provide the mts a real globalThis
([#1589](#1589))

Before this change, We create a function wrapper and a fake globalThis
for Javascript code.

    This caused some issues.

After this change, we will create an iframe for createing an isolated
Javascript context.

    This means the globalThis will be the real one.

## @lynx-js/[email protected]

### Minor Changes

- refactor: provide the mts a real globalThis
([#1589](#1589))

Before this change, We create a function wrapper and a fake globalThis
for Javascript code.

    This caused some issues.

After this change, we will create an iframe for createing an isolated
Javascript context.

    This means the globalThis will be the real one.

### Patch Changes

- refactor: add `:not([l-e-name])` at the end of selector for lazy
component ([#1622](#1622))

- fix: the SystemInfo in bts should be assigned to the globalThis
([#1599](#1599))

- Updated dependencies
\[[`1a32dd8`](1a32dd8),
[`bb53d9a`](bb53d9a),
[`c1f8715`](c1f8715)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Minor Changes

- refactor: provide the mts a real globalThis
([#1589](#1589))

Before this change, We create a function wrapper and a fake globalThis
for Javascript code.

    This caused some issues.

After this change, we will create an iframe for createing an isolated
Javascript context.

    This means the globalThis will be the real one.

## @lynx-js/[email protected]

### Minor Changes

- refactor: provide the mts a real globalThis
([#1589](#1589))

Before this change, We create a function wrapper and a fake globalThis
for Javascript code.

    This caused some issues.

After this change, we will create an iframe for createing an isolated
Javascript context.

    This means the globalThis will be the real one.

### Patch Changes

- fix: the SystemInfo in bts should be assigned to the globalThis
([#1599](#1599))

- Updated dependencies
\[[`1a32dd8`](1a32dd8),
[`bb53d9a`](bb53d9a),
[`1a32dd8`](1a32dd8),
[`c1f8715`](c1f8715)]:
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]
    -   @lynx-js/[email protected]

## @lynx-js/[email protected]

### Patch Changes

- Remove the "key is not on root element of snapshot" warning.
([#1558](#1558))

## [email protected]

### Patch Changes

- Use TypeScript [Project
References](https://www.typescriptlang.org/docs/handbook/project-references.html)
in templates.
([#1612](#1612))

## @lynx-js/[email protected]

### Patch Changes

- remove innerHTML, replace it by textContent
([#1622](#1622))

## @lynx-js/[email protected]

### Patch Changes

- Fix that `__webpack_require__.lynx_ce` is incorrectly injected when
lazy bundle is enabled.
([#1616](#1616))

## @lynx-js/[email protected]

### Patch Changes

- Should not load initial CSS chunks.
([#1601](#1601))

## [email protected]



## @lynx-js/[email protected]

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants