-
Notifications
You must be signed in to change notification settings - Fork 110
refactor: extract swc_plugin_snapshot & swc_plugin_list
#1819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 4d44ea6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
📝 WalkthroughWalkthroughAdds two new internal crates (swc_plugin_list, swc_plugin_snapshot), wires them into react/transform, moves shared JSX helpers into swc_plugins_shared, introduces an N-API bridge module (napi) for JSXTransformer, and updates import paths across files to use swc_plugins_shared and the new napi module. Includes a trivial changeset entry. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate 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. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
React Example#5490 Bundle Size — 237.5KiB (0%).4d44ea6(current) vs 69e0dc4 main#5481(baseline) Bundle metrics
|
| Current #5490 |
Baseline #5481 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
165 |
165 |
|
67 |
67 |
|
46.76% |
46.76% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #5490 |
Baseline #5481 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.74KiB |
91.74KiB |
Bundle analysis report Branch refactor/extract-swc-plugin-snap... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5484 Bundle Size — 365.4KiB (0%).4d44ea6(current) vs 69e0dc4 main#5475(baseline) Bundle metrics
|
| Current #5484 |
Baseline #5475 |
|
|---|---|---|
145.69KiB |
145.69KiB |
|
32KiB |
32KiB |
|
0% |
0% |
|
8 |
8 |
|
8 |
8 |
|
220 |
220 |
|
16 |
16 |
|
3.37% |
3.37% |
|
4 |
4 |
|
0 |
0 |
Bundle size by type no changes
| Current #5484 |
Baseline #5475 |
|
|---|---|---|
239.39KiB |
239.39KiB |
|
94.02KiB |
94.02KiB |
|
32KiB |
32KiB |
Bundle analysis report Branch refactor/extract-swc-plugin-snap... Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1819 will degrade performances by 6.73%Comparing Summary
Benchmarks breakdown
Footnotes
|
There was a problem hiding this 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
🧹 Nitpick comments (2)
packages/react/transform/crates/swc_plugin_snapshot/lib.rs (1)
25-25: napi module exposureGood separation. Consider re-exporting the napi types at crate root to reduce consumer churn.
Example:
+pub use napi::{JSXTransformer as NapiJSXTransformer, JSXTransformerConfig as NapiJSXTransformerConfig};packages/react/transform/crates/swc_plugin_snapshot/napi.rs (1)
31-42: Reduce duplication: derive defaults from core config to avoid drift.Keep one source of truth by delegating to CoreJSXTransformerConfig::default().
-impl Default for JSXTransformerConfig { - fn default() -> Self { - Self { - preserve_jsx: false, - runtime_pkg: "@lynx-js/react".into(), - jsx_import_source: Some("@lynx-js/react".into()), - filename: Default::default(), - target: TransformTarget::LEPUS, - is_dynamic_component: Some(false), - } - } -} +impl Default for JSXTransformerConfig { + fn default() -> Self { + CoreJSXTransformerConfig::default().into() + } +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.changeset/hot-socks-thank.md(1 hunks)packages/react/transform/Cargo.toml(1 hunks)packages/react/transform/crates/swc_plugin_list/Cargo.toml(1 hunks)packages/react/transform/crates/swc_plugin_list/lib.rs(2 hunks)packages/react/transform/crates/swc_plugin_snapshot/Cargo.toml(1 hunks)packages/react/transform/crates/swc_plugin_snapshot/lib.rs(2 hunks)packages/react/transform/crates/swc_plugin_snapshot/napi.rs(1 hunks)packages/react/transform/crates/swc_plugin_snapshot/slot_marker.rs(1 hunks)packages/react/transform/crates/swc_plugins_shared/Cargo.toml(1 hunks)packages/react/transform/crates/swc_plugins_shared/lib.rs(1 hunks)packages/react/transform/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/hot-socks-thank.md
🧠 Learnings (8)
📓 Common learnings
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.407Z
Learning: In packages/react/transform/src/swc_plugin_compat/mod.rs, the `add_pure_comment` function at lines 478-482 is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs. These are two distinct code paths that should be treated differently.
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/react/transform/crates/swc_plugins_shared/lib.rspackages/react/transform/crates/swc_plugin_snapshot/slot_marker.rspackages/react/transform/src/lib.rspackages/react/transform/crates/swc_plugin_snapshot/napi.rspackages/react/transform/crates/swc_plugin_snapshot/lib.rspackages/react/transform/crates/swc_plugin_list/lib.rs
📚 Learning: 2025-09-18T04:43:54.407Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.407Z
Learning: In packages/react/transform/src/swc_plugin_compat/mod.rs, the `add_pure_comment` function at lines 478-482 is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs. These are two distinct code paths that should be treated differently.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/slot_marker.rspackages/react/transform/src/lib.rspackages/react/transform/crates/swc_plugin_snapshot/napi.rspackages/react/transform/crates/swc_plugin_snapshot/lib.rs
📚 Learning: 2025-09-18T04:43:54.407Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1771
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js:2-2
Timestamp: 2025-09-18T04:43:54.407Z
Learning: In the lynx-family/lynx-stack repository, the `add_pure_comment` function in packages/react/transform/src/swc_plugin_compat/mod.rs (around lines 478-482) is specifically for `wrapWithLynxComponent` calls, not `createSnapshot` calls. The PURE comment injection for `createSnapshot` is handled separately in swc_plugin_snapshot/mod.rs.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/slot_marker.rspackages/react/transform/src/lib.rspackages/react/transform/crates/swc_plugin_snapshot/napi.rspackages/react/transform/crates/swc_plugin_snapshot/lib.rs
📚 Learning: 2025-09-10T11:42:36.855Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
Applied to files:
packages/react/transform/crates/swc_plugin_snapshot/Cargo.tomlpackages/react/transform/crates/swc_plugin_list/Cargo.tomlpackages/react/transform/Cargo.tomlpackages/react/transform/crates/swc_plugins_shared/Cargo.tomlpackages/react/transform/src/lib.rs
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/hot-socks-thank.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/hot-socks-thank.md
📚 Learning: 2025-09-09T12:38:10.450Z
Learnt from: CR
PR: lynx-family/lynx-stack#0
File: AGENTS.md:0-0
Timestamp: 2025-09-09T12:38:10.450Z
Learning: Applies to .changeset/*.md : For contributions, always generate a changeset and commit the resulting markdown file(s)
Applied to files:
.changeset/hot-socks-thank.md
🧬 Code graph analysis (5)
packages/react/transform/crates/swc_plugins_shared/lib.rs (1)
packages/react/transform/src/swc_plugin_snapshot/mod.rs (1)
jsx_helpers(23-23)
packages/react/transform/crates/swc_plugin_snapshot/slot_marker.rs (1)
packages/react/transform/crates/swc_plugins_shared/jsx_helpers.rs (7)
jsx_children_to_expr(162-205)jsx_has_dynamic_key(219-239)jsx_is_children_full_dynamic(255-314)jsx_is_custom(207-217)jsx_is_list(241-246)jsx_name(13-74)jsx_text_to_str(130-160)
packages/react/transform/crates/swc_plugin_snapshot/napi.rs (1)
packages/react/transform/crates/swc_plugin_snapshot/lib.rs (8)
default(990-999)with_content_hash(1024-1027)new(257-272)new(1029-1051)visit_mut_jsx_element(358-944)visit_mut_jsx_element(1097-1488)visit_mut_module_items(1490-1503)visit_mut_module(1505-1538)
packages/react/transform/crates/swc_plugin_snapshot/lib.rs (2)
packages/react/transform/crates/swc_plugins_shared/css.rs (1)
get_string_inline_style_from_literal(14-64)packages/react/transform/crates/swc_plugins_shared/jsx_helpers.rs (12)
jsx_attr_name(76-81)jsx_attr_to_prop(99-110)jsx_attr_value(83-97)jsx_children_to_expr(162-205)jsx_is_children_full_dynamic(255-314)jsx_is_custom(207-217)jsx_is_list(241-246)jsx_is_list_item(248-253)jsx_name(13-74)jsx_props_to_obj(112-128)jsx_text_to_str(130-160)transform_jsx_attr_str(317-360)
packages/react/transform/crates/swc_plugin_list/lib.rs (1)
packages/react/transform/crates/swc_plugins_shared/jsx_helpers.rs (3)
jsx_attr_value(83-97)jsx_children_to_expr(162-205)jsx_is_list_item(248-253)
🔇 Additional comments (19)
.changeset/hot-socks-thank.md (1)
1-3: Empty changeset is appropriate for internal refactorMatches repo convention for src/** changes without user-facing notes.
Please confirm CI’s changeset check passes with this file name.
packages/react/transform/crates/swc_plugins_shared/lib.rs (1)
2-2: Publicly exporting jsx_helpers LGTMCentralizing helpers here simplifies imports across plugins.
packages/react/transform/crates/swc_plugins_shared/Cargo.toml (1)
14-15: Adds required deps for jsx_helpersonce_cell and regex are needed by jsx_helpers; good addition.
packages/react/transform/Cargo.toml (1)
29-31: Wire-up of new cratesPath dependencies for swc_plugin_list and swc_plugin_snapshot look correct.
If the repo uses a top-level Cargo workspace, ensure these crates don’t also need to be added to workspace.members (usually not required for path deps nested under a member).
packages/react/transform/src/lib.rs (1)
61-61: Importing JSXTransformer via napi modulePath update aligns with the new API boundary.
Confirm swc_plugin_snapshot::napi re-exports match the used signatures (new, with_content_hash) so downstream compiles cleanly.
packages/react/transform/crates/swc_plugin_snapshot/Cargo.toml (1)
1-21: New crate manifest looks consistent with workspace depsFeature set mirrors other transform crates and includes swc_plugins_shared.
packages/react/transform/crates/swc_plugin_list/Cargo.toml (1)
1-13: New list plugin crate manifest LGTMDependencies align with usage: swc_core, swc_plugin_snapshot, swc_plugins_shared.
packages/react/transform/crates/swc_plugin_snapshot/lib.rs (3)
27-38: Shared imports consolidation LGTMMoving JSX helpers, target, mode, and utils under swc_plugins_shared is the right direction.
39-42: Local module boundaryattr_name and slot_marker remain internal; no issues spotted.
1566-1567: Test imports remain from crate rootConsistent with keeping core transformer types internal while exposing napi for external use.
Ensure napi::JSXTransformer maps to the same behavior as crate::JSXTransformer used in tests (type alias or wrapper), to prevent divergence.
packages/react/transform/crates/swc_plugin_list/lib.rs (2)
11-11: Imports realigned to shared helpers — looks good.Moving to swc_plugins_shared::jsx_helpers is correct and matches the new module structure.
192-194: Ensure From/Into conversions exist for swc_plugins_shared napi enums → core types
Repository search returned no impl From/Into matches for swc_plugins_shared::transform_mode_napi::TransformMode or swc_plugins_shared::target_napi::TransformTarget — add or confirm conversions to the core TransformMode/TransformTarget used by JSXTransformer::new.packages/react/transform/crates/swc_plugin_snapshot/napi.rs (4)
44-55: From -> Core mapping looks correct.Field parity and conversions are aligned.
57-68: From -> napi config mapping looks correct.Symmetric with the previous impl.
93-108: VisitMut delegation is straightforward.Delegating element/module transforms to the inner core transformer is appropriate.
86-90: Constructor delegation is correct — verify TransformMode/TransformTarget Into/From impls existRepository search for impl From<swc_plugins_shared::transform_mode_napi::TransformMode> and impl From<swc_plugins_shared::target_napi::TransformTarget> returned no matches; confirm those conversions are implemented in swc_plugins_shared or add them.
packages/react/transform/crates/swc_plugin_snapshot/slot_marker.rs (3)
100-101: unreachable! for JSXSpreadChild matches team preference.Retains the intentional unreachable! behavior for spread children per prior team decision. No action needed.
11-14: Import migration verified — no stale jsx_helpers imports remain.
All matches use swc_plugins_shared::jsx_helpers; no imports from crate|super|swc_plugin_* were found.
9-10: WRAPPER_NODE_2 import OK — single definition & parent-private visibility is sufficientWRAPPER_NODE_2 is defined once as a static in packages/react/transform/crates/swc_plugin_snapshot/lib.rs (around line 71) and is referenced from slot_marker.rs via super::WRAPPER_NODE_2; child modules can access parent-private items, so no pub change is required and there’s no duplicate definition.
Summary by CodeRabbit
Refactor
Chores
Release Impact
Checklist