fix(web): \_\_ElementFromBinary should mark all elements actively #1484

Sherry-hue · 2025-08-11T07:29:21Z

Summary by CodeRabbit

Bug Fixes
- Mark template elements and their parts consistently after cloning to prevent intermittent UI inconsistencies.
Tests
- Added unit tests verifying template part elements are correctly identified.
Chores
- Added release metadata for a patch update documenting the fix.

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 · 2025-08-11T07:29:25Z

🦋 Changeset detected

Latest commit: 7e8cb40

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	Patch
@lynx-js/web-core-server	Patch
@lynx-js/web-core	Patch
@lynx-js/web-worker-runtime	Patch
@lynx-js/web-constants	Patch
@lynx-js/web-worker-rpc	Patch
@lynx-js/web-style-transformer	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 · 2025-08-11T07:29:28Z

📝 Walkthrough

Walkthrough

Adds a changeset and tests; updates element-template processing to call __MarkPartElement when a builtin dirtyID equals the element id and to call __MarkTemplateElement for each cloned element after event application. No public API/type changes.

Changes

Cohort / File(s)	Summary of Changes
Release metadata `.changeset/silly-rockets-bet.md`	Adds a changeset bump for `@lynx-js/web-mainthread-apis` (patch) documenting a fix: "__ElementFromBinary should mark all elements actively."
Main-thread element processing `packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts`	In `createElementForElementTemplateData`, when a builtin attribute key is `dirtyID` and equals `data.id`, call `__MarkPartElement(element, value)`. In `__ElementFromBinary`, after applying events to `clonedElements`, call `__MarkTemplateElement` for each cloned element (post-processing `forEach`). No API/signature changes.
Tests & fixtures `packages/web-platform/web-tests/resources/web-core.main-thread.json`, `packages/web-platform/web-tests/tests/main-thread-apis.test.ts`	Fixture: a text node gains `builtinAttributes: { "dirtyID": "id-2" }`. Adds unit test "should mark part element" asserting the template parts map contains the part keyed by `id-2` for the cloned element.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

feat: add support for __MarkTemplateElement, __MarkPartElement, and __GetTemplateParts in all-on-ui #1275: Introduced/wired __MarkTemplateElement, __MarkPartElement, and template-parts APIs that this change invokes.
feat: move SSR hydrate essential info to the ssr attribute #1292: Modifies the same createMainThreadGlobalThis.ts area to add template/part-marking side-effects — overlapping edits.
feat: implement __ElementFromBinary #1391: Directly modifies __ElementFromBinary logic and element-marking behavior — strong overlap.

Suggested reviewers

PupilTong
colinaaa

Poem

I hop through cloned branches, whiskers bright,
I sniff a dirtyID and mark it right.
I tag the part, then mark the whole array,
Every cloned leaf shines in tidy play. 🐇🥕

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa6acda and 7e8cb40.

📒 Files selected for processing (4)

.changeset/silly-rockets-bet.md (1 hunks)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (2 hunks)
packages/web-platform/web-tests/resources/web-core.main-thread.json (1 hunks)
packages/web-platform/web-tests/tests/main-thread-apis.test.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

.changeset/silly-rockets-bet.md
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
packages/web-platform/web-tests/resources/web-core.main-thread.json

🔇 Additional comments (1)

packages/web-platform/web-tests/tests/main-thread-apis.test.ts (1)

1334-1336: Good: multi-thread skip guard added to avoid flakiness

Placing test.skip(ENABLE_MULTI_THREAD, 'NYI for multi-thread') at the top of the test aligns with the suite’s pattern and prevents flaky runs under multi-thread mode.

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

.changeset/silly-rockets-bet.md (1)

5-5: Nit: Make the release note a touch more descriptive.

Consider clarifying the impact, e.g., “Ensure elements produced by __ElementFromBinary are marked for template handling, fixing cases where downstream logic depended on the marker.”
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (1)
760-761: Use forEach instead of map for side effects.

map allocates an unused array. forEach communicates intent and avoids the waste.

Apply:
-      clonedElements.map(__MarkTemplateElement);
+      clonedElements.forEach(__MarkTemplateElement);

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05b0096 and bbe5138.

📒 Files selected for processing (2)

.changeset/silly-rockets-bet.md (1 hunks)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 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:

.changeset/silly-rockets-bet.md

📚 Learning: 2025-07-22T09:26:16.722Z

Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.

Applied to files:

.changeset/silly-rockets-bet.md

🧬 Code Graph Analysis (1)

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

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

__MarkTemplateElement (354-358)

🔇 Additional comments (1)

.changeset/silly-rockets-bet.md (1)

1-5: Changeset is appropriate for a user-visible bugfix (patch).

Targets the correct package and succinctly documents the fix. Good.

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts

codspeed-hq · 2025-08-11T07:42:57Z

CodSpeed Performance Report

Merging #1484 will not alter performance

_{Comparing Sherry-hue:fix/element-binary-mark (7e8cb40) with main (517ac33)}

Summary

✅ 10 untouched benchmarks

