Skip to content

feat: support createElement#2360

Open
HuJean wants to merge 1 commit intomainfrom
p/createElement
Open

feat: support createElement#2360
HuJean wants to merge 1 commit intomainfrom
p/createElement

Conversation

@HuJean
Copy link
Copy Markdown
Collaborator

@HuJean HuJean commented Mar 19, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced createElement support for dynamic component creation in React applications
    • Added DynamicTag helper component enabling flexible element rendering with custom styling
    • Enhanced snapshot handling for runtime-defined components, allowing automatic registration of unknown component types
  • Bug Fixes

    • Improved error handling for missing snapshot definitions with better distinction between compiled and runtime components
  • Tests

    • Expanded test coverage for component creation across various scenarios including array children and functional components

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
Copy Markdown

changeset-bot bot commented Mar 19, 2026

🦋 Changeset detected

Latest commit: 95d3cb3

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

This PR includes changesets to release 2 packages
Name Type
@lynx-js/react Patch
@lynx-js/react-umd Patch

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
Copy Markdown
Contributor

coderabbitai bot commented Mar 19, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The pull request adds environment-dependent createElement support to @lynx-js/react by introducing conditional imports between main-thread and background implementations, implementing runtime snapshot creation for dynamically-rendered components, and handling spread-like attribute updates with proper dynamic slot allocation.

Changes

Cohort / File(s) Summary
Core createElement Environment Switching
packages/react/runtime/src/index.ts, packages/react/runtime/src/lynx-api.ts
Introduced conditional createElement binding that selects between createElementMainThread and createElementBackground based on __MAIN_THREAD__ flag; updated Preact import source for createElement in lynx-api.ts.
Runtime Snapshot & Type Detection
packages/react/runtime/src/snapshot.ts, packages/react/runtime/src/backgroundSnapshot.ts
Added createRuntimeSnapshot() and isCompiledSnapshot() functions to support dynamic snapshot creation; updated SnapshotInstance constructor to conditionally instantiate runtime snapshots for unknown types; added spreadIndex support and spread-object update logic in setAttribute method; modified BackgroundSnapshotInstance to handle missing compiled snapshots gracefully.
Snapshot Type-Only Imports Refactoring
packages/react/runtime/src/snapshot/event.ts, packages/react/runtime/src/snapshot/gesture.ts, packages/react/runtime/src/snapshot/platformInfo.ts, packages/react/runtime/src/snapshot/spread.ts, packages/react/runtime/src/snapshot/workletEvent.ts
Converted SnapshotInstance imports from runtime to type-only imports across five modules, removing runtime dependencies while preserving type safety.
Test Coverage & Snapshot Detection Updates
packages/react/runtime/__test__/createElement.test.jsx, packages/react/runtime/__test__/snapshotPatch.test.jsx
Added comprehensive test suite for createElement covering main-thread and background rendering modes; updated missing-snapshot error tests to use compiled snapshot prefix __snapshot_.
Example & Changeset
examples/react/src/App.tsx, .changeset/tiny-doors-think.md
Added DynamicTag helper component demonstrating dynamic tag rendering with createElement; created patch-level changeset documenting createElement feature support.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • lynx-family/lynx-stack#1736 — Implements parallel runtime snapshot creation and compiled/runtime type distinction logic that underpins this PR's dynamic snapshot handling.
  • lynx-family/lynx-stack#2319 — Adjusts Preact import sources and createElement resolution, directly intersecting with the conditional createElement binding introduced here.
  • lynx-family/lynx-stack#1455 — Modifies BackgroundSnapshotInstance behavior for missing/unknown types, closely aligned with the runtime snapshot fallback logic in this PR.

Suggested reviewers

  • hzy
  • Yradex
  • colinaaa

Poem

🐰 Hop hop, createElement spreads its wing,
Dynamic snapshots dance and sing,
Runtime types and compiled align,
Spread attributes render just fine!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'feat: support createElement' clearly and concisely describes the main feature addition across multiple files in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch p/createElement

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 19, 2026

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!

