fix(react-transform): preserve children prop callback scope

[colinaaa]

Summary

• keep snapshot extraction for native JSX elements scoped to real JSX children instead of revisiting opening attributes after attrs have already been processed
• add a regression test for children={array.map(...)} so JSX inside the prop expression stays inside the map callback scope
• document the snapshot extractor recursion pitfall in .github/react-transform.instructions.md

Root cause

DynamicPartExtractor manually extracted native JSX opening attributes, then called n.visit_mut_children_with(...). That SWC traversal also revisited opening attrs, so JSX nested inside a prop expression such as children={items.map(...)} could be extracted into a parent snapshot slot. The extracted slot expression then referenced map callback locals like array123 and index123 outside their lexical scope.

Fixes #2523

Validation

• pnpm turbo build --filter @lynx-js/react-transform
• cargo fmt -p swc_plugin_snapshot -- --check
• pnpm dprint check .github/react-transform.instructions.md packages/react/transform/crates/swc_plugin_snapshot/lib.rs
• cargo test -p swc_plugin_snapshot -- --nocapture
• cargo test -p react-transform

Summary by CodeRabbit

• Bug Fixes
  • Resolved a React JSX transformation bug where nested JSX expressions in children prop callbacks—particularly with array methods like map()—were incorrectly handling variable scope. This patch ensures proper variable references during code generation.

[changeset-bot]

🦋 Changeset detected

Latest commit: 50dcd1f

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

This PR includes changesets to release 4 packages

Name	Type
@lynx-js/react	Patch
@lynx-js/react-rsbuild-plugin	Patch
@lynx-js/react-umd	Patch
@lynx-js/react-alias-rsbuild-plugin	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]

📝 Walkthrough

Walkthrough

This pull request fixes a JSX transformation bug where children={array.map(...)} expressions emitted stale callback-local variable references in compiled output. The fix changes the visitor traversal strategy to visit only the children field directly, preventing lexical scope contamination with callback parameters.

Changes

Cohort / File(s)	Summary
Changelog & Documentation .changeset/fix-react-children-map-scope.md, .github/react-transform.instructions.md	Added changeset release note documenting the patch fix and updated JSX transformation guidance to clarify the proper visitor traversal strategy for DynamicPartExtractor.
Core JSX Transform Fix packages/react/transform/crates/swc_plugin_snapshot/lib.rs	Modified DynamicPartExtractor to traverse only n.children via visit_mut_with(...) instead of visit_mut_children_with(...). Added regression test verifying that JSX within map callbacks remains scoped correctly and does not leak callback-local references to parent snapshots.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

framework:React

Suggested reviewers

• gaoachao
• Yradex

Poem

🐰 A map callback once leaked its local names,
Children floating free through snapshot frames,
But now we visit only what we need,
Each scope stays pure, no stale reference freed! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(react-transform): preserve children prop callback scope' directly describes the main change: fixing the react-transform to preserve callback scope when JSX is inside children prop expressions.
Linked Issues check	✅ Passed	The PR directly addresses all coding objectives from issue #2523: prevents stale identifier references in children prop callbacks by fixing the JSX transformation logic to scope extraction to actual children rather than revisiting attributes, and includes a regression test.
Out of Scope Changes check	✅ Passed	All changes are within scope of fixing the children prop callback scope issue: the core lib.rs fix, documentation of the pitfall, changeset metadata, and a targeted regression test.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

❌ Patch coverage is 95.65217% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
.../react/transform/crates/swc_plugin_snapshot/lib.rs	95.65%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[codspeed-hq]

Merging this PR will not alter performance

✅ 81 untouched benchmarks
⏩ 26 skipped benchmarks¹

---

_{Comparing colinaaa:codex/fix-react-transform-children-map-scope (50dcd1f) with main (989f345)}

Footnotes

1. 26 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]

React External

#743 Bundle Size — 680.2KiB (0%).

50dcd1f(current) vs 989f345 main#736(baseline)

Bundle metrics  no changes

	Current #743	Baseline #736
Initial JS	0B	0B
Initial CSS	0B	0B
Cache Invalidation	0%	0%
Chunks	0	0
Assets	3	3
Modules	17	17
Duplicate Modules	5	5
Duplicate Code	8.59%	8.59%
Packages	0	0
Duplicate Packages	0	0

Bundle size by type  no changes

	Current #743	Baseline #736
Other	680.2KiB	680.2KiB

Bundle analysis report Branch colinaaa:codex/fix-react-transfo... Project dashboard

---

^{Generated by RelativeCI Documentation Report issue}

[relativeci]

React MTF Example

#759 Bundle Size — 196.47KiB (0%).

50dcd1f(current) vs 989f345 main#751(baseline)

Bundle metrics  no changes

	Current #759	Baseline #751
Initial JS	0B	0B
Initial CSS	0B	0B
Cache Invalidation	0%	0%
Chunks	0	0
Assets	3	3
Modules	173	173
Duplicate Modules	66	66
Duplicate Code	44.07%	44.07%
Packages	2	2
Duplicate Packages	0	0

