refactor: integrate swc plugin into react-transform #856
Conversation
|
gaoachao
force-pushed
the
refactor/adopt-swc-plugin
branch
2 times, most recently
from
e9548ab to
6184df6
Compare
gaoachao
force-pushed
the
refactor/adopt-swc-plugin
branch
3 times, most recently
from
4c4e427 to
4bab36d
Compare
gaoachao
force-pushed
the
refactor/adopt-swc-plugin
branch
from
4bab36d to
30f7edf
Compare
gaoachao
changed the title
gaoachao
force-pushed
the
refactor/adopt-swc-plugin
branch
from
30f7edf to
be3c220
Compare
packages/react/transform/swc-plugin-reactlynx-compat/turbo.json
Outdated
Show resolved
Hide resolved
packages/react/transform/swc-plugin-reactlynx/.cargo/config.toml
Outdated
Show resolved
Hide resolved
| serde_json = { version = "1.0.134", features = ["preserve_order"] } | ||
| sha-1 = "0.10.1" | ||
| swc_core = { version = "23.2.0", features = ["base", "ecma_codegen", "ecma_parser", "ecma_minifier", "ecma_transforms_typescript", "ecma_utils", "ecma_quote", "ecma_transforms_react", "ecma_transforms_optimization", "css_parser", "css_ast", "css_visit", "css_codegen", "__visit", "__testing_transform"] } | ||
| swc_core = { version = "23.2.0", features = ["base", "ecma_codegen", "ecma_parser", "ecma_minifier", "ecma_transforms_typescript", "ecma_utils", "ecma_quote", "ecma_transforms_react", "ecma_transforms_optimization", "css_parser", "css_ast", "css_visit", "css_codegen", "__visit", "__testing_transform", "ecma_plugin_transform"] } |
There was a problem hiding this comment.
Can we use [workspace.dependencies] instead? It would be easier to upgrade.
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 80 files out of 299 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughThis update fully migrates the React transform pipeline from a Rust/N-API hybrid architecture to a modern, WASM-based SWC plugin system with TypeScript-centric APIs. It removes all legacy Rust/N-API code, synchronizes all loader and test infrastructure to use the new async WASM transform, introduces dedicated SWC plugin packages for both the main transform and compatibility layers, and refactors configuration, build, and type declaration files throughout the monorepo. Numerous test snapshots and configuration files are updated to reflect new snapshot identifiers and plugin behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Loader/Plugin
participant WASM SWC Plugin
participant Rust Plugin (removed)
User->>Loader/Plugin: Request transform (JS/TS code)
Loader/Plugin->>WASM SWC Plugin: Await transformReactLynx(code, options)
WASM SWC Plugin->>WASM SWC Plugin: Apply config, run transforms
WASM SWC Plugin-->>Loader/Plugin: Return { code, map, warnings, errors }
Loader/Plugin-->>User: Return transformed code
Note over Rust Plugin: Rust/N-API transform path removed
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Suggested labels
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (4)
packages/react/transform/swc-plugin-reactlynx/Cargo.toml (2)
9-11: Profile configuration may be ignored in workspace context.While LTO optimization is beneficial for WASM performance, profiles for non-root packages are typically ignored in Cargo workspaces. Consider moving this to the workspace root
Cargo.tomlto ensure it takes effect.
22-22: Consider using workspace dependencies for easier maintenance.The
ecma_plugin_transformfeature addition is correct for SWC plugin functionality. However, as previously suggested, using[workspace.dependencies]would make version upgrades easier to manage across the monorepo.packages/react/transform/main.js (2)
44-44: Consider using 'unknown' as default for isModuleAs suggested in the previous review, defaulting to
'unknown'might be more appropriate thantruewhen the module type is not specified.
54-54: Clarify the choice of ES2022 targetThe previous review questioned why ES2022 is used as the target. Please provide justification for this choice or consider documenting the reasoning.
🧹 Nitpick comments (15)
packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-modules-lazy-bundle.js (1)
9-9: Avoid magic number ‟0” – replace with an explicit constant or enumThe fourth argument has been switched from
undefinedto the literal0.
Unless0is a documented, stable enum value, the intent is opaque to future readers and brittle if the API ever changes. Consider:-}, null, null, 0, globDynamicComponentEntry, null); +}, null, null, SNAPSHOT_NO_STYLE, globDynamicComponentEntry, null);where
SNAPSHOT_NO_STYLE(or similar) is imported from the runtime.If the SWC transform guarantees the value will always be
0, please add a short inline comment for clarity.Also applies to: 20-20, 28-28, 40-40
packages/react/transform/swc-plugin-reactlynx-compat/.cargo/config.toml (1)
1-5: Config is fine but keep the two plugin configs consistentThe compat plugin defines the two WASM aliases but omits the
[net]section present in the sibling plugin.
Unless intentional, replicate thegit-fetch-with-cli = truestanza so all plugins share the same network policy.packages/react/transform/swc-plugin-reactlynx/build.js (2)
12-18: Consider adding cargo-wasi dependency check.The build script assumes
cargo-wasiis installed. Consider adding a check or documenting this prerequisite to provide clearer error messages if the tool is missing.+// Check for cargo-wasi availability +try { + execSync('cargo wasi --version', { stdio: 'pipe' }); +} catch { + throw new Error('cargo-wasi is required but not installed. Install with: cargo install cargo-wasi'); +} + execSync('cargo build-wasi --release', {
20-26: Improve build artifact handling robustness.The hardcoded target path is brittle and could break if the workspace structure changes. Consider making the build more robust:
+import { existsSync } from 'node:fs'; + +const wasmSource = path.resolve( + __dirname, + '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx.wasm', +); +const wasmTarget = path.resolve(__dirname, 'swc_plugin_reactlynx.wasm'); + +if (!existsSync(wasmSource)) { + throw new Error(`WASM build artifact not found at: ${wasmSource}`); +} + -await fs.copyFile( - path.resolve( - __dirname, - '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx.wasm', - ), - path.resolve(__dirname, 'swc_plugin_reactlynx.wasm'), -); +await fs.copyFile(wasmSource, wasmTarget);packages/react/transform/swc-plugin-reactlynx-compat/package.json (1)
5-5: Add a meaningful description.The package description is empty. Consider adding a brief description explaining the purpose of this compatibility plugin.
- "description": "", + "description": "SWC plugin for React Lynx compatibility transforms",packages/react/transform/swc-plugin-reactlynx/package.json (1)
5-5: Add a meaningful description.The package description is empty. Consider adding a brief description explaining the purpose of this main React transform plugin.
- "description": "", + "description": "SWC plugin for React Lynx transforms and optimizations",packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs (1)
4-8: Consider Rust naming conventions for enum variants.The enum variants use
UPPER_CASEnaming, which is unconventional in Rust (typicallyPascalCase). If this naming is required for external compatibility, consider adding a comment explaining why.If the naming must match external systems, consider adding a comment:
// Note: Variant names match external system requirements pub enum TransformTarget { LEPUS, JS, MIXED, }packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_css_scope/mod.rs (1)
44-49: Simplify error handling by removing explicit returnThe explicit
returnstatement is unnecessary as this is the last expression in the match.- _ => { - return Err(serde::de::Error::custom(format!( - "value `{}` does not match any variant of CSSScope", - s - ))); - } + _ => Err(serde::de::Error::custom(format!( + "value `{}` does not match any variant of CSSScope", + s + )))packages/react/package.json (1)
61-68: Consider reordering export fields for consistencyWhile the exports are correct, consider placing "types" before "default" to follow the conventional ordering used by most packages and as recommended by the Node.js documentation.
"./transform/swc-plugin-reactlynx-compat": { - "default": "./transform/swc-plugin-reactlynx-compat/swc_plugin_reactlynx_compat.wasm", - "types": "./transform/swc-plugin-reactlynx-compat/index.d.ts" + "types": "./transform/swc-plugin-reactlynx-compat/index.d.ts", + "default": "./transform/swc-plugin-reactlynx-compat/swc_plugin_reactlynx_compat.wasm" }, "./transform/swc-plugin-reactlynx": { - "default": "./transform/swc-plugin-reactlynx/swc_plugin_reactlynx.wasm", - "types": "./transform/swc-plugin-reactlynx/index.d.ts" + "types": "./transform/swc-plugin-reactlynx/index.d.ts", + "default": "./transform/swc-plugin-reactlynx/swc_plugin_reactlynx.wasm" },packages/react/transform/swc-plugin-reactlynx-compat/index.d.ts (2)
35-305: Consider planning deprecation removal strategyThe interface contains several deprecated properties (
simplifyCtorLikeReactLynx2,removeComponentAttrRegex,darkMode). While the deprecation warnings are well-documented, consider:
- Adding a deprecation timeline or version when these will be removed
- Creating a migration guide for users still using these features
The interface is otherwise well-structured with clear type definitions and comprehensive documentation.
307-314: Interface part of deprecated featureThis
DarkModeConfiginterface is only used by the deprecateddarkModeproperty inCompatVisitorConfig. It should be removed when the deprecated feature is removed.packages/react/transform/main.js (1)
5-5: Track the rslib migration TODOThe TODO comment indicates a planned refactoring to use rslib for improved TypeScript support. Consider creating an issue to track this technical debt.
Would you like me to create an issue to track this rslib migration task?
packages/react/transform/swc-plugin-reactlynx/src/lib.rs (1)
87-132: Consider documenting the "Nodiff" naming conventionThe struct name
TransformNodiffOptionsuses "Nodiff" which might not be immediately clear to users. Consider adding a comment explaining what "Nodiff" means in this context.packages/react/transform/swc-plugin-reactlynx/index.d.ts (2)
103-108: Consider using a discriminated union for better type safetyThe current tuple approach for
injectconfiguration works but could be more type-safe:-export interface InjectVisitorConfig { - inject: Record< - string, - ['expr', string] | ['importDefault', string] | ['importStarAs', string] | ['importNamed', string, string] - >; -} +export interface InjectVisitorConfig { + inject: Record<string, InjectType>; +} + +type InjectType = + | { type: 'expr'; value: string } + | { type: 'importDefault'; module: string } + | { type: 'importStarAs'; module: string } + | { type: 'importNamed'; module: string; name: string };This would provide better IntelliSense and type checking.
193-224: Consider separating internal and public interfacesMost properties in
JsxTransformerConfigand several inWorkletVisitorConfigare marked as@internal. Consider splitting these into separate internal and public interfaces to make the public API clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (79)
.gitignore(1 hunks)Cargo.toml(1 hunks)biome.jsonc(1 hunks)eslint.config.js(2 hunks)packages/react/.dprint.jsonc(1 hunks)packages/react/package.json(1 hunks)packages/react/runtime/package.json(0 hunks)packages/react/transform/.cargo/config.toml(0 hunks)packages/react/transform/.gitignore(0 hunks)packages/react/transform/.npmignore(0 hunks)packages/react/transform/__test__/basic.bench.js(1 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-all-lazy-bundle.js(3 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-modules-lazy-bundle.js(3 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-none-lazy-bundle.js(3 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-defaults.js(4 hunks)packages/react/transform/__test__/css/css-scope.spec.js(1 hunks)packages/react/transform/__test__/css/inline-style.spec.js(2 hunks)packages/react/transform/__test__/css/object-lit-style.spec.js(8 hunks)packages/react/transform/__test__/fixture.spec.js(43 hunks)packages/react/transform/__test__/typescript.spec.js(8 hunks)packages/react/transform/build.rs(0 hunks)packages/react/transform/cjs/main.cjs(0 hunks)packages/react/transform/cjs/main.d.cts(0 hunks)packages/react/transform/cjs/package.json(0 hunks)packages/react/transform/index.d.ts(1 hunks)packages/react/transform/main.js(1 hunks)packages/react/transform/package.json(1 hunks)packages/react/transform/scripts/build_wasm.js(0 hunks)packages/react/transform/src/bundle.rs(0 hunks)packages/react/transform/src/esbuild.rs(0 hunks)packages/react/transform/src/lib.rs(0 hunks)packages/react/transform/src/swc_plugin_compat_post/mod.rs(0 hunks)packages/react/transform/src/swc_plugin_refresh/mod.rs(0 hunks)packages/react/transform/src/target.rs(0 hunks)packages/react/transform/src/wasm.js(0 hunks)packages/react/transform/swc-plugin-reactlynx-compat/.cargo/config.toml(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/.gitignore(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/.npmignore(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/Cargo.toml(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/build.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/index.d.ts(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/package.json(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs(5 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/turbo.json(1 hunks)packages/react/transform/swc-plugin-reactlynx/.cargo/config.toml(1 hunks)packages/react/transform/swc-plugin-reactlynx/.gitignore(1 hunks)packages/react/transform/swc-plugin-reactlynx/.npmignore(1 hunks)packages/react/transform/swc-plugin-reactlynx/Cargo.toml(1 hunks)packages/react/transform/swc-plugin-reactlynx/build.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/index.d.ts(1 hunks)packages/react/transform/swc-plugin-reactlynx/package.json(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/lib.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_css_scope/mod.rs(3 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_define_dce/mod.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_directive_dce/mod.rs(2 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_dynamic_import/mod.rs(2 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_extract_str/mod.rs(0 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_inject/mod.rs(2 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_shake/mod.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_snapshot/mod.rs(2 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_worklet/mod.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/target.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx/turbo.json(1 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_compat_post/mod.rs/should_compat_dark_mode.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_compat_post/mod.rs/should_compat_dark_mode_custom_theme_expr.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_basic.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_custom_library.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_function_parameter.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_import_computed.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_import_default.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_import_named_computed.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_import_namespace_computed.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_init_value.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_local_ident.js(0 hunks)packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_refresh/mod.rs/should_handle_multiple_index.js(0 hunks)packages/react/transform/turbo.json(1 hunks)patches/@[email protected](0 hunks)pnpm-workspace.yaml(0 hunks)
💤 Files with no reviewable changes (31)
- packages/react/runtime/package.json
- packages/react/transform/.npmignore
- packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_extract_str/mod.rs
- pnpm-workspace.yaml
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_compat_post/mod.rs/should_compat_dark_mode.js
- packages/react/transform/.cargo/config.toml
- packages/react/transform/cjs/main.d.cts
- packages/react/transform/cjs/package.json
- packages/react/transform/cjs/main.cjs
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_import_namespace_computed.js
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_custom_library.js
- packages/react/transform/.gitignore
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_import_computed.js
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_import_default.js
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_function_parameter.js
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_init_value.js
- packages/react/transform/src/wasm.js
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_basic.js
- packages/react/transform/src/esbuild.rs
- packages/react/transform/build.rs
- packages/react/transform/src/target.rs
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_multiple_index.js
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_compat_post/mod.rs/should_compat_dark_mode_custom_theme_expr.js
- packages/react/transform/src/swc_plugin_compat_post/mod.rs
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_local_ident.js
- packages/react/transform/tests/swc_snapshots/src/swc_plugin_refresh/mod.rs/should_handle_import_named_computed.js
- patches/@[email protected]
- packages/react/transform/src/bundle.rs
- packages/react/transform/src/swc_plugin_refresh/mod.rs
- packages/react/transform/src/lib.rs
- packages/react/transform/scripts/build_wasm.js
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-modules-lazy-bundle.js (2)
packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-none-lazy-bundle.js (10)
__snapshot_2d408_test_2(11-20)__snapshot_2d408_test_3(22-28)pageId(4-4)pageId(12-12)pageId(23-23)pageId(35-35)el(5-5)el(13-13)el(24-24)el(36-36)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-modules.js (2)
__snapshot_2d408_test_2(11-20)__snapshot_2d408_test_3(22-28)
packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-all-lazy-bundle.js (3)
packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-modules-lazy-bundle.js (10)
__snapshot_2d408_test_2(11-20)__snapshot_2d408_test_3(22-28)pageId(4-4)pageId(12-12)pageId(23-23)pageId(35-35)el(5-5)el(13-13)el(24-24)el(36-36)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-none-lazy-bundle.js (10)
__snapshot_2d408_test_2(11-20)__snapshot_2d408_test_3(22-28)pageId(4-4)pageId(12-12)pageId(23-23)pageId(35-35)el(5-5)el(13-13)el(24-24)el(36-36)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-all.js (6)
__snapshot_2d408_test_2(11-20)__snapshot_2d408_test_3(22-28)pageId(4-4)pageId(12-12)el(5-5)el(13-13)
🔇 Additional comments (58)
.gitignore (1)
10-10: LGTM: Appropriate ignore pattern for SWC architecture.Adding
.swcto the global ignore patterns aligns with the architectural shift to SWC-based plugins and follows standard practices for ignoring SWC build artifacts and cache directories.eslint.config.js (2)
45-47: LGTM: Updated ignore patterns align with new plugin structure.The updated ESLint ignore patterns properly reflect the new directory structure with separate SWC plugin packages. The patterns are more targeted and specific, which improves maintainability while ensuring appropriate files are excluded from linting.
65-65: LGTM: Generalized TypeScript declaration ignore pattern.The updated pattern
packages/react/transform/**/index.d.tsappropriately covers generated TypeScript declarations across the new plugin subdirectories while being more flexible than the previous specific patterns.packages/react/transform/swc-plugin-reactlynx/.gitignore (1)
1-2: LGTM: Standard ignore patterns for WebAssembly plugin.The ignore patterns are appropriate for a WebAssembly-based SWC plugin, correctly excluding build artifacts (
dist) and the generated WASM binary (/*.wasm). The patterns are minimal and focused, following best practices for plugin packages.packages/react/transform/swc-plugin-reactlynx/.npmignore (1)
1-3: LGTM: Excellent whitelist approach for npm packaging.The
.npmignoreuses a whitelist strategy that's ideal for plugin packages - excluding everything by default and only including the essential runtime files (index.d.tsfor TypeScript support andswc_plugin_reactlynx.wasmfor the actual plugin functionality). This approach prevents accidental inclusion of source files and ensures minimal package size.packages/react/transform/swc-plugin-reactlynx-compat/.gitignore (1)
1-2: LGTM: Consistent ignore patterns across plugin packages.The ignore patterns match those in
swc-plugin-reactlynx, maintaining consistency across the plugin packages. The patterns appropriately exclude build artifacts (dist) and generated WASM binaries (/*.wasm), following the same architectural approach.packages/react/transform/swc-plugin-reactlynx-compat/.npmignore (1)
1-3: LGTM! Well-structured .npmignore for WASM plugin package.The configuration correctly excludes all files by default and includes only the essential files for the SWC plugin package - the TypeScript declarations and the compiled WebAssembly binary. This follows best practices for distributing WASM-based plugins.
packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-none-lazy-bundle.js (1)
9-9: LGTM! Consistent snapshot parameter standardization.The fourth parameter change from
undefinedto0in allReactLynx.createSnapshotcalls is consistent with the broader refactoring and matches the pattern applied across other snapshot test files.Also applies to: 20-20, 28-28, 40-40
Cargo.toml (1)
4-7: Excellent workspace restructuring for plugin modularization.The updated workspace members correctly reflect the architectural shift from a monolithic transform package to two specialized SWC plugin packages. This enables independent development, building, and versioning of each plugin while maintaining proper workspace-level coordination.
packages/react/transform/turbo.json (1)
7-8: Dependency list looks good – please double-check the task names exist
@lynx-js/swc-plugin-reactlynx#buildand@lynx-js/swc-plugin-reactlynx-compat#buildare sensible new dependencies that replace the removedbuild:wasmtask.
Runturbo run build --dry-runonce to ensure both tasks are discoverable and there are no circular deps.packages/react/transform/swc-plugin-reactlynx/.cargo/config.toml (1)
1-8: Confirm we still needgit-fetch-with-cli = truePrevious feedback questioned whether this setting is required. If crates are fetched via crates.io only, the flag can be removed to avoid the extra Git dependency.
Otherwise, leave it in place but add a comment explaining why (e.g. using git-based dependencies).
packages/react/transform/swc-plugin-reactlynx/Cargo.toml (1)
19-19: Clarify the serde version downgrade.The serde version was downgraded from 1.0.217 to 1.0.216. Please explain the reason for this downgrade - is it due to dependency conflicts or compatibility requirements?
packages/react/transform/swc-plugin-reactlynx/turbo.json (1)
1-12: LGTM! Turbo configuration is well-structured.The build task configuration correctly includes all necessary inputs (Rust source files, Cargo.toml, and build script) and properly specifies the WASM output. The configuration follows Turbo best practices.
biome.jsonc (1)
37-37: LGTM! Improved ignore pattern for new package structure.The generalized pattern
packages/react/transform/**/index.d.tsis more maintainable and correctly covers the new sub-packages introduced in this refactor.packages/react/transform/__test__/css/css-scope.spec.js (2)
8-25: LGTM! Test configuration updated for new API.The
defaultOptionsconfiguration correctly reflects the new API structure, replacing thejsxoption withsnapshotand removing deprecated properties likepluginNameandrefresh.
164-167: LGTM! Proper snapshot configuration override pattern.The test correctly uses the spread operator to override specific snapshot properties while maintaining the base configuration, demonstrating proper usage of the new API.
packages/react/transform/swc-plugin-reactlynx-compat/turbo.json (1)
1-11: LGTM! Past review feedback has been addressed.The configuration correctly:
- Removes the unnecessary
//#builddependency as suggested in previous reviews- Specifies the WASM output file
swc_plugin_reactlynx_compat.wasmin the outputs array- Properly defines inputs for Rust source files, Cargo manifest, and build script
packages/react/transform/__test__/css/inline-style.spec.js (3)
17-18: LGTM! Test assertions simplified and improved.The direct assertion of empty arrays for warnings and errors is cleaner than the previous
esbuild.formatMessagesapproach and aligns with the migration away from esbuild dependencies.
29-30: Consistent test assertion pattern.Good consistency with the simplified error/warning assertions across test cases.
42-43: Consistent test assertion pattern.Maintains the same simplified assertion pattern for the third test case.
packages/react/transform/__test__/typescript.spec.js (3)
13-15: LGTM! Proper syntax configuration for TypeScript parsing.The explicit
syntaxConfigwith TypeScript syntax andtsx: falseis appropriate for these test cases and aligns with the new transform function signature.
27-29: Consistent syntax configuration pattern.Good consistency in applying the same TypeScript syntax configuration across test cases.
40-42: Maintains consistent configuration across tests.The systematic application of syntax configuration ensures all TypeScript tests use the correct parser settings.
packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_directive_dce/mod.rs (2)
11-11: LGTM! Clean migration to Serde deserialization.Adding
serde::Deserializeimport supports the transition from N-API bindings to standard Rust serialization for the WASM plugin architecture.
39-43: Proper Serde configuration for WASM plugin.The migration from
#[napi(object)]to#[derive(Deserialize, Clone, Debug, PartialEq)]is correct and maintains the same configuration interface while enabling JSON deserialization for the SWC plugin system.packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_dynamic_import/mod.rs (2)
3-3: LGTM! Added Serde support for WASM plugin configuration.The
serde::Deserializeimport enables JSON-based configuration deserialization, replacing the previous N-API binding approach.
21-28: Excellent migration to Serde with JavaScript compatibility.The transition from
#[napi(object)]to Serde derives is well-executed:
#[serde(rename_all = "camelCase")]ensures proper JavaScript field name mapping- Standard Rust traits (Clone, Debug, PartialEq) provide good ergonomics
- Maintains the same configuration interface (runtime_pkg, layer fields)
packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_define_dce/mod.rs (1)
1-6: LGTM! Clean migration from N-API to Serde.The migration correctly replaces N-API bindings with Serde deserialization. The addition of
PartialEqderive is beneficial for testing and comparison operations.packages/react/transform/swc-plugin-reactlynx-compat/package.json (1)
1-22: LGTM! Package structure aligns with WebAssembly plugin architecture.The package configuration correctly defines the WebAssembly-based plugin with appropriate main entry, types, and build scripts.
packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_shake/mod.rs (1)
8-15: LGTM! Excellent migration with JavaScript interoperability.The migration correctly implements Serde deserialization with
#[serde(rename_all = "camelCase")]attribute, ensuring proper field name conversion between Rust (snake_case) and JavaScript (camelCase) conventions.packages/react/transform/swc-plugin-reactlynx/package.json (1)
1-22: LGTM! Package structure is consistent with the plugin architecture.The package configuration properly defines the main WebAssembly plugin with correct entry points and build configuration.
packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs (2)
10-28: LGTM! Excellent custom Deserialize implementation.The custom deserialization provides clear error messages and proper error handling. The implementation correctly uses
serde::de::Error::customfor meaningful error reporting.
30-69: LGTM! Comprehensive test coverage.The tests cover all valid variants and error cases, ensuring robust deserialization behavior. The error message validation in the test is particularly valuable.
packages/react/.dprint.jsonc (1)
10-12: LGTM! Exclusion patterns correctly updated for modular structure.The refined exclusion patterns properly reflect the new directory structure with
swc-plugin-reactlynxandswc-plugin-reactlynx-compatsub-packages, ensuring formatting tools ignore the appropriate snapshot and build artifact directories.packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_snapshot/mod.rs (2)
27-27: LGTM! Serde import added for deserialization.The addition of the Serde import is necessary for the migration from N-API to SWC plugin-based deserialization.
987-988: LGTM! Configuration struct properly migrated to Serde.The replacement of
#[napi(object)]with Serde derive attributes correctly implements the migration from N-API bindings to SWC plugin configuration deserialization. ThecamelCaserenaming ensures JavaScript compatibility.packages/react/transform/swc-plugin-reactlynx/src/target.rs (1)
1-69: Excellent enum implementation!This is a well-structured TransformTarget enum with proper Rust practices:
- Appropriate derives (Debug, PartialEq, Clone, Copy)
- Custom Deserialize implementation provides clear error messages
- Comprehensive test coverage for all variants and error cases
- Clean string matching logic with helpful error reporting
The implementation follows Rust best practices and integrates well with the new SWC plugin architecture.
packages/react/transform/package.json (2)
5-8: Good addition of license field.The Apache-2.0 license field addresses the previous review feedback. The empty metadata fields (description, keywords, author) are acceptable for internal packages, but consider completing them if this package will be published externally in the future.
12-17: Clean dependency simplification aligns with new architecture.The removal of devDependencies and build scripts, with only
@rspack/coreremaining, properly reflects the migration to the SWC plugin-based architecture where build complexity is handled by the individual plugin packages.packages/react/transform/swc-plugin-reactlynx-compat/Cargo.toml (1)
1-24: Well-configured Cargo manifest for SWC plugin.This is a professionally structured Cargo.toml with:
- Appropriate
cdylibcrate type for WebAssembly compilation- LTO optimization for efficient WASM output
- Comprehensive
swc_corefeatures covering all transformation needs- Well-chosen dependencies for plugin functionality
- Helpful build configuration comments
The manifest effectively supports the new SWC plugin architecture and WASM compilation targets.
packages/react/transform/__test__/css/object-lit-style.spec.js (2)
18-19: Good modernization of test assertions.The change from
toEqual([])totoMatchInlineSnapshot(\[]`)` for warnings and errors simplifies assertion maintenance and removes the dependency on esbuild message formatting. This aligns well with the overall diagnostic simplification in the new architecture.Also applies to: 32-33, 47-48, 264-265, 278-279
92-96: Configuration updates properly reflect API changes.The changes from
jsxtosnapshotand removal ofpluginName/refreshproperties correctly adapt the tests to the new transform API while preserving the core test logic and expected behavior.Also applies to: 153-157, 210-214
packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_css_scope/mod.rs (2)
3-3: Clean migration from N-API to SerdeThe transition from N-API to Serde-based deserialization is well-executed. The custom
Deserializeimplementation forCSSScopecorrectly handles all three variants with appropriate error handling.Also applies to: 27-27, 34-52, 54-54
332-366: Comprehensive test coverage for CSSScope deserializationExcellent test coverage! The tests properly verify all valid enum variants and the error handling for unknown values.
packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_inject/mod.rs (2)
14-95: Well-structured custom deserialization for InjectAs enumThe custom deserialization implementation correctly handles the array-based format with proper validation for each variant's expected arguments. The error messages are clear and helpful.
The array-based format
["variant", ...args]for theInjectAsenum is unconventional. Please verify this format is correctly documented in the API documentation and matches the expected input from the JavaScript/TypeScript side.
97-101: Clean migration of InjectVisitorConfig to SerdeThe struct properly derives
Deserializewith appropriatecamelCasefield naming for JavaScript interoperability.packages/react/transform/__test__/fixture.spec.js (3)
7-7: Import path uses ESM syntax correctlyThe import from
'../main.js'correctly includes the.jsextension as required for ESM modules.
124-124: Configuration API improvements enhance developer experienceThe changes from
jsxtosnapshotand from JSON stringsyntaxConfigto direct object notation improve type safety and API clarity.Also applies to: 179-183, 435-438
154-164: Simplified error handling improves test maintainabilityRemoving the esbuild dependency and using direct error/warning arrays makes the tests more maintainable and reduces external dependencies.
Also applies to: 241-242, 472-481
packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (2)
4-14: Excellent migration to SWC plugin architectureThe refactoring from N-API to SWC plugin architecture is well-executed:
- The
Eitherenum elegantly handles union types- Proper Serde deserialization throughout
- Clean plugin transform function implementation
Also applies to: 57-62, 72-77, 1289-1296
1291-1295: Safe configuration handling in plugin transformGood use of
unwrap_or_default()for both JSON retrieval and parsing, ensuring the plugin gracefully handles missing or invalid configuration.packages/react/transform/swc-plugin-reactlynx-compat/index.d.ts (1)
5-30: Well-structured interface with comprehensive documentationThe
AddComponentElementConfiginterface is properly typed and includes detailed JSDoc comments with practical examples.packages/react/transform/swc-plugin-reactlynx/src/lib.rs (3)
50-56: Good use of Either enum for flexible configurationThe
Eitherenum pattern allows users to specify simple boolean flags or detailed configuration objects, providing a nice balance between simplicity and flexibility.
57-86: Well-implemented custom deserialization with clear error messagesThe
TransformModeenum's custom deserialization provides descriptive error messages for invalid values, which will help users debug configuration issues.
362-471: Comprehensive test coverageThe test suite provides good coverage of configuration parsing, including edge cases and error scenarios. The tests are well-organized and clearly named.
packages/react/transform/swc-plugin-reactlynx/index.d.ts (1)
110-192: Excellent documentation for shake configurationThe
ShakeVisitorConfiginterface provides clear documentation about default values and the merging behavior, which is crucial for users to understand how their configuration interacts with defaults.packages/react/transform/index.d.ts (2)
1-40: Clean modular design with proper type importsThe refactored structure properly imports configuration types from the plugin packages, maintaining good separation of concerns. The
isModuleproperty correctly supports the 'unknown' value as discussed in previous reviews.
41-46: Output interface aligns with simplified error handlingThe
errorsandwarningsas string arrays match the implementation. As suggested in the review ofmain.js, ensure error objects are properly converted to strings.
| sourcemap: false, | ||
| cssScope: false, | ||
| jsx: true, | ||
| snapshot: true, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good refactoring of the configuration property.
The change from jsx to snapshot provides clearer semantic meaning and aligns with the broader API simplification in the SWC-based transform migration.
🏁 Script executed:
#!/bin/bash
# Description: Verify all test files use the new 'snapshot' property instead of 'jsx'
# Expected: No remaining 'jsx' properties in transform test configs
echo "Checking for any remaining 'jsx' property in test configs:"
rg -B 2 -A 2 "jsx:\s*(true|false)" packages/react/transform/__test__
echo -e "\nVerifying all test configs now use 'snapshot' property:"
rg -B 2 -A 2 "snapshot:\s*(true|false)" packages/react/transform/__test__Length of output: 8241
Legacy jsx property detected in test configs
The migration to the new SWC-based API isn’t complete—packages/react/transform/__test__/fixture.spec.js still uses the old jsx key. Please update all occurrences of jsx to snapshot, preserving the boolean values. For example:
- jsx: false,
+ snapshot: false,Once updated, rerun the benchmark and fixture tests to confirm everything now consistently uses snapshot.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/react/transform/__test__/basic.bench.js at line 25, replace the
legacy jsx property with the new snapshot property, keeping the boolean value
unchanged. After updating all jsx occurrences to snapshot, rerun the benchmark
and fixture tests to ensure consistency and correctness.
packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-all-lazy-bundle.js
Show resolved
Hide resolved
| /*#__PURE__*/ import { jsx as _jsx } from "@lynx-js/react/jsx-runtime"; | ||
| import * as ReactLynx from "@lynx-js/react"; | ||
| const __snapshot_da39a_4683e_1 = /*#__PURE__*/ ReactLynx.createSnapshot("__snapshot_da39a_4683e_1", function() { | ||
| const __snapshot_da39a_74a8fee4_1 = /*#__PURE__*/ ReactLynx.createSnapshot("__snapshot_da39a_74a8fee4_1", function() { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent fourth argument (undefined vs 0) – tests may diverge
Other updated snapshots switched the 4-th parameter of createSnapshot to 0.
This file still passes undefined, leading to inconsistent runtime semantics and brittle snapshot comparisons.
-}, null, null, undefined, globDynamicComponentEntry, null);
+}, null, null, 0, globDynamicComponentEntry, null);Apply to all four snapshot invocations in this file.
Also applies to: 10-10, 11-11, 21-21, 22-22, 31-31, 34-34, 43-43
🤖 Prompt for AI Agents
In packages/react/transform/__test__/css/__snapshots__/transform-jsx-defaults.js
around lines 3 and the other specified lines (10, 11, 21, 22, 31, 34, 43), the
fourth argument passed to ReactLynx.createSnapshot is currently undefined but
should be changed to 0 for consistency. Update all four snapshot invocations in
this file to pass 0 as the fourth argument instead of undefined to ensure
consistent runtime behavior and stable snapshot comparisons.
| try { | ||
| const transformResult = await swc.transform(source, transformOptions); | ||
| result.code = transformResult.code; | ||
| result.map = transformResult.map; | ||
| result.warnings = transformResult.diagnostics; | ||
| } catch (e) { | ||
| result.errors.push(e); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling with more context
The current error handling simply pushes the error to an array. Consider extracting more meaningful information from the error object:
try {
const transformResult = await swc.transform(source, transformOptions);
result.code = transformResult.code;
result.map = transformResult.map;
result.warnings = transformResult.diagnostics;
} catch (e) {
- result.errors.push(e);
+ result.errors.push(e.message || String(e));
}This ensures error messages are properly stringified for the output interface.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const transformResult = await swc.transform(source, transformOptions); | |
| result.code = transformResult.code; | |
| result.map = transformResult.map; | |
| result.warnings = transformResult.diagnostics; | |
| } catch (e) { | |
| result.errors.push(e); | |
| } | |
| try { | |
| const transformResult = await swc.transform(source, transformOptions); | |
| result.code = transformResult.code; | |
| result.map = transformResult.map; | |
| result.warnings = transformResult.diagnostics; | |
| } catch (e) { | |
| result.errors.push(e.message || String(e)); | |
| } |
🤖 Prompt for AI Agents
In packages/react/transform/main.js around lines 94 to 102, the catch block
currently pushes the raw error object to result.errors, which may not provide
clear or consistent error messages. Update the catch block to extract and
stringify meaningful information from the error object, such as using e.message
or converting the error to a string, before pushing it to result.errors. This
will ensure error messages are clear and properly formatted for the output
interface.
| execSync('cargo build-wasi --release', { | ||
| env: { | ||
| ...process.env, | ||
| RUSTFLAGS: '-C link-arg=--export-table -C link-arg=-s', | ||
| }, | ||
| stdio: 'inherit', | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and path validation.
The build script lacks error handling and uses hardcoded paths which could fail if the directory structure changes.
Consider adding error handling and path validation:
-execSync('cargo build-wasi --release', {
- env: {
- ...process.env,
- RUSTFLAGS: '-C link-arg=--export-table -C link-arg=-s',
- },
- stdio: 'inherit',
-});
+try {
+ execSync('cargo build-wasi --release', {
+ env: {
+ ...process.env,
+ RUSTFLAGS: '-C link-arg=--export-table -C link-arg=-s',
+ },
+ stdio: 'inherit',
+ });
+} catch (error) {
+ console.error('Failed to build WASM plugin:', error.message);
+ process.exit(1);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| execSync('cargo build-wasi --release', { | |
| env: { | |
| ...process.env, | |
| RUSTFLAGS: '-C link-arg=--export-table -C link-arg=-s', | |
| }, | |
| stdio: 'inherit', | |
| }); | |
| try { | |
| execSync('cargo build-wasi --release', { | |
| env: { | |
| ...process.env, | |
| RUSTFLAGS: '-C link-arg=--export-table -C link-arg=-s', | |
| }, | |
| stdio: 'inherit', | |
| }); | |
| } catch (error) { | |
| console.error('Failed to build WASM plugin:', error.message); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In packages/react/transform/swc-plugin-reactlynx-compat/build.js around lines 13
to 19, the execSync call to run the cargo build lacks error handling and uses
hardcoded paths that may break if the directory structure changes. To fix this,
wrap the execSync call in a try-catch block to handle any errors during
execution gracefully. Also, validate or dynamically resolve the paths used in
the build command to avoid hardcoding, ensuring the script adapts to directory
changes.
| await fs.copyFile( | ||
| path.resolve( | ||
| __dirname, | ||
| '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx_compat.wasm', | ||
| ), | ||
| path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm'), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify source file exists before copying.
The script assumes the WASM file exists at the hardcoded path without validation.
Add file existence check:
+import { access } from 'node:fs/promises';
+const wasmSource = path.resolve(
+ __dirname,
+ '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx_compat.wasm',
+);
+const wasmTarget = path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm');
+try {
+ await access(wasmSource);
+} catch {
+ throw new Error(`WASM file not found at: ${wasmSource}`);
+}
-await fs.copyFile(
- path.resolve(
- __dirname,
- '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx_compat.wasm',
- ),
- path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm'),
-);
+await fs.copyFile(wasmSource, wasmTarget);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await fs.copyFile( | |
| path.resolve( | |
| __dirname, | |
| '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx_compat.wasm', | |
| ), | |
| path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm'), | |
| ); | |
| import { access } from 'node:fs/promises'; | |
| const wasmSource = path.resolve( | |
| __dirname, | |
| '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx_compat.wasm', | |
| ); | |
| const wasmTarget = path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm'); | |
| try { | |
| await access(wasmSource); | |
| } catch { | |
| throw new Error(`WASM file not found at: ${wasmSource}`); | |
| } | |
| await fs.copyFile(wasmSource, wasmTarget); |
🤖 Prompt for AI Agents
In packages/react/transform/swc-plugin-reactlynx-compat/build.js around lines 21
to 27, the code copies a WASM file without verifying if the source file exists,
which can cause errors if the file is missing. Add a check using fs.existsSync
or fs.promises.access to confirm the source file exists before attempting to
copy it. If the file does not exist, handle the error gracefully by logging an
informative message or throwing an error to prevent silent failures.
| // let react_transformer = Optional::new( | ||
| // react::react( | ||
| // cm.clone(), | ||
| // Some(&comments), | ||
| // react::Options { | ||
| // next: Some(false), | ||
| // runtime: Some(react::Runtime::Automatic), | ||
| // import_source: snapshot_plugin_config.jsx_import_source.clone(), | ||
| // pragma: None, | ||
| // pragma_frag: None, | ||
| // // We may want `main-thread:foo={fooMainThreadFunc}` to work | ||
| // throw_if_namespace: Some(false), | ||
| // development: Some(matches!(options.mode, Some(TransformMode::Development))), | ||
| // refresh: None, | ||
| // ..Default::default() | ||
| // }, | ||
| // top_level_mark, | ||
| // unresolved_mark, | ||
| // ), | ||
| // enabled && !snapshot_plugin_config.preserve_jsx, | ||
| // ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove or document commented-out React transformer code
There's a large block of commented-out code for React transformation. This should either be:
- Removed if it's no longer needed
- Converted to a TODO comment explaining why it's kept and when it will be implemented
- Implemented if it's required functionality
Leaving commented code without explanation creates confusion about the implementation status.
🤖 Prompt for AI Agents
In packages/react/transform/swc-plugin-reactlynx/src/lib.rs around lines 232 to
252, there is a large block of commented-out React transformer code. You should
either remove this code if it is no longer needed, convert it into a clear TODO
comment explaining why it is retained and when it will be implemented, or
implement the functionality if it is required. This will clarify the codebase
and avoid confusion about the status of this code.
| #[derive(Deserialize, Clone, Debug, PartialEq)] | ||
| #[serde[rename_all = "camelCase"]] |
There was a problem hiding this comment.
Fix syntax error in serde attribute.
There's a syntax error in the serde attribute - it should use parentheses instead of square brackets.
-#[serde[rename_all = "camelCase"]]
+#[serde(rename_all = "camelCase")]Otherwise, the migration from N-API to Serde deserialization looks good and aligns well with the WebAssembly plugin architecture transition.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[derive(Deserialize, Clone, Debug, PartialEq)] | |
| #[serde[rename_all = "camelCase"]] | |
| #[derive(Deserialize, Clone, Debug, PartialEq)] | |
| #[serde(rename_all = "camelCase")] |
🤖 Prompt for AI Agents
In packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_worklet/mod.rs
at lines 28 to 29, the serde attribute uses square brackets instead of
parentheses, causing a syntax error. Replace the square brackets around
rename_all = "camelCase" with parentheses to fix the attribute syntax.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
packages/react/transform/main.js (3)
41-41: DefaultisModulevalue is set totrueinstead of'unknown'.A past review suggested using
'unknown'as the default. The current default oftrueassumes the input is always a module, which might not be correct for all use cases. Consider if this assumption is valid for your use case.
51-51: Clarify the choice of ES2022 as the SWC target.The SWC target is set to 'es2022'. Could you clarify why this specific target was chosen? This affects the output JavaScript syntax and browser compatibility.
91-98: Enhance error handling with more context.The current error handling pushes the raw error object to the array. Consider extracting meaningful information from the error:
try { const transformResult = await swc.transform(source, transformOptions); result.code = transformResult.code; result.map = transformResult.map; result.warnings = transformResult.diagnostics; } catch (e) { - result.errors.push(e); + result.errors.push(e.message || String(e)); }This ensures error messages are properly stringified for the output interface.
packages/react/transform/swc-plugin-reactlynx/src/lib.rs (1)
232-252: Remove or document commented-out React transformer code.There's a large block of commented-out code for React transformation. This should either be:
- Removed if it's no longer needed
- Converted to a TODO comment explaining why it's kept and when it will be implemented
- Implemented if it's required functionality
Leaving commented code without explanation creates confusion about the implementation status.
🧹 Nitpick comments (1)
packages/react/testing-library/loaders/jsx-loader.js (1)
29-29: Update error message to reflect new function name.The error message still references the old
transformReactLynxSyncfunction name but should referencetransformReactLynxto match the actual function being used.- throw new Error('transformReactLynxSync failed'); + throw new Error('transformReactLynx failed');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
CONTRIBUTING.md(1 hunks)packages/react/package.json(1 hunks)packages/react/runtime/__test__/basic.test.jsx(1 hunks)packages/react/runtime/__test__/debug/printSnapshot.test.js(1 hunks)packages/react/runtime/__test__/delayed-lifecycle-events.test.jsx(2 hunks)packages/react/runtime/__test__/hydrate.test.jsx(4 hunks)packages/react/runtime/__test__/lifecycle.test.jsx(3 hunks)packages/react/runtime/__test__/lifecycle/reload.test.jsx(16 hunks)packages/react/runtime/__test__/list.test.jsx(24 hunks)packages/react/runtime/__test__/lynx/timing.test.jsx(4 hunks)packages/react/runtime/__test__/mainThreadLazy.test.jsx(1 hunks)packages/react/runtime/__test__/preact.test.jsx(10 hunks)packages/react/runtime/__test__/renderToOpcodes.test.jsx(13 hunks)packages/react/runtime/__test__/snapshot/event.test.jsx(4 hunks)packages/react/runtime/__test__/snapshot/ref.test.jsx(10 hunks)packages/react/runtime/__test__/snapshot/workletEvent.test.jsx(4 hunks)packages/react/runtime/__test__/snapshot/workletRef.test.jsx(1 hunks)packages/react/runtime/__test__/snapshotPatch.test.jsx(9 hunks)packages/react/runtime/__test__/ssr.test.jsx(10 hunks)packages/react/runtime/__test__/utils/globals.js(1 hunks)packages/react/runtime/vitest.config.ts(1 hunks)packages/react/testing-library/loaders/jsx-loader.js(2 hunks)packages/react/testing-library/src/__tests__/act.test.jsx(4 hunks)packages/react/testing-library/src/__tests__/end-to-end.test.jsx(4 hunks)packages/react/testing-library/src/__tests__/list.test.jsx(9 hunks)packages/react/testing-library/src/__tests__/worklet.test.jsx(5 hunks)packages/react/testing-library/src/vitest.config.js(2 hunks)packages/react/testing-library/turbo.json(1 hunks)packages/react/transform/__test__/css/__snapshots__/transform-all-css-scoped-all.js(1 hunks)packages/react/transform/__test__/css/__snapshots__/transform-all-css-scoped-modules.js(1 hunks)packages/react/transform/__test__/css/__snapshots__/transform-all-css-scoped-none.js(2 hunks)packages/react/transform/__test__/css/__snapshots__/transform-imports-css-scoped-all.js(2 hunks)packages/react/transform/__test__/css/__snapshots__/transform-imports-css-scoped-modules.js(2 hunks)packages/react/transform/__test__/css/__snapshots__/transform-imports-css-scoped-none.js(2 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-all-lazy-bundle.js(2 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-all.js(4 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-modules-lazy-bundle.js(2 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-modules.js(4 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-none-lazy-bundle.js(2 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-css-scoped-none.js(4 hunks)packages/react/transform/__test__/css/__snapshots__/transform-jsx-defaults.js(4 hunks)packages/react/transform/__test__/css/css-scope.spec.js(1 hunks)packages/react/transform/__test__/css/object-lit-style.spec.js(8 hunks)packages/react/transform/__test__/fixture.spec.js(44 hunks)packages/react/transform/index.d.ts(1 hunks)packages/react/transform/main.js(1 hunks)packages/react/transform/package.json(1 hunks)packages/react/transform/rslib.config.ts(1 hunks)packages/react/transform/swc-plugin-reactlynx/index.d.ts(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/lib.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_css_scope/mod.rs(10 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_snapshot/mod.rs(34 hunks)packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_worklet/mod.rs(57 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_css_scope/mod.rs/scoped_all_transform_imports.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_css_scope/mod.rs/scoped_modules_transform_imports.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling_jsx.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling_jsx_dev.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_event.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_expr_container.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_expr_container_with_static_sibling.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_list.js(4 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_list_with_fragment.js(3 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_ref.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_spread.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_timing_flag.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/gesture.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/inline_style_literal.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/inline_style_literal_unknown_property.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/mode_development_spread.js(3 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/page_component.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/page_element.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/page_element_dev.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_escape_newline_character.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_extract_css_id.js(4 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_extract_css_id_dynamic_component.js(4 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_extract_css_id_dynamic_component_without_css_id.js(4 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_extract_css_id_without_css_id.js(4 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_inject_implicit_flatten.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_static_extract_dynamic_inline_style.js(4 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_static_extract_inline_style.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/should_wrap_dynamic_key.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/worklet.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/class_in_worklet_1.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_extract_ident_from_this_js.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_extract_ident_from_this_lepus.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_extract_idents_outside_of_ctx.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_2_js.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_3_js.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_4_js.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_js.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_not_capture_type_annotations.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_not_extract_catch_clause_params.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_not_extract_ident_from_this.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_not_extract_idents_inside_of_ctx.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_transform_fn_decl_js.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_transform_js_fn_in_run_on_js_js.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_transform_js_fn_in_run_on_js_lepus.js(2 hunks)packages/react/transform/swc-plugin-reactlynx/tests/__swc_snapshots__/src/swc_plugin_worklet/mod.rs/should_transform_ui_worklet_js.js(1 hunks)packages/react/transform/turbo.json(1 hunks)packages/react/turbo.json(1 hunks)packages/rspeedy/plugin-react/api-extractor.json(1 hunks)packages/rspeedy/plugin-react/package.json(1 hunks)packages/rspeedy/plugin-react/src/index.ts(1 hunks)packages/rspeedy/plugin-react/src/loaders.ts(5 hunks)
⛔ Files not processed due to max files limit (13)
- packages/rspeedy/plugin-react/src/pluginReactLynx.ts
- packages/rspeedy/plugin-react/src/swc.ts
- packages/rspeedy/plugin-react/test/basic.test.ts
- packages/rspeedy/plugin-react/test/config.test.ts
- packages/rspeedy/plugin-react/test/swc-config.test.ts
- packages/webpack/react-refresh-webpack-plugin/test/hotCases/hook/useContext/snapshot/rspack/1.snap.txt
- packages/webpack/react-refresh-webpack-plugin/test/hotCases/hook/useState/snapshot/rspack/1.snap.txt
- packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/basic/snapshot/rspack/1.snap.txt
- packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/basic/snapshot/rspack/2.snap.txt
- packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/recovery/snapshot/rspack/2.snap.txt
- packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/value/snapshot/rspack/1.snap.txt
- packages/webpack/react-refresh-webpack-plugin/test/hotCases/jsx/value/snapshot/rspack/2.snap.txt
- packages/webpack/react-webpack-plugin/package.json
✅ Files skipped from review due to trivial changes (84)
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_extract_idents_outside_of_ctx.js
- packages/react/runtime/test/mainThreadLazy.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_transform_fn_decl_js.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_2_js.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_css_scope/mod.rs/scoped_modules_transform_imports.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_transform_js_fn_in_run_on_js_js.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/worklet.js
- packages/react/runtime/test/basic.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/inline_style_literal_unknown_property.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_4_js.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_extract_ident_from_this_js.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_escape_newline_character.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_event.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_timing_flag.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_not_extract_ident_from_this.js
- packages/rspeedy/plugin-react/api-extractor.json
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_not_extract_catch_clause_params.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/mode_development_spread.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/page_element.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_transform_ui_worklet_js.js
- packages/react/runtime/test/hydrate.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/gesture.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_transform_js_fn_in_run_on_js_lepus.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/inline_style_literal.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling_jsx.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_component.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_css_scope/mod.rs/scoped_all_transform_imports.js
- packages/react/runtime/test/preact.test.jsx
- packages/react/runtime/test/debug/printSnapshot.test.js
- packages/react/runtime/test/lifecycle.test.jsx
- packages/react/runtime/test/list.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_expr_container.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_full_static.js
- packages/react/runtime/test/ssr.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_ref.js
- packages/react/runtime/test/lynx/timing.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_list_with_fragment.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_3_js.js
- packages/react/testing-library/src/tests/act.test.jsx
- packages/react/runtime/test/utils/globals.js
- packages/react/transform/test/css/snapshots/transform-jsx-css-scoped-all.js
- packages/react/turbo.json
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_not_capture_type_annotations.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_list.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_expr_container_with_static_sibling.js
- packages/react/runtime/test/snapshot/workletRef.test.jsx
- packages/react/transform/test/css/snapshots/transform-jsx-css-scoped-none.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_inject_implicit_flatten.js
- packages/react/runtime/test/delayed-lifecycle-events.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_component_with_static_sibling_jsx_dev.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/page_component.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_not_extract_idents_inside_of_ctx.js
- packages/react/runtime/test/renderToOpcodes.test.jsx
- packages/react/testing-library/src/tests/list.test.jsx
- packages/react/transform/test/css/snapshots/transform-all-css-scoped-modules.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/class_in_worklet_1.js
- packages/react/transform/test/css/snapshots/transform-imports-css-scoped-modules.js
- packages/react/transform/test/css/snapshots/transform-all-css-scoped-all.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_extract_ident_from_this_lepus.js
- packages/react/transform/test/css/snapshots/transform-all-css-scoped-none.js
- packages/react/transform/test/css/snapshots/transform-imports-css-scoped-none.js
- packages/react/testing-library/src/tests/end-to-end.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/basic_spread.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_extract_css_id_without_css_id.js
- packages/react/transform/test/css/snapshots/transform-jsx-defaults.js
- packages/react/transform/test/css/snapshots/transform-imports-css-scoped-all.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_worklet/mod.rs/should_extract_member_expr_js.js
- packages/react/transform/rslib.config.ts
- packages/rspeedy/plugin-react/src/index.ts
- packages/react/runtime/test/snapshot/ref.test.jsx
- packages/react/runtime/test/snapshot/workletEvent.test.jsx
- packages/react/testing-library/src/tests/worklet.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_static_extract_inline_style.js
- packages/react/runtime/test/snapshot/event.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_extract_css_id.js
- packages/react/transform/test/css/snapshots/transform-jsx-css-scoped-modules.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/page_element_dev.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_wrap_dynamic_key.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_extract_css_id_dynamic_component_without_css_id.js
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_static_extract_dynamic_inline_style.js
- packages/react/runtime/test/snapshotPatch.test.jsx
- packages/react/transform/swc-plugin-reactlynx/tests/swc_snapshots/src/swc_plugin_snapshot/mod.rs/should_extract_css_id_dynamic_component.js
- packages/react/runtime/test/lifecycle/reload.test.jsx
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/react/transform/test/css/snapshots/transform-jsx-css-scoped-none-lazy-bundle.js
- packages/react/transform/test/css/snapshots/transform-jsx-css-scoped-modules-lazy-bundle.js
- packages/react/transform/test/css/snapshots/transform-jsx-css-scoped-all-lazy-bundle.js
- packages/react/transform/turbo.json
- packages/react/transform/test/css/object-lit-style.spec.js
- packages/react/transform/test/css/css-scope.spec.js
- packages/react/transform/package.json
- packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_css_scope/mod.rs
- packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_worklet/mod.rs
- packages/react/transform/swc-plugin-reactlynx/index.d.ts
- packages/react/package.json
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/rspeedy/plugin-react/package.json (3)
Learnt from: colinaaa
PR: #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.
Learnt from: colinaaa
PR: #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.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/react/testing-library/loaders/jsx-loader.js (1)
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/react/runtime/vitest.config.ts (1)
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/react/testing-library/src/vitest.config.js (4)
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-mainthread-apis/src/createMainThreadGlobalThis.ts:214-217
Timestamp: 2025-07-16T06:25:41.055Z
Learning: In the lynx-stack codebase, CSS strings produced by genCssContent are considered trusted developer input, so additional sanitization/escaping is not required.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T05:57:29.837Z
Learning: In the lynx-stack codebase, PupilTong prefers avoiding regex for parsing in performance-critical code paths like SSR hydration, preferring simple string manipulation operations even if they're less robust, when the input format is predictable and controlled.
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
packages/react/transform/__test__/fixture.spec.js (5)
Learnt from: colinaaa
PR: #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.
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
Learnt from: PupilTong
PR: #1292
File: packages/web-platform/web-core-server/src/createLynxView.ts:144-151
Timestamp: 2025-07-15T10:00:56.154Z
Learning: In the lynx-stack codebase, PupilTong prefers the "let it crash" approach over defensive null safety checks when the condition should never occur in normal operation. This applies to cases like the element.getAttribute(lynxUniqueIdAttribute)! call in SSR event capture where the attribute is expected to always be present.
packages/react/transform/main.js (2)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_snapshot/mod.rs (1)
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
packages/react/transform/index.d.ts (2)
Learnt from: colinaaa
PR: #1238
File: packages/react/runtime/src/debug/component-stack.ts:70-90
Timestamp: 2025-07-18T04:27:18.291Z
Learning: The component-stack.ts file in packages/react/runtime/src/debug/component-stack.ts is a direct fork from https://github.com/preactjs/preact/blob/main/debug/src/component-stack.js. The team prefers to keep it aligned with the upstream Preact version and may contribute improvements back to Preact in the future.
Learnt from: PupilTong
PR: #1029
File: packages/web-platform/web-core/src/uiThread/createRenderAllOnUI.ts:95-99
Timestamp: 2025-07-16T06:28:26.463Z
Learning: In the lynx-stack codebase, CSS selectors in SSR hydration are generated by their own packages, ensuring a predictable format that makes simple string manipulation safe and preferable over regex for performance reasons.
🧬 Code Graph Analysis (2)
packages/react/runtime/vitest.config.ts (1)
packages/react/transform/main.js (1)
transformReactLynx(18-101)
packages/react/transform/index.d.ts (3)
packages/react/transform/swc-plugin-reactlynx/index.d.ts (6)
CssScopeVisitorConfig(1-4)JsxTransformerConfig(192-201)DirectiveDceVisitorConfig(58-61)WorkletVisitorConfig(202-215)DynamicImportVisitorConfig(62-67)InjectVisitorConfig(101-106)packages/rspeedy/plugin-react/src/index.ts (3)
ShakeVisitorConfig(22-22)CompatVisitorConfig(16-16)DefineDceVisitorConfig(20-20)packages/react/transform/main.js (1)
transformReactLynx(18-101)
🔇 Additional comments (13)
CONTRIBUTING.md (1)
16-16: LGTM! Correct WASM target update.The change from
wasm32-unknown-unknowntowasm32-wasip1correctly aligns with the new SWC plugin architecture that targets the WASI environment. This ensures contributors install the proper WASM target for building the new WebAssembly-based plugins.packages/rspeedy/plugin-react/package.json (1)
51-52: LGTM! Correct dependency migration.The addition of the new SWC plugin dependencies (
@lynx-js/swc-plugin-reactlynx-compatand@lynx-js/swc-plugin-reactlynx) properly reflects the migration from the monolithic@lynx-js/react-transformpackage to modular WebAssembly-based SWC plugins.packages/react/testing-library/turbo.json (1)
7-7: LGTM! Correct build dependency update.Changing from
build:wasmtobuildaligns with the new transform architecture where WASM functionality has been moved to dedicated SWC plugin packages, eliminating the need for separate WASM build targets in the main transform package.packages/react/testing-library/loaders/jsx-loader.js (1)
1-32: LGTM! Correct async migration with one nitpick.The migration from synchronous to asynchronous transform is well-implemented:
- Proper use of
this.async()for webpack async loader pattern- Correct await usage for the new async
transformReactLynxfunction- Appropriate simplification of transform options
packages/react/runtime/vitest.config.ts (1)
15-39: LGTM! Excellent async migration with proper syntax detection.The migration to the async
transformReactLynxfunction is well-implemented:
- Correct async/await pattern for the transform function
- Robust syntax detection logic that properly handles TypeScript, TSX, and ECMAScript files
- Appropriate
syntaxConfigconfiguration that aligns with the new transform API- Clean simplification of transform options removing deprecated parameters
The syntax detection using file extensions is reliable and the configuration options (decorators, tsx, jsx) are correctly set based on the file type.
packages/react/transform/__test__/fixture.spec.js (1)
1-1366: Test suite updates look good!The test file has been properly updated to align with the new async transform API:
- Correctly migrated from synchronous to asynchronous transform calls
- Removed esbuild dependencies and directly checks error/warning arrays
- Updated configuration objects to match the new API structure
- Properly updated snapshot assertions to match the new output format
packages/react/transform/swc-plugin-reactlynx/src/swc_plugin_snapshot/mod.rs (2)
987-998: Clean migration to serde-based configurationThe migration from
#[napi(object)]to#[derive(Deserialize)]with camelCase field naming is well-executed. Removingpreserve_jsxandfilenamefrom the config struct improves separation of concerns.
1038-1065: Confirm removal ofvisit_mut_exprin Snapshot Transformer
It’s expected that the snapshot plugin no longer implements a customvisit_mut_expr; its snapshot serialization now occurs via explicit calls in the JSX transformer rather than an AST-wide visitor. No additionalvisit_mut_exprimplementation is present or needed inpackages/react/transform/swc-plugin-reactlynx/src/swc_plugin_snapshot/mod.rs. All existingvisit_mut_exprmethods remain in other swc‐plugin crates (compat, worklet, extract_str, inject, css_scope), so no migration is required.packages/react/transform/main.js (1)
5-16: LGTM!The ESM imports and WASM plugin path resolution are correctly implemented.
packages/react/transform/swc-plugin-reactlynx/src/lib.rs (1)
1-368: Well-structured SWC plugin implementation!The plugin architecture is excellent with:
- Clear separation of visitor modules
- Flexible configuration options using the
Eitherenum- Proper error handling and marks management
- Comprehensive transform pipeline with multiple passes
The code follows Rust best practices and integrates well with the SWC ecosystem.
packages/rspeedy/plugin-react/src/loaders.ts (2)
55-127: Excellent refactoring of loader configuration!The extraction of SWC loader options into helper functions improves code organization and maintainability. The separation between main thread and background configurations is clear and well-structured.
158-164: Correct plugin ordering with clear documentation!The explicit placement of the compat plugin before the reactlynx plugin is properly implemented with clear comments explaining the requirement. This ensures the transformation pipeline works correctly.
Also applies to: 191-197
packages/react/transform/index.d.ts (1)
1-54: Clean and well-organized type definitions!The simplified type definitions properly:
- Import visitor configs from the new plugin packages
- Use
SwcLoaderParserConfigfrom @rspack/core for syntax configuration- Define clear input/output interfaces for the transform function
- Correctly type errors and warnings as string arrays (aligning with the need to stringify errors)
The modular approach with type imports improves maintainability.
| const isTS = /\.[mc]?ts$/.exec(relativePath); | ||
| const isTSX = /\.tsx$/.exec(relativePath); | ||
| const syntax = (isTS || isTSX) ? 'typescript' : 'ecmascript'; | ||
| const syntaxConfig = { | ||
| syntax, | ||
| decorators: true, | ||
| tsx: !isTS, | ||
| jsx: true, | ||
| }; |
There was a problem hiding this comment.
Fix incorrect TSX flag logic
The tsx flag is incorrectly set to !isTS, which would be true for both JavaScript files and TSX files. TSX files should have tsx: true.
Apply this fix:
-const isTS = /\.[mc]?ts$/.exec(relativePath);
-const isTSX = /\.tsx$/.exec(relativePath);
+const isTS = /\.[mc]?ts$/.test(relativePath);
+const isTSX = /\.tsx$/.test(relativePath);
const syntax = (isTS || isTSX) ? 'typescript' : 'ecmascript';
const syntaxConfig = {
syntax,
decorators: true,
- tsx: !isTS,
+ tsx: isTSX,
jsx: true,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isTS = /\.[mc]?ts$/.exec(relativePath); | |
| const isTSX = /\.tsx$/.exec(relativePath); | |
| const syntax = (isTS || isTSX) ? 'typescript' : 'ecmascript'; | |
| const syntaxConfig = { | |
| syntax, | |
| decorators: true, | |
| tsx: !isTS, | |
| jsx: true, | |
| }; | |
| const isTS = /\.[mc]?ts$/.test(relativePath); | |
| const isTSX = /\.tsx$/.test(relativePath); | |
| const syntax = (isTS || isTSX) ? 'typescript' : 'ecmascript'; | |
| const syntaxConfig = { | |
| syntax, | |
| decorators: true, | |
| tsx: isTSX, | |
| jsx: true, | |
| }; |
🤖 Prompt for AI Agents
In packages/react/testing-library/src/vitest.config.js around lines 55 to 63,
the tsx flag is incorrectly set to !isTS, causing it to be true for JavaScript
files as well. Change the tsx flag to be true only when the file is a TSX file
by setting tsx: !!isTSX. This ensures tsx is true only for TSX files and false
otherwise.
| ...options, | ||
| jsc: { | ||
| ...options.jsc, | ||
| target: 'es2019', |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistent ES target versions across the codebase.
The main thread loader sets the target to 'es2019' while packages/react/transform/main.js uses 'es2022'. This inconsistency could lead to different transformation outputs. Please align the ES target versions or document why different targets are needed.
🏁 Script executed:
#!/bin/bash
# Search for all ES target configurations in the codebase
echo "Searching for ES target configurations..."
rg -A 2 -B 2 "target.*:.*['\"]es\d{4}" --type ts --type jsLength of output: 10504
🏁 Script executed:
#!/bin/bash
# Show lines around the ES target in plugin-react loader
rg -n "target.*es2019" -C3 packages/rspeedy/plugin-react/src/loaders.tsLength of output: 253
Align ES target versions between plugin-react loader and main transformer
The plugin-react loader in packages/rspeedy/plugin-react/src/loaders.ts defaults to
jsc: {
…,
target: 'es2019',
…
}(around line 144), whereas the standalone transform in packages/react/transform/main.js sets
target: 'es2022'for its jsc options.
This mismatch can lead to different code output depending on how SWC is invoked. Please either:
- Unify both defaults to the same ES target (e.g. upgrade the loader default to
es2022), or - Add a comment/doc explaining why the loader intentionally targets an older version (
es2019).
Files to update:
• packages/rspeedy/plugin-react/src/loaders.ts (line 144)
• packages/react/transform/main.js (where target: 'es2022' is declared)
🤖 Prompt for AI Agents
In packages/rspeedy/plugin-react/src/loaders.ts at line 144, the ES target is
set to 'es2019', which conflicts with the 'es2022' target in
packages/react/transform/main.js. To fix this, update the loader's jsc target to
'es2022' to unify the ES target versions across both files, ensuring consistent
code output when SWC is invoked.
<!-- Thank you for submitting a pull request! We appreciate the time and effort you have invested in making these changes. Please ensure that you provide enough information to allow others to review your pull request. Upon submission, your pull request will be automatically assigned with reviewers. If you want to learn more about contributing to this project, please visit: https://github.com/lynx-family/lynx-stack/blob/main/CONTRIBUTING.md. --> <!-- Can you explain the reasoning behind implementing this change? What problem or issue does this pull request resolve? --> <!-- It would be helpful if you could provide any relevant context, such as GitHub issues or related discussions. --> <!--- Check and mark with an "x" --> - [ ] Tests updated (or not required). - [ ] Documentation updated (or not required). chore: update Cargo.toml
gaoachao
force-pushed
the
refactor/adopt-swc-plugin
branch
from
5ed47e6 to
305ece3
Compare
Summary
Rspack v1.3.12 introduces support for
rspack.experiments.swc.transformand includes several bug fixes, enabling the use of WASM plugins and exposingdiagnosticsin transformResult.In this PR, @lynx-js/react-transform has been refactored into three separate packages:
transformReactLynxbased onrspack.experiments.swc.transforminmain.js(TODO: migrate to rslib with better TypeScript support)All tests have passed successfully, including both Rust-side tests and Node.js-side tests in
packages/react/transform.Changes to
PluginReactLynxandReactWebpackPluginwill be addressed in subsequent PRs.Checklist
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
Documentation
Tests