@HuJean HuJean force-pushed the p/createElement branch 4 times, most recently from f56b8a4 to d67f3ce Compare March 20, 2026 09:26
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 20, 2026

Merging this PR will degrade performance by 24.89%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 6 regressed benchmarks
✅ 56 untouched benchmarks
⏩ 21 skipped benchmarks1

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

Performance Changes

Benchmark BASE HEAD Efficiency
003-hello-list/main-thread.js_LoadScript 1.7 ms 1.8 ms -7.17%
006-static-raw-text/main-thread.js_LoadScript 408.2 µs 447.5 µs -8.78%
003-hello-list/background.js_LoadScript 1.7 ms 2 ms -14.2%
002-hello-reactLynx/main-thread.js_LoadScript 409.4 µs 449.1 µs -8.83%
002-hello-reactLynx-destroyBackground 931.4 µs 680.5 µs +36.88%
basic-performance-div-1000 27.3 ms 36.3 ms -24.89%
transform 1000 view elements 40.4 ms 43.5 ms -7.21%

Comparing p/createElement (95d3cb3) with main (36b38eb)

Open in CodSpeed

Footnotes

  1. 21 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@relativeci
Copy link
Copy Markdown

relativeci bot commented Mar 20, 2026

Web Explorer

#8421 Bundle Size — 724.73KiB (0%).

95d3cb3(current) vs 36b38eb main#8397(baseline)

Bundle metrics  no changes
                 Current
#8421
     Baseline
#8397
No change  Initial JS 41.99KiB 41.99KiB
No change  Initial CSS 1.99KiB 1.99KiB
No change  Cache Invalidation 0% 0%
No change  Chunks 8 8
No change  Assets 10 10
No change  Modules 149 149
No change  Duplicate Modules 11 11
No change  Duplicate Code 34.75% 34.75%
No change  Packages 3 3
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#8421
     Baseline
#8397
No change  Other 382.38KiB 382.38KiB
No change  JS 340.36KiB 340.36KiB
No change  CSS 1.99KiB 1.99KiB

Bundle analysis reportBranch p/createElementProject dashboard


Generated by RelativeCIDocumentationReport issue