Bundle size by type  no changes

	Current #759	Baseline #751
IMG	111.23KiB	111.23KiB
Other	85.24KiB	85.24KiB

Bundle analysis report Branch colinaaa:codex/fix-react-transfo... Project dashboard

---

^{Generated by RelativeCI Documentation Report issue}

[relativeci]

Web Explorer

#9200 Bundle Size — 900.02KiB (0%).

50dcd1f(current) vs 989f345 main#9192(baseline)

Bundle metrics   2 changes

	Current #9200	Baseline #9192
Initial JS	44.46KiB	44.46KiB
Initial CSS	2.22KiB	2.22KiB
Cache Invalidation	0%	0%
Chunks	9	9
Assets	11	11
Modules	229(+0.88%)	227
Duplicate Modules	11	11
Duplicate Code	27.28%(-0.04%)	27.29%
Packages	10	10
Duplicate Packages	0	0

Bundle size by type  no changes

	Current #9200	Baseline #9192
JS	495.88KiB	495.88KiB
Other	401.92KiB	401.92KiB
CSS	2.22KiB	2.22KiB

Bundle analysis report Branch colinaaa:codex/fix-react-transfo... Project dashboard

---

^{Generated by RelativeCI Documentation Report issue}

[relativeci]

React Example

#7627 Bundle Size — 225.31KiB (0%).

50dcd1f(current) vs 989f345 main#7619(baseline)

Bundle metrics  no changes

	Current #7627	Baseline #7619
Initial JS	0B	0B
Initial CSS	0B	0B
Cache Invalidation	0%	0%
Chunks	0	0
Assets	4	4
Modules	179	179
Duplicate Modules	69	69
Duplicate Code	44.57%	44.57%
Packages	2	2
Duplicate Packages	0	0

Bundle size by type  no changes

	Current #7627	Baseline #7619
IMG	145.76KiB	145.76KiB
Other	79.55KiB	79.55KiB

Bundle analysis report Branch colinaaa:codex/fix-react-transfo... Project dashboard

---

^{Generated by RelativeCI Documentation Report issue}

[coderabbitai]

🧹 Nitpick comments (1)

packages/react/transform/crates/swc_plugin_snapshot/lib.rs (1)

1730-1738: Assertions are string-fragile – consider an __swc_snapshots__ based test.

The positive assertion relies on swc's codegen emitting (item, index123, array123)=> verbatim (no space around =>), and the negative assertion matches the literal slot-prop form $0: array123[index123]. Both are tight couplings to the current printer output and to the react::react lowering shape; a future swc upgrade tweaking arrow-fn formatting or prop ordering could make this test fail without a real regression.

Given that the rest of this module uses the test! macro backed by __swc_snapshots__, converting this regression case to a snapshot test (and checking the snapshot for absence of array123/index123 references in the createSnapshot dynamic parts) would be more robust. Optional – keep as-is if you prefer the explicit behavioral asserts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react/transform/crates/swc_plugin_snapshot/lib.rs` around lines 1730
- 1738, Replace the fragile string assertions around the generated code (the two
assert! checks that inspect output for the exact "(item, index123, array123)=>"
and for the literal "$0: array123[index123]") with a snapshot-based test using
the existing test! / __swc_snapshots__ harness used elsewhere in this module:
produce a snapshot of the full transformed output (the same value currently in
output) and then in the snapshot assertion verify the snapshot contents do not
contain references to the map-local names in createSnapshot dynamic parts (i.e.,
ensure no occurrences of "array123" or "index123" leak outside the map
callback), keeping the intent but removing dependency on exact printer spacing
or prop ordering; update the test that builds output (the variable named output
and the surrounding test case) to use test! / __swc_snapshots__ instead of the
two string assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/react/transform/crates/swc_plugin_snapshot/lib.rs`:
- Around line 1730-1738: Replace the fragile string assertions around the
generated code (the two assert! checks that inspect output for the exact "(item,
index123, array123)=>" and for the literal "$0: array123[index123]") with a
snapshot-based test using the existing test! / __swc_snapshots__ harness used
elsewhere in this module: produce a snapshot of the full transformed output (the
same value currently in output) and then in the snapshot assertion verify the
snapshot contents do not contain references to the map-local names in
createSnapshot dynamic parts (i.e., ensure no occurrences of "array123" or
"index123" leak outside the map callback), keeping the intent but removing
dependency on exact printer spacing or prop ordering; update the test that
builds output (the variable named output and the surrounding test case) to use
test! / __swc_snapshots__ instead of the two string assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8920fe8d-a395-4d83-9470-02f33a5b91be

📥 Commits

Reviewing files that changed from the base of the PR and between 989f345 and 50dcd1f.

📒 Files selected for processing (3)

• .changeset/fix-react-children-map-scope.md
• .github/react-transform.instructions.md
• packages/react/transform/crates/swc_plugin_snapshot/lib.rs