relativeci · 2025-08-11T07:44:50Z

React Example

#4099 Bundle Size — 236.88KiB (0%).

7e8cb40(current) vs 517ac33 main#4069(baseline)

Bundle metrics no changes

	Current #4099	Baseline #4069
Initial JS	`0B`	`0B`
Initial CSS	`0B`	`0B`
Cache Invalidation	`0%`	`0%`
Chunks	`0`	`0`
Assets	`4`	`4`
Modules	`158`	`158`
Duplicate Modules	`64`	`64`
Duplicate Code	`45.85%`	`45.85%`
Packages	`2`	`2`
Duplicate Packages	`0`	`0`

Bundle size by type no changes

	Current #4099	Baseline #4069
IMG	`145.76KiB`	`145.76KiB`
Other	`91.12KiB`	`91.12KiB`

Bundle analysis report Branch Sherry-hue:fix/element-binary-ma... Project dashboard

^{Generated by RelativeCI Documentation Report issue}

relativeci · 2025-08-11T07:44:51Z

Web Explorer

#4095 Bundle Size — 344.15KiB (+0.01%).

7e8cb40(current) vs 517ac33 main#4064(baseline)

Bundle metrics

1 change

	Current #4095	Baseline #4064
Initial JS	`143.27KiB`	`143.27KiB`
Initial CSS	`31.84KiB`	`31.84KiB`
Cache Invalidation	`6.72%`	`0%`
Chunks	`7`	`7`
Assets	`7`	`7`
Modules	`211`	`211`
Duplicate Modules	`17`	`17`
Duplicate Code	`3.96%`	`3.96%`
Packages	`4`	`4`
Duplicate Packages	`0`	`0`

Bundle size by type

1 change

1 regression

	Current #4095	Baseline #4064
JS	`229.35KiB` (`+0.02%`)	`229.31KiB`
Other	`82.95KiB`	`82.95KiB`
CSS	`31.84KiB`	`31.84KiB`

Bundle analysis report Branch Sherry-hue:fix/element-binary-ma... Project dashboard

^{Generated by RelativeCI Documentation Report issue}

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/web-platform/web-tests/tests/main-thread-apis.test.ts (1)

1333-1349: Consider adding a negative case for robustness

Add a companion test ensuring that when builtinAttributes.dirtyID does not match the element id, the element is not marked as a part. This guards against accidental over-marking in the template processing logic.

If you want, I can draft the fixture tweak and the corresponding test that loads a similar template where dirtyID ≠ id and asserts that the parts map remains empty.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 324ea1c and 61dc218.

📒 Files selected for processing (4)

.changeset/silly-rockets-bet.md (1 hunks)
packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts (2 hunks)
packages/web-platform/web-tests/resources/web-core.main-thread.json (1 hunks)
packages/web-platform/web-tests/tests/main-thread-apis.test.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts
.changeset/silly-rockets-bet.md

🔇 Additional comments (1)

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

53-55: LGTM: Dirty part marking fixture looks correct

Assigning builtinAttributes.dirtyID to "id-2" on the text node aligns with the intended behavior to mark that element as a part via __MarkPartElement and enables the new test to validate the parts map.

packages/web-platform/web-tests/tests/main-thread-apis.test.ts