@HuJean HuJean marked this pull request as ready for review March 20, 2026 10:56
@HuJean HuJean requested review from Yradex and hzy as code owners March 20, 2026 10:56
@HuJean HuJean marked this pull request as draft March 20, 2026 10:56
Copy link
Copy Markdown
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: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/react/package.json`:
- Line 177: The dependency entry for "preact" in packages/react/package.json
currently points to a PR-preview URL; replace that URL with a stable, published
npm version (e.g., a specific semver like "preact": "10.x.x") in the
dependencies of the `@lynx-js/react` package, ensure the version is pinned (no
preview URL), update the lockfile (npm/yarn/pnpm) and run install to validate,
and verify that any import or build referencing the "preact" dependency
(package.json "preact" entry) still resolves correctly for CI and downstream
consumers.

In `@packages/react/runtime/__test__/createElement.test.jsx`:
- Around line 37-60: The tests never exercise the background createElement
branch because they build `element` with createElement before calling
`globalEnvManager.switchToBackground()`, then reuse that object in the
background assertions; modify the tests to call
`globalEnvManager.switchToBackground()` (and any needed setup) before
constructing the element so the background `createElement` path in
packages/react/runtime/src/index.ts (the `preact/compat` background export) is
invoked — specifically update the block that currently does
`globalEnvManager.switchToBackground(); root.render(element, __root);` to
instead switch to background first and then call `createElement(...)` to build a
fresh element for `root.render`, and apply the same change to the second
affected block (the lines around 81-110).

In `@packages/react/runtime/src/snapshot.ts`:
- Around line 709-720: Remove the development console logging in the dynamic
snapshot update path: delete the console.log('update spread', oldSpread,
this.__values[0]); statement inside the block that handles dynamic snapshots in
snapshot.ts (the block using this.__snapshot_def.isDynamic, this.__values,
oldSpread and calling this.__snapshot_def.update![0]!(this, 0, oldSpread)).
Ensure no other console.log calls remain in that dynamic-prop update code so
runtime updates do not emit user payloads to the console.
- Around line 322-325: The current isBuiltInSnapshot only checks
startsWith(DEFAULT_SNAPSHOT) and misses entry-qualified or URL-prefixed ids like
"entryName:__snapshot_xxx" or "https://...:__snapshot_xxx"; update
isBuiltInSnapshot to detect __snapshot when it appears at the start or
immediately after a colon or slash (e.g. use a regex like /(^|[:/])__snapshot/
or test for type === DEFAULT_SNAPSHOT || type.includes(':__snapshot') ||
type.includes('/__snapshot')) so createSnapshot-registered ids are treated as
built-ins; change the implementation of isBuiltInSnapshot (and the similar check
around lines 349-357) accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5f189423-a127-4b67-bd90-971fadec457e

📥 Commits

Reviewing files that changed from the base of the PR and between 402ec2b and d67f3ce.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (11)
  • examples/react/src/App.tsx
  • packages/react/package.json
  • packages/react/runtime/__test__/createElement.test.jsx
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/index.ts
  • packages/react/runtime/src/snapshot.ts
  • packages/react/runtime/src/snapshot/event.ts
  • packages/react/runtime/src/snapshot/gesture.ts
  • packages/react/runtime/src/snapshot/platformInfo.ts
  • packages/react/runtime/src/snapshot/spread.ts
  • packages/react/runtime/src/snapshot/workletEvent.ts

Comment thread packages/react/package.json Outdated
Comment thread packages/react/runtime/__test__/createElement.test.jsx Outdated
Comment thread packages/react/runtime/src/snapshot.ts Outdated
Comment thread packages/react/runtime/src/snapshot.ts Outdated
@HuJean HuJean force-pushed the p/createElement branch 2 times, most recently from 35779b9 to 70ef122 Compare March 25, 2026 07:59
@HuJean HuJean marked this pull request as ready for review March 25, 2026 07:59
Copy link
Copy Markdown
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 (1)
packages/react/runtime/src/snapshot.ts (1)

323-325: ⚠️ Potential issue | 🔴 Critical

Keep entry-qualified __snapshot ids on the compiled-snapshot path.

Line 325 still only recognizes ids that start with __snapshot. Values like entryName:__snapshot_xxx or https://...:__snapshot_xxx will miss this check and fall through to createRuntimeSnapshot(), which hides missing lazy-bundle registrations instead of failing fast.

🐛 Proposed fix
-export const isCompiledSnapshot: (type: string) => boolean = (type: string) => type.startsWith(COMPILED_SNAPSHOT);
+export const isCompiledSnapshot = (type: string): boolean =>
+  type.slice(type.lastIndexOf(':') + 1).startsWith(COMPILED_SNAPSHOT);

Also applies to: 350-359

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/snapshot.ts` around lines 323 - 325, The
isCompiledSnapshot check only matches strings that start with "__snapshot" so
entry-qualified ids like "entry:__snapshot_xxx" are missed; update the
isCompiledSnapshot implementation to return true not only when
type.startsWith(COMPILED_SNAPSHOT) but also when type === COMPILED_SNAPSHOT or
type.includes(':' + COMPILED_SNAPSHOT) (so "entry:__snapshot_xxx" and URLs with
":__snapshot_xxx" are recognized), and make the same change to any duplicate
checks around the createRuntimeSnapshot usage (the logic referenced in the block
at ~350-359) so entry-qualified compiled snapshot ids short-circuit correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/react/runtime/src/snapshot.ts`:
- Around line 130-150: createRuntimeSnapshot's create(snapshotInstance) returns
element(s) cached by type but never applies CSS scoping, so runtime snapshots
are reused unscoped across entries; update the create function registered in
snapshotManager.values.set to detect snapshotInstance.cssId or
snapshotInstance.entryName and call __SetCSSId (or the existing scoping helper)
on the created node(s) before returning them (handle both the List path via
snapshotCreateList and the single-node path via __CreateElement) so each
snapshot instance gets its cssId/entryName applied even though snapshots are
cached by type.

---

Duplicate comments:
In `@packages/react/runtime/src/snapshot.ts`:
- Around line 323-325: The isCompiledSnapshot check only matches strings that
start with "__snapshot" so entry-qualified ids like "entry:__snapshot_xxx" are
missed; update the isCompiledSnapshot implementation to return true not only
when type.startsWith(COMPILED_SNAPSHOT) but also when type === COMPILED_SNAPSHOT
or type.includes(':' + COMPILED_SNAPSHOT) (so "entry:__snapshot_xxx" and URLs
with ":__snapshot_xxx" are recognized), and make the same change to any
duplicate checks around the createRuntimeSnapshot usage (the logic referenced in
the block at ~350-359) so entry-qualified compiled snapshot ids short-circuit
correctly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e0ad017c-7153-48f4-bc0a-4826ca198f65

📥 Commits

Reviewing files that changed from the base of the PR and between d67f3ce and 70ef122.

📒 Files selected for processing (15)
  • .changeset/tiny-doors-think.md
  • examples/react/src/App.tsx
  • packages/react/runtime/__test__/createElement.test.jsx
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/compat/element.ts
  • packages/react/runtime/src/index.ts
  • packages/react/runtime/src/internal.ts
  • packages/react/runtime/src/lynx-api.ts
  • packages/react/runtime/src/lynx/suspense.ts
  • packages/react/runtime/src/snapshot.ts
  • packages/react/runtime/src/snapshot/event.ts
  • packages/react/runtime/src/snapshot/gesture.ts
  • packages/react/runtime/src/snapshot/platformInfo.ts
  • packages/react/runtime/src/snapshot/spread.ts
  • packages/react/runtime/src/snapshot/workletEvent.ts
✅ Files skipped from review due to trivial changes (7)
  • .changeset/tiny-doors-think.md
  • packages/react/runtime/src/snapshot/platformInfo.ts
  • packages/react/runtime/src/snapshot/gesture.ts
  • packages/react/runtime/src/snapshot/workletEvent.ts
  • packages/react/runtime/src/snapshot/spread.ts
  • examples/react/src/App.tsx
  • packages/react/runtime/src/snapshot/event.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/test/createElement.test.jsx
  • packages/react/runtime/src/index.ts

Comment thread packages/react/runtime/src/snapshot.ts
@HuJean HuJean force-pushed the p/createElement branch 2 times, most recently from 57aa765 to 3ca0cc9 Compare March 25, 2026 10:18
Copy link
Copy Markdown
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.

♻️ Duplicate comments (1)
packages/react/runtime/src/snapshot.ts (1)

323-325: ⚠️ Potential issue | 🟠 Major

Entry-qualified snapshot IDs not detected as compiled snapshots.

The isCompiledSnapshot check uses startsWith('__snapshot') which won't match entry-qualified IDs like entryName:__snapshot_xxx or URL-prefixed forms like https://example.com/main.lynx.bundle:__snapshot_xxx. These IDs would incorrectly fall through to createRuntimeSnapshot instead of throwing an error for missing compiled snapshots.

🐛 Proposed fix
-export const isCompiledSnapshot: (type: string) => boolean = (type: string) => type.startsWith(COMPILED_SNAPSHOT);
+export const isCompiledSnapshot = (type: string): boolean => {
+  const lastColonIndex = type.lastIndexOf(':');
+  const suffix = lastColonIndex === -1 ? type : type.slice(lastColonIndex + 1);
+  return suffix.startsWith(COMPILED_SNAPSHOT);
+};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/snapshot.ts` around lines 323 - 325, The current
isCompiledSnapshot implementation only checks type.startsWith(COMPILED_SNAPSHOT)
so entry-qualified or URL-prefixed IDs like "entry:__snapshot_xxx" or
"https://...:__snapshot_xxx" are missed; update isCompiledSnapshot (and keep
COMPILED_SNAPSHOT) to detect compiled snapshots anywhere in the identifier (e.g.
use type.includes(COMPILED_SNAPSHOT) or check for
`:${COMPILED_SNAPSHOT}`/endsWith cases) so those IDs are recognized as compiled
snapshots and do not fall through to createRuntimeSnapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/react/runtime/src/snapshot.ts`:
- Around line 323-325: The current isCompiledSnapshot implementation only checks
type.startsWith(COMPILED_SNAPSHOT) so entry-qualified or URL-prefixed IDs like
"entry:__snapshot_xxx" or "https://...:__snapshot_xxx" are missed; update
isCompiledSnapshot (and keep COMPILED_SNAPSHOT) to detect compiled snapshots
anywhere in the identifier (e.g. use type.includes(COMPILED_SNAPSHOT) or check
for `:${COMPILED_SNAPSHOT}`/endsWith cases) so those IDs are recognized as
compiled snapshots and do not fall through to createRuntimeSnapshot.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 65fec9d4-f60d-4618-9b8f-237347108e8d

📥 Commits

Reviewing files that changed from the base of the PR and between 57aa765 and 3ca0cc9.

📒 Files selected for processing (13)
  • .changeset/tiny-doors-think.md
  • examples/react/src/App.tsx
  • packages/react/runtime/__test__/createElement.test.jsx
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/index.ts
  • packages/react/runtime/src/lynx-api.ts
  • packages/react/runtime/src/lynx/suspense.ts
  • packages/react/runtime/src/snapshot.ts
  • packages/react/runtime/src/snapshot/event.ts
  • packages/react/runtime/src/snapshot/gesture.ts
  • packages/react/runtime/src/snapshot/platformInfo.ts
  • packages/react/runtime/src/snapshot/spread.ts
  • packages/react/runtime/src/snapshot/workletEvent.ts
✅ Files skipped from review due to trivial changes (8)
  • .changeset/tiny-doors-think.md
  • packages/react/runtime/src/snapshot/spread.ts
  • packages/react/runtime/src/snapshot/gesture.ts
  • packages/react/runtime/src/snapshot/platformInfo.ts
  • packages/react/runtime/src/lynx/suspense.ts
  • packages/react/runtime/src/snapshot/event.ts
  • packages/react/runtime/src/snapshot/workletEvent.ts
  • packages/react/runtime/src/lynx-api.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/index.ts
  • packages/react/runtime/test/createElement.test.jsx

Copy link
Copy Markdown
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.

♻️ Duplicate comments (2)
packages/react/runtime/src/snapshot.ts (2)

130-150: ⚠️ Potential issue | 🟠 Major

Runtime createElement() snapshots still bypass CSS scoping.

These definitions are cached only by raw type, and they never record cssId / entryName. ensureElements() only calls __SetCSSId(...) from those fields, so runtime-created nodes from lazy bundles stay unscoped, and the same cached definition is reused across entries for the same tag.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/snapshot.ts` around lines 130 - 150,
createRuntimeSnapshot currently caches snapshot definitions by raw type and
never records or applies CSS scoping info, so runtime-created nodes bypass
__SetCSSId and get reused across entries; fix by recording css scoping
identifiers on the cached descriptor and ensuring create() applies them (or
change the cache key to include cssId/entryName) so each snapshot definition
includes the cssId/entryName and calls __SetCSSId(cssId, entryName) (or
otherwise applies scoping) before returning the created node; update the entry
stored in snapshotManager.values for the given type within createRuntimeSnapshot
(the object with create, update, slot, isListHolder, spreadIndex) to carry and
use css metadata so lazy/runtime bundles get properly scoped.

