refactor: add swc-plugin-reactlynx-compat
#1744
Conversation
|
📝 WalkthroughWalkthroughAdds a new SWC React‑Lynx compatibility plugin (Rust + WASM), TypeScript declarations, build/turbo config and npm workspace inclusion, expanded repo ignore/exclude globs, and many new SWC test snapshots covering JSX transformations, event props, component wrapping, and constructor simplification. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Web Explorer#5235 Bundle Size — 364.39KiB (0%).0fb02d3(current) vs 588819b main#5227(baseline) Bundle metrics
|
| Current #5235 |
Baseline #5227 |
|
|---|---|---|
 Initial JS |
145.41KiB |
145.41KiB |
| Initial CSS |
31.89KiB |
31.89KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
218(-0.46%) |
219 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
3.37% |
3.37% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5235 |
Baseline #5227 |
|
|---|---|---|
| JS |
238.48KiB |
238.48KiB |
| Other |
94.02KiB |
94.02KiB |
| CSS |
31.89KiB |
31.89KiB |
Bundle analysis report Branch refactor/add-swc-plugin-reactlyn... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#5242 Bundle Size — 237.52KiB (0%).0fb02d3(current) vs 588819b main#5234(baseline) Bundle metrics
|
| Current #5242 |
Baseline #5234 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
162 |
162 |
| Duplicate Modules |
65 |
65 |
| Duplicate Code |
46.71% |
46.71% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #5242 |
Baseline #5234 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.76KiB |
91.76KiB |
Bundle analysis report Branch refactor/add-swc-plugin-reactlyn... Project dashboard
Generated by RelativeCI Documentation Report issue
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react/transform/swc-plugin-reactlynx/src/lib.rs (1)
1-2: Empty plugin entrypoint — add a no-op transform or remove the crate from the workspace.File: packages/react/transform/swc-plugin-reactlynx/src/lib.rs — this crate is listed in the workspace but exports nothing; add a minimal stub or remove the crate.
Suggested minimal stub:
+// Copyright 2025 The Lynx Authors. All rights reserved. +use swc_core::ecma::ast::Program; +use swc_core::plugin::{plugin_transform, proxies::TransformPluginProgramMetadata}; + +#[plugin_transform] +pub fn transform(program: Program, _metadata: TransformPluginProgramMetadata) -> Program { + program +}
🧹 Nitpick comments (29)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_left_no_remove_component_element_attr.js (1)
1-1: Minor formatting consistency.Ensure snapshots follow the repo’s formatting convention (semicolon usage and trailing newline) consistently across all new snapshots added in this PR.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.js (1)
5-5: Clarify the intent of the “__a” edge caseIf
__ais intentionally treated as non-component for legacy compat, consider adding a brief note in the test source explaining that, or add an explicit lowercase tag case (e.g.,<a .../>) in a separate test to make the distinction obvious.I can draft a small companion test that asserts attrs are retained on lowercase intrinsic elements.
biome.jsonc (2)
38-38: Replace "REMOVE ME" with a tracked TODO.Use a stable TODO with an issue link to avoid lingering markers in config.
- // REMOVE ME + // TODO(maintainers): Temporary ignore for generated d.ts until lint rules are adjusted. Track in #<issue-id>.
42-42: Redundant specific ignore entry; wildcard already covers it."packages/react/transform/index.d.ts" above is now redundant given the /**/ pattern. Consider removing the specific line to reduce drift.
packages/react/transform/swc-plugin-reactlynx/.cargo/config.toml (1)
1-4: Alias looks good; consider WASM size/perf release tweaks.Optional: set a release profile (in the crate or workspace Cargo.toml) to reduce .wasm size and improve load times.
Add to Cargo.toml (not this file):
[profile.release] lto = "fat" codegen-units = 1 opt-level = "s" panic = "abort"packages/react/transform/swc-plugin-reactlynx-compat/.cargo/config.toml (1)
1-4: Cargo alias is fine; consider deduping at workspace level.Placing this alias in a root
.cargo/config.tomlwould let both plugins share it and avoid drift.packages/react/.dprint.jsonc (1)
15-20: Deduplicate snapshot ignore globs.
transform/tests/__swc_snapshots__/**is covered by the recursive glob below; remove the specific entry.Apply:
- "transform/tests/__swc_snapshots__/**",eslint.config.js (2)
47-53: Consolidate snapshot ignores to recursive patterns.Avoid duplicate specific globs; keep only the recursive variants.
Apply:
- // REMOVE ME - 'packages/react/transform/tests/__swc_snapshots__/**', - 'packages/react/transform/__test__/**/__snapshots__/**', - - 'packages/react/transform/**/tests/__swc_snapshots__/**', + // Snapshots (recursive) + 'packages/react/transform/**/tests/__swc_snapshots__/**', + 'packages/react/transform/**/__test__/**/__snapshots__/**',
71-76: Trim redundantindex.d.tsignore.The recursive ignore makes the specific one unnecessary.
Apply:
- 'packages/react/transform/index.d.ts', 'packages/react/transform/index.cjs', 'packages/react/transform/**/index.d.ts',packages/react/transform/swc-plugin-reactlynx/Cargo.toml (2)
20-20: Trim__testing_transformfrom release feature set (optional).
__testing_transformis test-only; carrying it in the main feature list increases compile time. Consider moving it behinddev-dependenciesor a crate feature used only in tests.-swc_core = { workspace = true, 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"] } +swc_core = { workspace = true, 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", "ecma_plugin_transform"] }If tests rely on it, add under
[dev-dependencies] swc_corewithfeatures = ["__testing_transform"].
18-19: Align serde_json default-features across reactlynx plugins (optional)swc-plugin-reactlynx currently has serde_json = { workspace = true } while swc-plugin-reactlynx-compat sets default-features = false and packages/react/transform/Cargo.toml uses features = ["preserve_order"]. Either add default-features = false to swc-plugin-reactlynx to match compat (recommended for wasm size/behavior parity) or document why this crate intentionally differs. Files: packages/react/transform/Cargo.toml, packages/react/transform/swc-plugin-reactlynx/Cargo.toml, packages/react/transform/swc-plugin-reactlynx-compat/Cargo.toml.
packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs (2)
36-37: Compile the regex once; current code recompiles per constructor.Use a static
Lazy<Regex>to avoid repeated compilation and reduce overhead.-use regex::Regex; +use once_cell::sync::Lazy; +use regex::Regex; +static RE_FLAG_IDENT: Lazy<Regex> = Lazy::new(|| Regex::new(r"^__[A-Z_]+__$").unwrap()); @@ - let re = Regex::new(r"^__[A-Z_]+__$").unwrap(); for stmt in &body.stmts { @@ - if re.captures(test_ident.sym.as_str()).is_some() { + if RE_FLAG_IDENT.is_match(test_ident.sym.as_str()) { self.remain_stmts.push(stmt.clone()) }
115-127: Detect more dynamic patterns in state props.
- Arrow functions are not explicitly flagged; rely on traversal is brittle. Mark them directly.
- Treat
new Foo()as dynamic similar to calls.struct CallExprDetectVisitor { pub has: bool, } @@ impl Visit for CallExprDetectVisitor { fn visit_call_expr(&mut self, _n: &CallExpr) { self.has = true; } + fn visit_new_expr(&mut self, _n: &NewExpr) { + self.has = true; + } } @@ struct FuncDetectVisitor { pub has: bool, } @@ impl Visit for FuncDetectVisitor { fn visit_function(&mut self, _n: &Function) { self.has = true; } + fn visit_arrow_expr(&mut self, _n: &ArrowExpr) { + self.has = true; + } fn visit_block_stmt_or_expr(&mut self, _n: &BlockStmtOrExpr) { self.has = true; } }Also applies to: 139-157
packages/react/transform/swc-plugin-reactlynx-compat/src/is_component_class.rs (1)
21-27: Parity: mark JSX usage viacreateSnapshotInstance()calls.In other transforms, this call is treated as JSX presence. Add a
visit_call_exprto sethas_jsx = truewhen callee ident iscreateSnapshotInstance.impl Visit for TransformVisitor { fn visit_jsx_element(&mut self, _n: &JSXElement) { self.has_jsx = true; } fn visit_jsx_fragment(&mut self, _n: &JSXFragment) { self.has_jsx = true; } + fn visit_call_expr(&mut self, n: &CallExpr) { + if let Callee::Expr(callee) = &n.callee { + if let Expr::Ident(ident) = &**callee { + if ident.sym == *"createSnapshotInstance" { + self.has_jsx = true; + return; + } + } + } + n.visit_children_with(self); + }packages/react/transform/swc-plugin-reactlynx-compat/index.d.ts (3)
62-65: Deduplicate the repeated target union with a shared alias.Improves maintainability and consistency with the Rust enum.
Within these ranges, replace the inline union with an alias:
- target: 'LEPUS' | 'JS' | 'MIXED'; + target: TransformTarget;Add once near the top (outside these ranges):
export type TransformTarget = 'LEPUS' | 'JS' | 'MIXED';Also applies to: 197-206, 218-218
217-220: Align visibility forruntimePkgin Worklet config.
DynamicImportVisitorConfig.runtimePkgis marked@internalbut Worklet’s isn’t. Either make it internal too or document it.customGlobalIdentNames?: Array<string>; /** @internal */ target: 'LEPUS' | 'JS' | 'MIXED'; - runtimePkg: string; + /** @internal */ + runtimePkg: string;
60-60: Consider allowing non-string literals indefineDCE.define.Most define systems accept booleans/numbers. If you intend “as-code” replacement, keep
string; otherwise broaden:- define: Record<string, string>; + define: Record<string, string | number | boolean>;If kept as
string, please add a note clarifying that values must be stringified code.packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_1.js (1)
1-1: Consider making this case distinct from _2 or justify duplication in test nameIf the goal is coverage breadth, tweak one snapshot to exercise a different event prop combination; otherwise clarify via a comment in the test that duplication is intentional.
Apply if you want to diversify:
-const a = <view bindtap catchtap/>; +const a = <view bindtap catchtap catchtouchstart/>;packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.js (1)
1-3: Prefer named import for __ComponentIsPolyfill to improve tree-shakingExport is a named const (packages/react/runtime/src/internal.ts:54) — switch to a named import.
-import * as ReactLynx from "@lynx-js/react/internal"; -let c = <ReactLynx.__ComponentIsPolyfill is="xxxx"/>; +import { __ComponentIsPolyfill } from "@lynx-js/react/internal"; +let c = <__ComponentIsPolyfill is="xxxx"/>;packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded.js (1)
1-2: LGTM — PURE hints present; consider hoisting identical renderers to reduce duplicationBoth wrappers use the same (__c)=>{__c} renderer. A future transform optimization could hoist this to a single const to reduce code size. Snapshot is fine as-is.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js (1)
1-4: LGTM — good recursive coverage of mixed intrinsic and custom elementsNice mix of lowercase intrinsic (view/scroll-view) and capitalized View with different event props.
Optionally add a captured event (e.g., capturetap) in one node to cover that branch too.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.js (1)
2-3: Duplicate snapshot entries — intentional?Lines 2 and 3 are identical. If not meant to probe idempotency, consider removing one to reduce noise.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_compiler_only.js (1)
9-10: Potential duplicate snapshot lines.Lines 9 and 10 are identical. If not deliberate, drop one to avoid brittle snapshots.
packages/react/transform/swc-plugin-reactlynx-compat/build.js (1)
21-27: Improve error reporting if the wasm artifact is missing.Wrap copy in a try/catch to surface a clear message that points to the expected path.
-await fs.copyFile( +try { + await fs.copyFile( path.resolve( __dirname, '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx_compat.wasm', ), path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm'), -); + ); +} catch (err) { + console.error('Failed to locate swc_plugin_reactlynx_compat.wasm. Did the Wasi build succeed?'); + throw err; +}packages/react/transform/swc-plugin-reactlynx-compat/package.json (1)
1-22: Declare Node engine to match ESM + top-level await usage.
build.jsrelies on ESM and top-level await; pin an engine for deterministic CI."scripts": { "build": "node ./build.js", "test:cargo": "cargo test" - } + }, + "engines": { + "node": ">=18.17" + }Given the package is
"private": true, no changeset is required (per team learning), but ensure workspace tooling picks up the new package.packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs (2)
12-27: Consider also implementing Serialize/Display (optional).Adding
SerializeandDisplay/FromStrcan simplify logging and CLI/config plumbing; not required here.-#[derive(Debug, PartialEq, Clone, Copy)] +#[derive(Debug, PartialEq, Clone, Copy, serde::Serialize)] pub enum TransformTarget {
22-25: Error message is fine; optional: list allowed values.If you want a more actionable error, include the valid variants in the message (will require updating the test).
packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (2)
849-852: Precompile per-pass regexes to reduce hot-path allocations.
Regex::new(...)inside attr loops is expensive. Hoist to statics.Add once statics near the top:
static EVENT_ATTR_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"^(global-bind|bind|catch|capture-bind|capture-catch)([A-Za-z]+)$").unwrap()); static DATA_ATTR_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"^data-([A-Za-z]+)$").unwrap());Then replace the in-loop constructions with
EVENT_ATTR_RE.is_match(...)andDATA_ATTR_RE.is_match(...).Also applies to: 876-879
399-400: Remove unused state to avoid warnings.
is_old_runtime_pkgis set but never read.- is_old_runtime_pkg: bool,And drop related assignments.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
.changeset/deep-ideas-nail.md(1 hunks).gitignore(1 hunks)CODEOWNERS(1 hunks)Cargo.toml(1 hunks)biome.jsonc(1 hunks)eslint.config.js(2 hunks)packages/react/.dprint.jsonc(1 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/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/is_component_class.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_compiler_only.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded_compiler_only.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_left_no_remove_component_element_attr.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_redeclaration.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_rename_view.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_correct_order.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_macro.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_1.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_2.js(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/Cargo.toml(1 hunks)packages/react/transform/swc-plugin-reactlynx/build.js(1 hunks)packages/react/transform/swc-plugin-reactlynx/package.json(1 hunks)packages/react/transform/swc-plugin-reactlynx/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, always generate a changeset and commit the resulting markdown file(s)
Files:
.changeset/deep-ideas-nail.md
🧠 Learnings (14)
📓 Common learnings
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.810Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
📚 Learning: 2025-09-12T09:43:04.810Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.810Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
packages/react/transform/swc-plugin-reactlynx/.gitignorepackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.jspackages/react/transform/swc-plugin-reactlynx/package.jsonpackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded.jsCargo.tomlpackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.jspackages/react/transform/swc-plugin-reactlynx-compat/.gitignorepackages/react/transform/swc-plugin-reactlynx/Cargo.tomlpackages/react/transform/swc-plugin-reactlynx-compat/Cargo.tomlpackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.jspackages/react/transform/swc-plugin-reactlynx-compat/package.json
📚 Learning: 2025-09-12T09:43:04.810Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.810Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
packages/react/transform/swc-plugin-reactlynx/.gitignorepackages/react/transform/swc-plugin-reactlynx/package.jsonpackages/react/transform/swc-plugin-reactlynx/src/lib.rspackages/react/transform/swc-plugin-reactlynx-compat/.gitignore.changeset/deep-ideas-nail.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
packages/react/transform/swc-plugin-reactlynx/.gitignorepackages/react/transform/swc-plugin-reactlynx-compat/.gitignore.changeset/deep-ideas-nail.md
📚 Learning: 2025-09-10T11:42:36.855Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/.cargo/config.tomlpackages/react/transform/swc-plugin-reactlynx/.cargo/config.tomlCargo.tomlpackages/react/transform/swc-plugin-reactlynx/Cargo.tomlpackages/react/transform/swc-plugin-reactlynx-compat/Cargo.tomlpackages/react/transform/swc-plugin-reactlynx-compat/src/target.rspackages/react/transform/swc-plugin-reactlynx-compat/turbo.json
📚 Learning: 2025-08-27T12:42:01.095Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1616
File: packages/webpack/cache-events-webpack-plugin/test/cases/not-cache-events/lazy-bundle/index.js:3-3
Timestamp: 2025-08-27T12:42:01.095Z
Learning: In webpack, properties like __webpack_require__.lynx_ce are injected during compilation/build time when webpack processes modules and generates bundles, not at runtime when dynamic imports execute. Tests for such properties don't need to wait for dynamic imports to complete.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.jspackages/react/transform/swc-plugin-reactlynx-compat/build.jspackages/react/transform/swc-plugin-reactlynx/build.js
📚 Learning: 2025-08-12T16:09:32.413Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1497
File: packages/react/transform/tests/__swc_snapshots__/src/swc_plugin_snapshot/mod.rs/basic_full_static.js:9-10
Timestamp: 2025-08-12T16:09:32.413Z
Learning: In the Lynx stack, functions prefixed with `__` that are called in transformed code may be injected globally by the Lynx Engine at runtime rather than exported from the React runtime package. For example, `__CreateFrame` is injected globally by the Lynx Engine, not exported from lynx-js/react.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.js
📚 Learning: 2025-08-13T09:20:00.936Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1502
File: packages/react/testing-library/types/entry.d.ts:71-71
Timestamp: 2025-08-13T09:20:00.936Z
Learning: In lynx-js/react testing library, wrapper components must have children as a required prop because they are always called with `h(WrapperComponent, null, innerElement)` where innerElement is passed as children. The type `React.JSXElementConstructor<{ children: React.ReactNode }>` correctly requires children to be mandatory.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded_compiler_only.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.jseslint.config.js
📚 Learning: 2025-08-19T11:25:36.127Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1558
File: .changeset/solid-squids-fall.md:2-2
Timestamp: 2025-08-19T11:25:36.127Z
Learning: In the lynx-family/lynx-stack repository, changesets should use the exact package name from package.json#name, not generic or unscoped names. Each package has its own specific scoped name (e.g., "lynx-js/react-transform" for packages/react/transform).
Applied to files:
packages/react/transform/swc-plugin-reactlynx/package.jsonpackages/react/transform/swc-plugin-reactlynx-compat/package.json
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_left_no_remove_component_element_attr.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded_compiler_only.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.jspackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.jspackages/react/transform/swc-plugin-reactlynx-compat/src/is_component_class.rspackages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.530Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.js
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.js
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
eslint.config.js
🧬 Code graph analysis (15)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.js (1)
packages/rspeedy/plugin-react/test/fixtures/special-var-name/ReactLynx/index.js (1)
ReactLynx(1-3)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.js (2)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js (4)
a(3-3)a(5-5)a(6-6)View(2-2)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_rename_view.js (1)
a(1-1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.js (1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.js (1)
c(3-3)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.js (3)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.js (1)
c(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls.js (1)
c(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_macro.js (1)
c(1-1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_rename_view.js (4)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js (1)
a(1-4)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js (3)
a(3-3)a(5-5)a(6-6)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_1.js (1)
a(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_2.js (1)
a(1-1)
packages/react/transform/swc-plugin-reactlynx-compat/build.js (1)
packages/react/transform/swc-plugin-reactlynx/build.js (2)
__filename(10-10)__dirname(11-11)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded_compiler_only.js (1)
packages/react/transform/src/wasm.js (1)
view(24-28)
packages/react/transform/swc-plugin-reactlynx/build.js (1)
packages/react/transform/swc-plugin-reactlynx-compat/build.js (2)
__filename(10-10)__dirname(11-11)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_redeclaration.js (1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js (4)
View(2-2)a(3-3)a(5-5)a(6-6)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js (4)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js (1)
a(1-4)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.js (1)
a(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_redeclaration.js (1)
a(3-3)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_rename_view.js (1)
a(1-1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js (6)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.js (1)
a(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js (4)
a(3-3)a(5-5)a(6-6)View(2-2)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_redeclaration.js (1)
a(3-3)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_rename_view.js (1)
a(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_1.js (1)
a(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_2.js (1)
a(1-1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_1.js (2)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js (1)
a(1-4)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_2.js (1)
a(1-1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_2.js (1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js (1)
a(1-4)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls.js (4)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.js (1)
c(3-3)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.js (1)
c(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_macro.js (1)
c(1-1)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.js (1)
c(1-1)
packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (3)
packages/react/transform/swc-plugin-reactlynx-compat/src/is_component_class.rs (1)
new(12-18)packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs (1)
new(21-30)packages/react/transform/index.d.ts (1)
DarkModeConfig(47-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (26)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_left_no_remove_component_element_attr.js (1)
1-1: Verify snapshot matches test intent (attributes preserved).Test name suggests component element attributes should be preserved, but the snapshot shows
<Component/>;with no attrs. Please confirm the input actually contains attributes and the transform is a no-op for them. If yes, the snapshot should include those attrs; otherwise, consider renaming the test for clarity.packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.js (2)
2-3: LGTM: component vs host coverage looks correctSnapshot shows attrs removed on components (
<A/>,<A.B/>, and with spread<A {...x}/>), and retained on host (<view .../>). This matches the intent of “remove component attr regex.”Also applies to: 6-6, 7-7
4-4: Edit test source comment and regenerate snapshotUpdate the FIXME in packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (around line 1841) and re-run tests — do not edit the generated snapshot.
-// FIXME: the `is_component` check is align with previous version of ReactLynx for compat reason +// FIXME: the `is_component` check is aligned with the previous ReactLynx behavior for compatibilityCODEOWNERS (1)
4-4: LGTM.Scoping ownership of packages/react/transform/** to @gaoachao is clear and won’t affect existing patterns. If multiple owners are desired, append them here.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.js (1)
1-1: Snapshot addition looks correct.Covers JSXMemberExpression; good guard to ensure no unintended rename/transform.
packages/react/transform/swc-plugin-reactlynx/.gitignore (1)
1-2: Ignore patterns are appropriate.dist and top-level .wasm artifacts should be untracked for this package.
.gitignore (1)
10-10: Do not change.swc→**/.swc;.swcalready ignores nested.swcdirectories.
Git ignore patterns without a slash match names at any depth; root .gitignore already contains.swc(line 10) and a repo scan found no.swcdirs to add.Likely an incorrect or invalid review comment.
packages/react/transform/swc-plugin-reactlynx-compat/.gitignore (1)
1-2: Ignore patterns are appropriate.Matches the build outputs for this package.
.changeset/deep-ideas-nail.md (1)
1-3: ```shell
#!/bin/bash
set -euo pipefailecho "=== Git branch ==="
git rev-parse --abbrev-ref HEAD || true
echoBASE=$(git merge-base origin/main HEAD 2>/dev/null || git merge-base HEAD main 2>/dev/null || true)
echo "=== Merge base ==="
echo "$BASE"
echoecho "=== Changed files (name-status) since merge base ==="
if [ -n "$BASE" ]; then
git diff --name-status "$BASE"..HEAD || true
else
git diff --name-status HEAD~10..HEAD || true
fi
echoecho "=== Changed files (names) ==="
if [ -n "$BASE" ]; then
git diff --name-only "$BASE"..HEAD || true
else
git diff --name-only HEAD~10..HEAD || true
fi
echoecho "=== .changeset/deep-ideas-nail.md content (if exists) ==="
if [ -f ".changeset/deep-ideas-nail.md" ]; then
sed -n '1,200p' .changeset/deep-ideas-nail.md
else
echo "MISSING"
fi
echoecho "=== Listing .changeset directory ==="
ls -la .changeset || true
echoecho "=== package.json files touched by this PR (in changed files) ==="
if [ -n "$BASE" ]; then
git diff --name-only "$BASE"..HEAD | rg '^(./)?package.json$' -n || echo "none"
else
git diff --name-only HEAD~10..HEAD | rg '^(./)?package.json$' -n || echo "none"
fi
echoecho "=== All package.json files in repo (show name, private, version) ==="
if command -v fd >/dev/null 2>&1; then
PKGS=$(fd --hidden --type f 'package.json' || true)
else
PKGS=$(rg --files --hidden -g '!node_modules' 'package.json' || true)
fifor f in $PKGS; do
echo "---- $f ----"
if command -v jq >/dev/null 2>&1; then
jq '{name: .name, private: .private, version: .version}' "$f" || cat "$f"
else
rg '"name"|"private"|"version"' -n "$f" || cat "$f"
fi
done
echoecho "=== Search repo for 'swc-plugin' or '@lynx-js' ==="
rg -n --hidden 'swc-plugin' || true
rg -n --hidden '@lynx-js' || true
echoecho "=== Show package.json for any packages whose name contains 'swc-plugin' ==="
if command -v jq >/dev/null 2>&1; then
for f in $PKGS; do
name=$(jq -r '.name // empty' "$f" 2>/dev/null || true)
if [ -n "$name" ] && echo "$name" | rg -i 'swc-plugin' -q 2>/dev/null; then
echo "---- $f ----"
jq '{name: .name, private: .private, version: .version, description: .description}' "$f" || true
fi
done
fi</blockquote></details> <details> <summary>Cargo.toml (1)</summary><blockquote> `22-22`: **Workspace already enables serde derive — no change needed.** packages/react/transform/Cargo.toml already contains `serde = { workspace = true, features = ["derive"] }`; member crates (packages/react/transform/swc-plugin-reactlynx-compat/Cargo.toml, packages/react/transform/swc-plugin-reactlynx/Cargo.toml) use `serde = { workspace = true }` and will inherit the derive feature. </blockquote></details> <details> <summary>packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_redeclaration.js (1)</summary><blockquote> `1-4`: **LGTM: covers alias-based non-rename scenario.** </blockquote></details> <details> <summary>packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_rename_view.js (1)</summary><blockquote> `1-1`: **LGTM: minimal snapshot for rename case.** </blockquote></details> <details> <summary>packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js (1)</summary><blockquote> `1-6`: **LGTM: exercises scoped capitalized vs lowercase elements.** </blockquote></details> <details> <summary>packages/react/transform/swc-plugin-reactlynx-compat/src/is_component_class.rs (1)</summary><blockquote> `28-50`: ```shell #!/bin/bash set -euo pipefail # Find the target file file=$(rg --hidden --no-ignore --files | rg 'is_component_class.rs' || true) if [ -z "$file" ]; then echo "FILE_NOT_FOUND" echo "Searching for related symbols..." rg -n --hidden --no-ignore -S 'struct\s+IsComponentClass|has_render_method|has_super_class|visit_class' || true exit 0 fi echo "FOUND:$file" echo "----- FILE (head) -----" sed -n '1,240p' "$file" || true echo "----- FILE (rest) -----" sed -n '241,9999p' "$file" || true echo "----- USAGES -----" rg -n --hidden --no-ignore -S 'IsComponentClass|has_render_method|has_super_class' || truepackages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.js (1)
1-1: LGTM — snapshot reflects runtime remap; confirm subpath existsImport path matches the legacy runtime mapping; repository search for '"legacy-react-runtime"' in package.json files produced no matches — confirm the package exports the "legacy-react-runtime" subpath or update the test fixture.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor.js (1)
1-9: LGTM — snapshot captures ctor simplification shape.Matches expectations for class fields and minimal render.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls.js (1)
1-9: LGTM — ensures local decls don’t linger in state init.Good targeted coverage for the “remain local decls” case.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_2.js (1)
1-1: Duplicate snapshot — confirm intent or diversifyThese snapshots are identical:
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_transform_event_props_1.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_transform_event_props_2.js
If they should exercise different inputs, change one (e.g., use a different event like catchtouchstart or a non-boolean value); otherwise collapse into a single test.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded_compiler_only.js (1)
1-1: LGTM — matches “compiler_only” expectation (no runtime wrapper)Snapshot shows pure element nesting, consistent with a compiler-only pathway. Confirm the paired “embedded” test covers the runtime-wrapped variant.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_macro.js (1)
6-18: Macro identifiers look inconsistent — confirm spellings are intentionalrg finds both LEPUS and LE_P_US only in the snapshot file packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_macro.js; if unintentional, unify to a single macro in the source and regenerate the snapshot; if intentional, add a short comment in the test explaining the distinction.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_correct_order.js (1)
5-9: Snapshot correctly asserts last-write-wins for duplicate keys.The duplicate "a" property verifies ordering semantics; keeping only the last value is correct for this case.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.js (1)
7-10: Key stays on Component while other props move to wrapper — good.This validates the intended split: key remains on , while style/className/id flow to the wrapper.
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.js (1)
6-11: Covers spread + shorthand mix in state initializer.Good coverage to ensure locals used via shorthand (c) remain and spreads are preserved in order.
packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (3)
520-566: Import handling and deprecation warnings look good.Changing old runtime imports to
${new_runtime_pkg}/legacy-react-runtimeand warning users aligns with the compat goals.
684-704: Target element detection logic is sound.Covers both intrinsic lowercase tags and identifiers imported from components pkg; matches existing tests.
491-494: Bug: Option doesn’t have add_pure_comment; guard the call.This will not compile as
Option<C>doesn’t implementComments. Useas_ref()and call when present.- self.comments.add_pure_comment(pure_span.lo); + if let Some(c) = self.comments.as_ref() { + c.add_pure_comment(pure_span.lo); + }Likely an incorrect or invalid review comment.
| if self.opts.target == TransformTarget::LEPUS { | ||
| if let Some(remove_component_attr_regex) = &self.opts.remove_component_attr_regex { | ||
| let re = regex::Regex::new(remove_component_attr_regex).unwrap(); | ||
| n.opening.attrs.retain(|p| { | ||
| if let JSXAttrOrSpread::JSXAttr(JSXAttr { | ||
| name: JSXAttrName::Ident(IdentName { sym, .. }), | ||
| .. | ||
| }) = p | ||
| { | ||
| if re.is_match(sym.as_ref()) { | ||
| return false; | ||
| } | ||
| } | ||
| true | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid unwrap on user-provided regex.
A bad removeComponentAttrRegex will panic the transform. Validate and warn instead.
- if let Some(remove_component_attr_regex) = &self.opts.remove_component_attr_regex {
- let re = regex::Regex::new(remove_component_attr_regex).unwrap();
+ if let Some(remove_component_attr_regex) = &self.opts.remove_component_attr_regex {
+ let re = match regex::Regex::new(remove_component_attr_regex) {
+ Ok(r) => r,
+ Err(e) => {
+ if !self.opts.disable_deprecated_warning {
+ HANDLER.with(|handler| {
+ handler
+ .struct_span_warn(n.span, format!("Invalid removeComponentAttrRegex: {e}").as_str())
+ .emit()
+ });
+ }
+ return;
+ }
+ };📝 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.
| if self.opts.target == TransformTarget::LEPUS { | |
| if let Some(remove_component_attr_regex) = &self.opts.remove_component_attr_regex { | |
| let re = regex::Regex::new(remove_component_attr_regex).unwrap(); | |
| n.opening.attrs.retain(|p| { | |
| if let JSXAttrOrSpread::JSXAttr(JSXAttr { | |
| name: JSXAttrName::Ident(IdentName { sym, .. }), | |
| .. | |
| }) = p | |
| { | |
| if re.is_match(sym.as_ref()) { | |
| return false; | |
| } | |
| } | |
| true | |
| }); | |
| } | |
| } | |
| if self.opts.target == TransformTarget::LEPUS { | |
| if let Some(remove_component_attr_regex) = &self.opts.remove_component_attr_regex { | |
| let re = match regex::Regex::new(remove_component_attr_regex) { | |
| Ok(r) => r, | |
| Err(e) => { | |
| if !self.opts.disable_deprecated_warning { | |
| HANDLER.with(|handler| { | |
| handler | |
| .struct_span_warn(n.span, format!("Invalid removeComponentAttrRegex: {e}").as_str()) | |
| .emit() | |
| }); | |
| } | |
| return; | |
| } | |
| }; | |
| n.opening.attrs.retain(|p| { | |
| if let JSXAttrOrSpread::JSXAttr(JSXAttr { | |
| name: JSXAttrName::Ident(IdentName { sym, .. }), | |
| .. | |
| }) = p | |
| { | |
| if re.is_match(sym.as_ref()) { | |
| return false; | |
| } | |
| } | |
| true | |
| }); | |
| } | |
| } |
| impl Visit for LocalDeclsDetectVisitor<'_> { | ||
| fn visit_ident(&mut self, _n: &Ident) { | ||
| if let Some(bindings) = self.bindings { | ||
| if bindings.contains(&_n.to_id()) { | ||
| self.has = true; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Avoid false positives from object literal keys.
Identifiers used as property keys (e.g., { foo: 1 }) aren’t references to local bindings. Skip visiting keys to prevent accidentally dropping valid props.
impl Visit for LocalDeclsDetectVisitor<'_> {
+ fn visit_prop_name(&mut self, _n: &PropName) {
+ // Intentionally ignore keys; only values/spreads should influence `has`.
+ }
fn visit_ident(&mut self, _n: &Ident) {
if let Some(bindings) = self.bindings {
if bindings.contains(&_n.to_id()) {
self.has = true;
}
}
}
}Please add/confirm a test where the constructor declares let foo = 1; but state has { foo: 123 } (as a literal key). The prop should be retained.
Skip object-literal property keys in LocalDeclsDetectVisitor
Override visit_prop_name so identifier keys (e.g. { foo: 1 }) don't count as uses of local bindings.
File: packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs (lines 164-171)
impl Visit for LocalDeclsDetectVisitor<'_> {
+ fn visit_prop_name(&mut self, _n: &PropName) {
+ // Intentionally ignore keys; only values/spreads should influence `has`.
+ }
fn visit_ident(&mut self, _n: &Ident) {
if let Some(bindings) = self.bindings {
if bindings.contains(&_n.to_id()) {
self.has = true;
}
}
}
}Add/confirm a test where the constructor declares let foo = 1; but state uses a literal key { foo: 123 } — the prop must be retained.
📝 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.
| impl Visit for LocalDeclsDetectVisitor<'_> { | |
| fn visit_ident(&mut self, _n: &Ident) { | |
| if let Some(bindings) = self.bindings { | |
| if bindings.contains(&_n.to_id()) { | |
| self.has = true; | |
| } | |
| } | |
| } | |
| impl Visit for LocalDeclsDetectVisitor<'_> { | |
| fn visit_prop_name(&mut self, _n: &PropName) { | |
| // Intentionally ignore keys; only values/spreads should influence `has`. | |
| } | |
| fn visit_ident(&mut self, _n: &Ident) { | |
| if let Some(bindings) = self.bindings { | |
| if bindings.contains(&_n.to_id()) { | |
| self.has = true; | |
| } | |
| } | |
| } | |
| } |
| "build": { | ||
| "dependsOn": [], | ||
| "inputs": ["src/**/*.rs", "Cargo.toml", "build.js"], | ||
| "outputs": ["swc_plugin_reactlynx_compat.wasm"] | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Include Cargo.lock and .cargo config in task inputs for correct cache invalidation
When Rust deps or cargo config change, the build should rerun. Add Cargo.lock and .cargo/** to inputs.
Apply:
"build": {
- "dependsOn": [],
- "inputs": ["src/**/*.rs", "Cargo.toml", "build.js"],
+ "dependsOn": [],
+ "inputs": ["src/**/*.rs", "Cargo.toml", "Cargo.lock", ".cargo/**", "build.js"],
"outputs": ["swc_plugin_reactlynx_compat.wasm"]
}📝 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.
| "build": { | |
| "dependsOn": [], | |
| "inputs": ["src/**/*.rs", "Cargo.toml", "build.js"], | |
| "outputs": ["swc_plugin_reactlynx_compat.wasm"] | |
| } | |
| "build": { | |
| "dependsOn": [], | |
| "inputs": ["src/**/*.rs", "Cargo.toml", "Cargo.lock", ".cargo/**", "build.js"], | |
| "outputs": ["swc_plugin_reactlynx_compat.wasm"] | |
| } |
🤖 Prompt for AI Agents
In packages/react/transform/swc-plugin-reactlynx-compat/turbo.json around lines
5 to 9, the build task's inputs omit Cargo.lock and .cargo/** which can change
dependency resolution and cargo config; update the "inputs" array to include
"Cargo.lock" and ".cargo/**" so changes to the lockfile or cargo config
invalidate cache and trigger rebuilds.
| serde = { workspace = true } | ||
| serde_json = { workspace = true } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enable serde derive feature.
Mirror the react/transform workspace requirement to prevent missing macro errors at compile time.
-serde = { workspace = true }
+serde = { workspace = true, features = ["derive"] }📝 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.
| serde = { workspace = true } | |
| serde_json = { workspace = true } | |
| serde = { workspace = true, features = ["derive"] } | |
| serde_json = { workspace = true } |
🤖 Prompt for AI Agents
In packages/react/transform/swc-plugin-reactlynx/Cargo.toml around lines 17-18,
the serde dependency must enable the derive feature to match the workspace
react/transform requirement; update the serde entry to include features =
["derive"] (e.g. serde = { workspace = true, features = ["derive"] }) so the
serde_derive macros are available at compile time while leaving serde_json as
workspace = true.
CodSpeed Performance ReportMerging #1744 will degrade performances by 5.81%Comparing 🎉 Hooray!
|
| Benchmark | BASE |
HEAD |
Change | |
|---|---|---|---|---|
| ❌ | transform 1000 effects |
30.6 ms | 32.5 ms | -5.81% |
| ❌ | basic-performance-large-css |
10.5 ms | 11.1 ms | -5.48% |
gaoachao
force-pushed
the
refactor/add-swc-plugin-reactlynx-compat
branch
from
21b92f5 to
62a1447
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs (1)
164-171: Do not treat object-literal keys as identifier uses (prevents false positives).Keys like
{ foo: 1 }should not markfooas a local use. Skip visiting property names in LocalDeclsDetectVisitor.impl Visit for LocalDeclsDetectVisitor<'_> { + fn visit_prop_name(&mut self, _n: &PropName) { + // Ignore keys; only values/spreads should influence `has`. + } fn visit_ident(&mut self, _n: &Ident) { if let Some(bindings) = self.bindings { if bindings.contains(&_n.to_id()) { self.has = true; } } } }packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (1)
777-793: Avoid unwrap on user-provided regex (graceful failure + warning).Invalid
removeComponentAttrRegexwill panic. Validate and warn instead; then skip removal.- if let Some(remove_component_attr_regex) = &self.opts.remove_component_attr_regex { - let re = regex::Regex::new(remove_component_attr_regex).unwrap(); + if let Some(remove_component_attr_regex) = &self.opts.remove_component_attr_regex { + let re = match regex::Regex::new(remove_component_attr_regex) { + Ok(r) => r, + Err(e) => { + if !self.opts.disable_deprecated_warning { + HANDLER.with(|handler| { + handler + .struct_span_warn(n.span, format!("Invalid removeComponentAttrRegex: {e}").as_str()) + .emit() + }); + } + return; + } + };
🧹 Nitpick comments (5)
packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs (3)
5-10: Provide a Default for TransformTarget (unblocks serde field defaults).Add Default to make LEPUS the implicit target when omitted in config.
#[derive(Debug, PartialEq, Clone, Copy)] pub enum TransformTarget { LEPUS, JS, MIXED, } + +impl Default for TransformTarget { + fn default() -> Self { + TransformTarget::LEPUS + } +}
12-26: Consider deriving serde instead of a manual Deserialize impl.Deriving with rename keeps behavior but simplifies code and avoids custom string plumbing. If you adopt this, adjust the unknown-variant test to assert is_err() rather than matching the exact message.
-impl<'de> Deserialize<'de> for TransformTarget { - fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> - where - D: Deserializer<'de>, - { - let s = String::deserialize(deserializer)?; - match s.as_str() { - "LEPUS" => Ok(TransformTarget::LEPUS), - "JS" => Ok(TransformTarget::JS), - "MIXED" => Ok(TransformTarget::MIXED), - _ => Err(serde::de::Error::custom(format!( - "value `{s}` does not match any variant of TransformTarget" - ))), - } - } -} +#[derive(serde::Serialize, serde::Deserialize, Debug, PartialEq, Clone, Copy)] +#[serde(rename_all = "UPPERCASE")] +pub enum TransformTarget { + LEPUS, + JS, + MIXED, +}
54-66: Test asserts shouldn’t depend on exact serde error strings.Comparing the full string is brittle across serde versions. Prefer asserting Err(…) and optionally that err.to_string().contains("unknown") if you need signal.
- if let Err(err) = result { - assert_eq!( - err.to_string(), - "value `unknown` does not match any variant of TransformTarget" - ); - } + assert!(result.is_err());packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs (1)
36-56: Precompile the regex used in constructors.Compiling
Regexper constructor visit is avoidable. Use a static Lazy to cut overhead in large files.- let re = Regex::new(r"^__[A-Z_]+__$").unwrap(); + static RE_MACRO_IDENT: once_cell::sync::Lazy<Regex> = + once_cell::sync::Lazy::new(|| Regex::new(r"^__[A-Z_]+__$").unwrap()); ... - if re.captures(test_ident.sym.as_str()).is_some() { + if RE_MACRO_IDENT.is_match(test_ident.sym.as_str()) { self.remain_stmts.push(stmt.clone()) }packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (1)
859-867: Doc vs. code: clarify handling of ref/key.Comment says “ref and key should still be bound on Component,” which matches code (they are not moved), but it’s implied via omission. Consider a brief comment stating this branch intentionally does not move
ref/key.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
biome.jsonc(1 hunks)eslint.config.js(2 hunks)packages/react/.dprint.jsonc(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/index.d.ts(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/is_component_class.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_compiler_only.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_embedded_compiler_only.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_is_import.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_change_runtime_pkg.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_handle_recursive.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_left_no_remove_component_element_attr.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_handle_jsx_member_expression.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_in_scope.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_not_rename_view_redeclaration.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_remove_component_attr_regex.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_rename_view.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_correct_order.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_macro.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_1.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_transform_event_props_2.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/turbo.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_rename_view.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_not_rename_view_redeclaration.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_simplify_ctor.js
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_not_handle_jsx_member_expression.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_transform_event_props_2.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_change_runtime_pkg.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_add_component_element_embedded.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_simplify_ctor_not_remain_local_decls.js
- biome.jsonc
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_handle_recursive.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_not_rename_view_in_scope.js
- packages/react/transform/swc-plugin-reactlynx-compat/turbo.json
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_simplify_ctor_correct_order.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_left_no_remove_component_element_attr.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_remove_component_attr_regex.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_spread.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_add_component_element_embedded_compiler_only.js
- packages/react/transform/swc-plugin-reactlynx-compat/src/is_component_class.rs
- packages/react/transform/swc-plugin-reactlynx-compat/index.d.ts
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_add_component_element.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_transform_event_props_1.js
- eslint.config.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_simplify_ctor_not_remain_local_decls_with_macro.js
- packages/react/transform/swc-plugin-reactlynx-compat/tests/swc_snapshots/src/lib.rs/should_add_component_is_import.js
- packages/react/.dprint.jsonc
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: upupming
PR: lynx-family/lynx-stack#1370
File: packages/webpack/cache-events-webpack-plugin/src/LynxCacheEventsRuntimeModule.ts:23-27
Timestamp: 2025-08-21T08:46:54.494Z
Learning: In Lynx webpack runtime modules, the team prioritizes performance and simplicity over defensive runtime error handling. They prefer relying on compile-time type safety (TypeScript) rather than adding runtime checks like try-catch blocks or type validation, especially for performance-critical code like cache event setup/cleanup functions.
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.810Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
📚 Learning: 2025-09-10T11:42:36.855Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/src/target.rs
📚 Learning: 2025-08-21T07:21:51.621Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1562
File: packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs:261-283
Timestamp: 2025-08-21T07:21:51.621Z
Learning: In packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs, the team prefers to keep the original unreachable! logic for JSXSpreadChild in jsx_is_children_full_dynamic function rather than implementing defensive error handling.
Applied to files:
packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs
🧬 Code graph analysis (1)
packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs (4)
packages/react/transform/swc-plugin-reactlynx-compat/src/is_component_class.rs (1)
new(12-18)packages/react/transform/swc-plugin-reactlynx-compat/src/simplify_ctor_like_react_lynx_2.rs (1)
new(21-30)packages/react/transform/index.d.ts (1)
DarkModeConfig(47-53)packages/react/transform/src/swc_plugin_snapshot/jsx_helpers.rs (1)
jsx_name(13-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: build / Build (Windows)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (1)
packages/react/transform/swc-plugin-reactlynx-compat/tests/__swc_snapshots__/src/lib.rs/should_add_component_element_compiler_only.js (1)
1-15: Confirm intentional duplicate snapshot lines (1↔2, 9↔10).Looks correct overall (spread props remain unwrapped in compilerOnly; key stays on Component; style/id moved to wrapper). Please confirm duplicates are expected from distinct inputs, not an accidental double-append in the snapshot generator.
| wrap_call = match wrap_call { | ||
| Expr::Call(mut call) => { | ||
| let pure_span = Span::dummy_with_cmt(); | ||
| self.comments.add_pure_comment(pure_span.lo); | ||
| call.span = pure_span; | ||
| Expr::Call(call) | ||
| } |
There was a problem hiding this comment.
Fix compile error: Option doesn’t have add_pure_comment.
self.comments is Option<C>. Guard before calling add_pure_comment.
- let pure_span = Span::dummy_with_cmt();
- self.comments.add_pure_comment(pure_span.lo);
+ let pure_span = Span::dummy_with_cmt();
+ if let Some(c) = &self.comments {
+ c.add_pure_comment(pure_span.lo);
+ }📝 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.
| wrap_call = match wrap_call { | |
| Expr::Call(mut call) => { | |
| let pure_span = Span::dummy_with_cmt(); | |
| self.comments.add_pure_comment(pure_span.lo); | |
| call.span = pure_span; | |
| Expr::Call(call) | |
| } | |
| wrap_call = match wrap_call { | |
| Expr::Call(mut call) => { | |
| let pure_span = Span::dummy_with_cmt(); | |
| if let Some(c) = &self.comments { | |
| c.add_pure_comment(pure_span.lo); | |
| } | |
| call.span = pure_span; | |
| Expr::Call(call) | |
| } |
🤖 Prompt for AI Agents
In packages/react/transform/swc-plugin-reactlynx-compat/src/lib.rs around lines
488 to 494, the code calls self.comments.add_pure_comment(...) but self.comments
is an Option<C>; guard the call by first checking for Some and only calling
add_pure_comment on the inner value (e.g. use if let Some(comments) =
self.comments.as_mut() { comments.add_pure_comment(pure_span.lo); } or match
self.comments { Some(ref mut c) => c.add_pure_comment(pure_span.lo), None => {},
} ), then proceed to set call.span and return Expr::Call(call).
| @@ -1 +1,1870 @@ | |||
| use convert_case::{Case, Casing}; | |||
There was a problem hiding this comment.
Instead of copying the whole file, can we re-use the existing mod or crate? E.g.:
- crates/swc_plugin_* <-- pure Rust crates that contains swc plugins
- packages/react/transform <-- napi-rs binding
- packages/react/transform/swc-plugin-* <-- WASM plugin binding
There was a problem hiding this comment.
While this is theoretically achievable, it would require some refactoring costs:
- The current
swc_plugin_*mods are not completely pure - they contain#[napi(object)] - In the WASM plugin, we would need
serdefor serialization and deserialization - If napi-rs bindings are to be completely removed in the future, this refactoring would be redundant
However, the current approach of directly copying source code has some drawbacks:
- Need to maintain consistency between two codebases
- Loss of commit history, making git blame unavailable
There was a problem hiding this comment.
The current swc_plugin_* mods are not completely pure - they contain #[napi(object)]
Yes it is. So what you need to do is to extract the swc plugin into a pure Rust crate (which could be reused by napi and WASM plugin). And handling the napi binding in the napi crate.
If napi-rs bindings are to be completely removed in the future, this refactoring would be redundant
We may still making changes before finishing the refactor. And we could make pure Rust benchmark & tests after this refactor.
gaoachao
force-pushed
the
refactor/add-swc-plugin-reactlynx-compat
branch
from
62a1447 to
58c71fd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/react/transform/swc-plugin-reactlynx-compat/build.js (1)
17-19: Add portable cargo fallback(s) when the build-wasi alias isn’t configured.
Relying solely on the alias will fail on envs without that config. Fall back to explicit targets.Apply:
-execSync('cargo build-wasi --release', { - env: { ...process.env, RUSTFLAGS }, - stdio: 'inherit', -}); +const env = { ...process.env, RUSTFLAGS }; +try { + execSync('cargo build-wasi --release', { env, stdio: 'inherit' }); +} catch { + try { + // Prefer the newer target first + execSync('cargo build --target wasm32-wasip1 --release', { env, stdio: 'inherit' }); + } catch { + // Fallback for older toolchains + execSync('cargo build --target wasm32-wasi --release', { env, stdio: 'inherit' }); + } +}
🧹 Nitpick comments (1)
packages/react/transform/swc-plugin-reactlynx-compat/build.js (1)
22-28: Resolve artifact path robustly (CARGO_TARGET_DIR and -wasip1/-wasi).
Current copy path assumes default target dir and -wasip1; honor CARGO_TARGET_DIR and probe both triples.Apply:
-await fs.copyFile( - path.resolve( - __dirname, - '../../../../target/wasm32-wasip1/release/swc_plugin_reactlynx_compat.wasm', - ), - path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm'), -); +const targetDir = + process.env.CARGO_TARGET_DIR ?? + path.resolve(__dirname, '../../../../target'); +const candidates = [ + path.join(targetDir, 'wasm32-wasip1', 'release', 'swc_plugin_reactlynx_compat.wasm'), + path.join(targetDir, 'wasm32-wasi', 'release', 'swc_plugin_reactlynx_compat.wasm'), +]; +let srcWasm = null; +for (const p of candidates) { + try { + await fs.access(p); + srcWasm = p; + break; + } catch {} +} +if (!srcWasm) { + throw new Error(`swc_plugin_reactlynx_compat.wasm not found in: ${candidates.join(', ')}`); +} +await fs.copyFile(srcWasm, path.resolve(__dirname, 'swc_plugin_reactlynx_compat.wasm'));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/react/.dprint.jsonc(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/build.js(1 hunks)packages/react/transform/swc-plugin-reactlynx-compat/turbo.json(1 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/react/transform/swc-plugin-reactlynx-compat/turbo.json
- packages/react/.dprint.jsonc
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-06T08:25:15.392Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1447
File: pnpm-workspace.yaml:20-22
Timestamp: 2025-08-06T08:25:15.392Z
Learning: In pnpm, configDependencies in pnpm-workspace.yaml (used for plugins like pnpm/plugin-better-defaults and pnpm/plugin-trusted-deps) do not appear in or modify pnpm-lock.yaml. These plugins are managed separately by pnpm itself, not as part of the workspace's dependency tree. This is different from the internal configDependencies field that can appear within pnpm-lock.yaml for tracking configuration file dependencies.
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-08-06T08:25:15.392Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1447
File: pnpm-workspace.yaml:20-22
Timestamp: 2025-08-06T08:25:15.392Z
Learning: In pnpm, configDependencies in pnpm-workspace.yaml (used for plugins like pnpm/plugin-better-defaults and pnpm/plugin-trusted-deps) do not appear in or modify pnpm-lock.yaml. These plugins are managed separately by pnpm itself as configuration-level dependencies, not as part of the workspace's dependency tree. Running pnpm install after adding configDependencies will not change the lockfile.
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.530Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Applied to files:
pnpm-workspace.yaml
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
PR: lynx-family/lynx-stack#1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
pnpm-workspace.yaml
🔇 Additional comments (3)
packages/react/transform/swc-plugin-reactlynx-compat/build.js (2)
13-15: Good: RUSTFLAGS are preserved and extended, not clobbered.
This keeps CI/user flags intact while adding the required link args.
22-28: ESM/top-level await — verified in CI. packages/react/transform/swc-plugin-reactlynx-compat/package.json sets "type": "module"; root package.json engines.node = ^22 || ^24 and CI workflows use actions/setup-node with node-version "22"/"24" — Node ≥18 covered.pnpm-workspace.yaml (1)
11-11: Confirm intent: keep both parent package and child-glob, or drop the parent entry.
packages/react/transform is a package: @lynx-js/react-transform (private: true, version: 0.2.0).
File: pnpm-workspace.yaml — line 11 (contains " - packages/react/transform/*").
If you want the parent package plus any nested packages, keep both entries; if you only want nested packages, remove the parent entry to avoid accidental inclusion.
A cherry-pick of #1458.
Follow up #1740, so we should merge it first.
Summary by CodeRabbit
New Features
Tests
Chores
Checklist