codecov · 2025-08-12T09:46:55Z

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!

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/[email protected] ### Patch Changes - fix css transform error in testing library ([#1500](#1500)) - fix the type error of `wrapper` option in testing library's `render` and `renderHook` function ([#1502](#1502)) - Introduce recursive hydration for lists to prevent double remove/insert on moves. ([#1401](#1401)) - Handle `<frame/>` correctly. ([#1497](#1497)) ## @lynx-js/[email protected] ### Patch Changes - `output.inlineScripts` defaults to `false` when chunkSplit strategy is not `'all-in-one'` ([#1504](#1504)) ## @lynx-js/[email protected] ### Patch Changes - `output.inlineScripts` defaults to `false` when chunkSplit strategy is not `'all-in-one'` ([#1504](#1504)) - Updated dependencies \[[`51a0b19`](51a0b19), [`b391ef5`](b391ef5)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix that `lynxTestingEnv.jsdom` cannot be initialized correctly when `global.jsdom` is not defined. ([#1422](#1422)) ## @lynx-js/[email protected] ### Patch Changes - fix: systeminfo in mts function ([#1537](#1537)) - feat: add MTS API: \_\_UpdateComponentInfo ([#1485](#1485)) - fix: `__ElementFromBinary` needs to correctly apply the dataset in elementTemplate to the Element ([#1487](#1487)) - fix: all attributes except `id` and `type` under ElementTemplateData are optional. ([#1483](#1483)) - feat: add MTS API \_\_GetAttributeByName ([#1486](#1486)) - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - fix: systeminfo in mts function ([#1537](#1537)) - refactor: use utf-8 string ([#1473](#1473)) - Fix mtsGlobalThis race condition in createRenderAllOnUI ([#1506](#1506)) - Updated dependencies \[[`405a917`](405a917), [`b8f89e2`](b8f89e2), [`f76aae9`](f76aae9), [`b8b060b`](b8b060b), [`d8381a5`](d8381a5), [`214898b`](214898b), [`ab8cee4`](ab8cee4)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: use utf-8 string ([#1473](#1473)) ## @lynx-js/[email protected] ### Patch Changes - fix: systeminfo in mts function ([#1537](#1537)) - refactor: use utf-8 string ([#1473](#1473)) - feat: add MTS API: \_\_UpdateComponentInfo ([#1485](#1485)) - fix: \_\_ElementFromBinary should mark all elements actively ([#1484](#1484)) - fix: `__ElementFromBinary` needs to correctly apply the dataset in elementTemplate to the Element ([#1487](#1487)) - fix: all attributes except `id` and `type` under ElementTemplateData are optional. ([#1483](#1483)) - feat: add MTS API \_\_GetAttributeByName ([#1486](#1486)) - Updated dependencies \[[`405a917`](405a917), [`b8f89e2`](b8f89e2), [`f76aae9`](f76aae9), [`d8381a5`](d8381a5), [`214898b`](214898b), [`ab8cee4`](ab8cee4)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - refactor: use utf-8 string ([#1473](#1473)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`405a917`](405a917), [`b8f89e2`](b8f89e2), [`f76aae9`](f76aae9), [`b8b060b`](b8b060b), [`d8381a5`](d8381a5), [`214898b`](214898b), [`ab8cee4`](ab8cee4)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix "emit different content to the same filename" error ([#1482](#1482)) ## @lynx-js/[email protected] ### Patch Changes - Fix invalid `module.exports=;` syntax in app-service.js. ([#1501](#1501)) ## [email protected] ## @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Sherry-hue self-assigned this Aug 11, 2025

Sherry-hue requested a review from PupilTong as a code owner August 11, 2025 07:29

Sherry-hue added the platform:Web label Aug 11, 2025

Sherry-hue added this to Lynx for Web Aug 11, 2025

PupilTong previously approved these changes Aug 11, 2025

View reviewed changes

Sherry-hue enabled auto-merge (squash) August 11, 2025 07:36

coderabbitai bot reviewed Aug 11, 2025

View reviewed changes

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts Outdated Show resolved Hide resolved

Sherry-hue dismissed PupilTong’s stale review via 324ea1c August 11, 2025 09:37

Sherry-hue force-pushed the fix/element-binary-mark branch from bbe5138 to 324ea1c Compare August 11, 2025 09:37

PupilTong reviewed Aug 11, 2025

View reviewed changes

packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts Show resolved Hide resolved

Sherry-hue force-pushed the fix/element-binary-mark branch from 324ea1c to 61dc218 Compare August 12, 2025 09:15

coderabbitai bot reviewed Aug 12, 2025

View reviewed changes

packages/web-platform/web-tests/tests/main-thread-apis.test.ts Show resolved Hide resolved

Sherry-hue force-pushed the fix/element-binary-mark branch from 61dc218 to fa6acda Compare August 12, 2025 09:26

fix(web): \_\_ElementFromBinary should mark all elements actively

7e8cb40

Sherry-hue force-pushed the fix/element-binary-mark branch from fa6acda to 7e8cb40 Compare August 12, 2025 11:32

PupilTong approved these changes Aug 12, 2025

View reviewed changes

Sherry-hue merged commit b8b060b into lynx-family:main Aug 12, 2025
92 of 97 checks passed

github-project-automation bot moved this to Done in Lynx for Web Aug 12, 2025

github-actions bot mentioned this pull request Aug 12, 2025

chore: Release 2025-08-15 13:07:22 #1494

Merged

coderabbitai bot mentioned this pull request Aug 12, 2025

fix(web): __ElementFromBinary needs to correctly apply the dataset in… #1487

Merged

3 tasks

fix(web): \_\_ElementFromBinary should mark all elements actively #1484

fix(web): \_\_ElementFromBinary should mark all elements actively #1484

Uh oh!

Conversation

Sherry-hue commented Aug 11, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Checklist

Uh oh!

changeset-bot bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

coderabbitai bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR/Issue comments)

Other keywords and placeholders

Status, Documentation and Community

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codspeed-hq bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #1484 will not alter performance

Summary

Uh oh!

relativeci bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

React Example

#4099 Bundle Size — 236.88KiB (0%).

Uh oh!

relativeci bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Web Explorer

#4095 Bundle Size — 344.15KiB (+0.01%).

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Sherry-hue commented Aug 11, 2025 •

edited by coderabbitai bot

Loading

changeset-bot bot commented Aug 11, 2025 •

edited

Loading

coderabbitai bot commented Aug 11, 2025 •

edited

Loading

codspeed-hq bot commented Aug 11, 2025 •

edited

Loading

relativeci bot commented Aug 11, 2025 •

edited

Loading

relativeci bot commented Aug 11, 2025 •

edited

Loading

codecov bot commented Aug 12, 2025 •

edited

Loading