323-325: ⚠️ Potential issue | 🔴 Critical

Treat entry-qualified __snapshot ids as compiled snapshots.

startsWith('__snapshot') only catches bare ids. A missing lazy-bundle snapshot like entryName:__snapshot_xxx or https://example.com/main.lynx.bundle:__snapshot_xxx falls through to createRuntimeSnapshot(type) here, so a real registration failure gets silently converted into a bogus runtime element instead of throwing. The updated test only covers the bare-prefix form, so this path is still unguarded.

🐛 Proposed fix
 const DEFAULT_ENTRY_NAME = '__Card__';
 const DEFAULT_CSS_ID = 0;
 const COMPILED_SNAPSHOT = '__snapshot';
 
-export const isCompiledSnapshot: (type: string) => boolean = (type: string) => type.startsWith(COMPILED_SNAPSHOT);
+export const isCompiledSnapshot = (type: string): boolean =>
+  type.slice(type.lastIndexOf(':') + 1).startsWith(COMPILED_SNAPSHOT);

Also applies to: 350-358

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/react/runtime/src/snapshot.ts` around lines 323 - 325,
isCompiledSnapshot currently only checks type.startsWith(COMPILED_SNAPSHOT) so
entry-qualified ids like "entry:__snapshot_xxx" are missed; update the
isCompiledSnapshot implementation (and the same logic used around lines
referenced near createRuntimeSnapshot) to return true when any colon-separated
segment begins with COMPILED_SNAPSHOT (e.g., split type by ':' and check
segment.startsWith(COMPILED_SNAPSHOT) or use a regex that matches
(^|:)__snapshot). Reference COMPILED_SNAPSHOT and isCompiledSnapshot when making
the change so both bare and entry-qualified snapshot ids are treated as compiled
snapshots.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@packages/react/runtime/src/snapshot.ts`:
- Around line 130-150: createRuntimeSnapshot currently caches snapshot
definitions by raw type and never records or applies CSS scoping info, so
runtime-created nodes bypass __SetCSSId and get reused across entries; fix by
recording css scoping identifiers on the cached descriptor and ensuring create()
applies them (or change the cache key to include cssId/entryName) so each
snapshot definition includes the cssId/entryName and calls __SetCSSId(cssId,
entryName) (or otherwise applies scoping) before returning the created node;
update the entry stored in snapshotManager.values for the given type within
createRuntimeSnapshot (the object with create, update, slot, isListHolder,
spreadIndex) to carry and use css metadata so lazy/runtime bundles get properly
scoped.
- Around line 323-325: isCompiledSnapshot currently only checks
type.startsWith(COMPILED_SNAPSHOT) so entry-qualified ids like
"entry:__snapshot_xxx" are missed; update the isCompiledSnapshot implementation
(and the same logic used around lines referenced near createRuntimeSnapshot) to
return true when any colon-separated segment begins with COMPILED_SNAPSHOT
(e.g., split type by ':' and check segment.startsWith(COMPILED_SNAPSHOT) or use
a regex that matches (^|:)__snapshot). Reference COMPILED_SNAPSHOT and
isCompiledSnapshot when making the change so both bare and entry-qualified
snapshot ids are treated as compiled snapshots.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e11260a5-9c08-4b22-a117-13d0411b93ae

📥 Commits

Reviewing files that changed from the base of the PR and between 3ca0cc9 and 6a890e5.

📒 Files selected for processing (13)
  • .changeset/tiny-doors-think.md
  • examples/react/src/App.tsx
  • packages/react/runtime/__test__/createElement.test.jsx
  • packages/react/runtime/__test__/snapshotPatch.test.jsx
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/index.ts
  • packages/react/runtime/src/lynx-api.ts
  • packages/react/runtime/src/snapshot.ts
  • packages/react/runtime/src/snapshot/event.ts
  • packages/react/runtime/src/snapshot/gesture.ts
  • packages/react/runtime/src/snapshot/platformInfo.ts
  • packages/react/runtime/src/snapshot/spread.ts
  • packages/react/runtime/src/snapshot/workletEvent.ts
✅ Files skipped from review due to trivial changes (6)
  • .changeset/tiny-doors-think.md
  • packages/react/runtime/src/snapshot/spread.ts
  • packages/react/runtime/src/snapshot/workletEvent.ts
  • packages/react/runtime/src/snapshot/platformInfo.ts
  • packages/react/runtime/src/snapshot/event.ts
  • packages/react/runtime/test/createElement.test.jsx
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/react/runtime/src/snapshot/gesture.ts
  • packages/react/runtime/src/lynx-api.ts
  • packages/react/runtime/src/backgroundSnapshot.ts
  • packages/react/runtime/src/index.ts

@HuJean HuJean force-pushed the p/createElement branch 5 times, most recently from b4bf87a to 976b3df Compare March 26, 2026 02:31
]),
};

export function createRuntimeSnapshot(type: string): void {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This runtime snapshot definition does not carry any CSS scope metadata (cssId / entryName). SnapshotInstance.ensureElements() only calls __SetCSSId(...) when those fields are present, so dynamically created host nodes will diverge from compiled JSX nodes in scoped-CSS bundles: the element is created, but its scoped class rules never match. Can we propagate the current bundle scope into runtime snapshots as well?

const DEFAULT_CSS_ID = 0;
const COMPILED_SNAPSHOT = '__snapshot';

export const isCompiledSnapshot: (type: string) => boolean = (type: string) => type.startsWith(COMPILED_SNAPSHOT);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think startsWith("__snapshot") is too narrow here. This codebase already documents compiled snapshot ids like entryName:__snapshot_xxx and https://example.com/main.lynx.bundle:__snapshot_xxx for lazy / standalone bundles. Those would now be treated as runtime tags, so a missing compiled snapshot would silently fall back to createRuntimeSnapshot(type) instead of surfacing the expected missing-snapshot error. It would be safer to detect __snapshot after the optional prefix rather than only at the beginning of the whole string.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think can add a key of snapshot rather than using type only

const oldValue = (this.__extraProps ??= {})[key];
this.__extraProps[key] = value;
// dynamic snapshot should update __extraProps to element
if (this.__snapshot_def.spreadIndex !== undefined && !isDirectOrDeepEqual(oldValue, value)) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This new main-thread branch now consumes string-key props through spreadIndex, but the background path still does not mirror that conversion. BackgroundSnapshotInstance.setAttribute(key, value) continues to push raw string-key values into snapshotPatch, bypassing setAttributeImpl / transformSpread. That means event handlers, refs and other non-JSON-safe props on createElement()-generated host nodes will still be serialized incorrectly during hydration/update (for example functions become null once the patch is JSON-serialized). Could we make the background string-key path produce the same spread payload that this branch expects?

</view>
<text className='Title'>React</text>
<text className='Subtitle'>on Lynx</text>
<DynamicTag
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I’d suggest keeping the React example as simple and declarative as possible. This PR is about adding createElement support, but the main example also serves as the facade demo for new users. Replacing straightforward JSX with DynamicTag here makes the entry example harder to scan and may unintentionally suggest that this indirection is the preferred style. It would be clearer to keep the main demo in plain JSX and showcase createElement in a separate focused example or a small isolated section.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

get

}

export const __DynamicPartChildren_0: [DynamicPartType, number][] = [[DynamicPartType.Children, 0]];
export const __DynamicPartListChildren_0: [DynamicPartType, number][] = [[DynamicPartType.ListChildren, 0]];
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since we add this, we can simple snapshot generation by swc.

Change:

[
ReactLynx.__DynamicPartListChildren,
0
]

to something like this:

}, null, ReactLynx.__DynamicPartChildren_0, undefined, globDynamicComponentEntry, null, true);

We can do this in future PRs.


export function createRuntimeSnapshot(type: string): void {
const isListHolder = type === 'list';
snapshotManager.values.set(type, {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe we should only set once. We should add if (!snapshotManager.values.has(type)) guard here.

...oldSpread,
__spread: true,
[key]: value as unknown,
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we need ...oldSpread. Can we add a test to show attribute remove is okay here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants