-
Notifications
You must be signed in to change notification settings - Fork 110
refactor: extract swc_plugin_css_scope
#1762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: f59fcae The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthroughAdds two new Rust crates under packages/react/transform/crates (swc_plugin_css_scope and swc_plugins_shared), wires them into the workspace, and relocates N-API bindings for CSS scope into a dedicated module. Updates imports to use the shared utilities crate. Expands ESLint and dprint ignore patterns and adds a placeholder changeset. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. Comment Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
f5b68f0 to
3e43c86
Compare
f24ecdf to
f59fcae
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (9)
packages/react/transform/crates/swc_plugins_shared/lib.rs (1)
1-1: Consider re‑exporting stable utilities at the crate root.Re‑exporting selected helpers (e.g., pub use utils::{calc_hash, jsonify};) reduces import churn and keeps API stable if utils/* reorganizes later.
packages/react/transform/crates/swc_plugin_css_scope/napi.rs (2)
10-15: Use #[napi(string_enum)] to remove unsafe N-API conversions.The derive handles string mapping and TS types, eliminating manual unsafe impls.
Apply:
-#[derive(Clone, Copy, Debug)] +#[napi(string_enum)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum CSSScope { All, None, Modules, } - -impl napi::bindgen_prelude::FromNapiValue for CSSScope { - unsafe fn from_napi_value( - env: napi::bindgen_prelude::sys::napi_env, - napi_val: napi::bindgen_prelude::sys::napi_value, - ) -> napi::bindgen_prelude::Result<Self> { - let val = <&str>::from_napi_value(env, napi_val).map_err(|e| { - napi::bindgen_prelude::error!( - e.status, - "Failed to convert napi value into enum `{}`. {}", - "CSSScope", - e, - ) - })?; - match val { - "all" => Ok(CSSScope::All), - "none" => Ok(CSSScope::None), - "modules" => Ok(CSSScope::Modules), - _ => Err(napi::bindgen_prelude::error!( - napi::bindgen_prelude::Status::InvalidArg, - "value `{}` does not match any variant of enum `{}`", - val, - "CSSScope" - )), - } - } -} - -impl napi::bindgen_prelude::ToNapiValue for CSSScope { - unsafe fn to_napi_value( - env: napi::bindgen_prelude::sys::napi_env, - val: Self, - ) -> napi::bindgen_prelude::Result<napi::bindgen_prelude::sys::napi_value> { - match val { - CSSScope::All => <&str>::to_napi_value(env, "all"), - CSSScope::None => <&str>::to_napi_value(env, "none"), - CSSScope::Modules => <&str>::to_napi_value(env, "modules"), - } - } -}Also applies to: 17-55
10-15: Nit: derive Eq/PartialEq for CSSScope (useful in tests).Already adding with the enum refactor above; if you keep manual impls, still consider deriving.
packages/react/transform/crates/swc_plugins_shared/Cargo.toml (1)
9-14: Trim swc_core features to only what's required.Cargo.toml enables many swc_core features, but this crate only uses swc_core::ecma::ast::* (packages/react/transform/crates/swc_plugins_shared/utils.rs). Remove non‑AST features (ecma_codegen, ecma_minifier, ecma_transforms_typescript, ecma_transforms_react, ecma_transforms_optimization, ecma_quote) from this crate and keep only the minimal features that expose AST types (e.g., base, ecma_utils); re-enable other features only in crates that need them. Verify by running cargo build or cargo tree -e features.
packages/react/transform/crates/swc_plugin_css_scope/lib.rs (4)
70-77: Avoid float-to-int cast forcss_idbase.Use an integer literal to prevent inadvertent float conversions.
- + 1e6 as usize, + + 1_000_000usize,
125-146: Compile the CSS filename regex once.
Regex::new(..)per module visit is avoidable; cache it withonce_cell::sync::Lazy.+ use once_cell::sync::Lazy; + static CSS_FILE_RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"\.(scss|sass|css|less)$").unwrap()); ... - let re = Regex::new(r"\.(scss|sass|css|less)$").unwrap(); ... - if re.is_match(import_decl.src.value.to_string().as_str()) { + if CSS_FILE_RE.is_match(import_decl.src.value.to_string().as_str()) {
128-131: Fix misleading comment.Condition
specifiers.is_empty()means side‑effects import; current comment says named/default/namespace.- if matches!(self.cfg.mode, CSSScope::Modules) && import_decl.specifiers.is_empty() { - // Is named/default/namespace import, nothing to do + if matches!(self.cfg.mode, CSSScope::Modules) && import_decl.specifiers.is_empty() { + // Side-effects import in Modules mode: skip (not treated as CSS Module) continue; }
148-157: Ensurecommentsis always provided when this pass is used.
add_leadingis called unconditionally whenhas_css_import; ifcommentswasNone, this would panic. Current call sites passSome(&comments), but this contract should be documented at the constructor or guarded.Add an assert in
new:pub fn new(cfg: CSSScopeVisitorConfig, comments: Option<C>) -> Self { CSSScopeVisitor { + // Safety: downstream relies on comments for emitting @jsxCSSId + // assert!(comments.is_some(), "CSSScopeVisitor requires comments");packages/react/transform/src/swc_plugin_snapshot/mod.rs (1)
26-27: Unify worklet hash to shared calc_hash (use 5 hex chars)swc_plugins_shared::utils::calc_hash returns 5 hex chars while packages/react/transform/src/swc_plugin_worklet/hash.rs implements a local calc_hash that returns 4 — this causes inconsistent IDs; switch the worklet file to reuse the shared implementation.
Affected locations:
- packages/react/transform/src/swc_plugin_worklet/hash.rs (local fn calc_hash, ~line 22)
- packages/react/transform/crates/swc_plugins_shared/utils.rs (pub fn calc_hash, ~line 63)
Proposed change (apply in packages/react/transform/src/swc_plugin_worklet/hash.rs):
- use sha1::{Digest, Sha1}; - use hex; + use swc_plugins_shared::utils::calc_hash; - fn calc_hash(s: &str) -> String { - let mut hasher = Sha1::new(); - hasher.update(s.as_bytes()); - let sum = hasher.finalize(); - - hex::encode(sum)[0..4].to_string() - } + // Reuse shared implementation for consistency (5 chars). + // pub(crate) use calc_hash from swc_plugins_shared
📜 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 (14)
.changeset/fine-places-boil.md(1 hunks)Cargo.toml(2 hunks)eslint.config.js(1 hunks)packages/react/.dprint.jsonc(1 hunks)packages/react/transform/Cargo.toml(1 hunks)packages/react/transform/crates/swc_plugin_css_scope/Cargo.toml(1 hunks)packages/react/transform/crates/swc_plugin_css_scope/lib.rs(1 hunks)packages/react/transform/crates/swc_plugin_css_scope/napi.rs(1 hunks)packages/react/transform/crates/swc_plugins_shared/Cargo.toml(1 hunks)packages/react/transform/crates/swc_plugins_shared/lib.rs(1 hunks)packages/react/transform/src/css.rs(1 hunks)packages/react/transform/src/lib.rs(2 hunks)packages/react/transform/src/swc_plugin_dynamic_import/mod.rs(1 hunks)packages/react/transform/src/swc_plugin_snapshot/mod.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/fine-places-boil.md
🧠 Learnings (5)
📓 Common learnings
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.
📚 Learning: 2025-09-10T11:42:36.855Z
Learnt from: gaoachao
PR: lynx-family/lynx-stack#1714
File: packages/react/transform/Cargo.toml:19-19
Timestamp: 2025-09-10T11:42:36.855Z
Learning: In packages/react/transform/Cargo.toml, the crate uses serde derive macros (#[derive(Serialize, Deserialize)]) in multiple files including src/esbuild.rs and src/swc_plugin_extract_str/mod.rs, so the "derive" feature is required when migrating to workspace dependencies.
Applied to files:
packages/react/transform/crates/swc_plugins_shared/Cargo.tomlpackages/react/transform/crates/swc_plugin_css_scope/Cargo.tomlCargo.tomlpackages/react/transform/Cargo.toml
📚 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:
.changeset/fine-places-boil.md
📚 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:
eslint.config.js
📚 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/.dprint.jsonc
🧬 Code graph analysis (6)
packages/react/transform/src/css.rs (1)
packages/react/transform/crates/swc_plugins_shared/utils.rs (1)
jsonify(6-61)
packages/react/transform/src/swc_plugin_dynamic_import/mod.rs (1)
packages/react/transform/crates/swc_plugins_shared/utils.rs (1)
jsonify(6-61)
packages/react/transform/src/lib.rs (2)
packages/react/transform/crates/swc_plugins_shared/utils.rs (1)
calc_hash(63-69)packages/react/transform/src/swc_plugin_worklet/hash.rs (1)
calc_hash(22-28)
packages/react/transform/crates/swc_plugin_css_scope/napi.rs (2)
packages/react/transform/src/lib.rs (11)
from_napi_value(87-110)from_napi_value(148-161)from_napi_value(192-222)to_napi_value(114-124)to_napi_value(165-179)to_napi_value(226-235)from(131-133)from(186-188)default(137-144)default(273-295)new(315-317)packages/react/transform/crates/swc_plugin_css_scope/lib.rs (5)
default(45-50)new(70-81)visit_mut_expr(88-96)n(111-121)visit_mut_module(98-158)
packages/react/transform/src/swc_plugin_snapshot/mod.rs (2)
packages/react/transform/src/css.rs (1)
get_string_inline_style_from_literal(14-64)packages/react/transform/crates/swc_plugins_shared/utils.rs (1)
calc_hash(63-69)
packages/react/transform/crates/swc_plugin_css_scope/lib.rs (1)
packages/react/transform/crates/swc_plugins_shared/utils.rs (1)
calc_hash(63-69)
⏰ 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 (14)
.changeset/fine-places-boil.md (1)
1-3: Placeholder changeset is acceptable for internal refactors.Matches repo practice for non-user‑facing changes. Please tick the “changeset added” checkbox in the PR description to keep process signals consistent.
eslint.config.js (1)
50-51: LGTM: expanded snapshot ignores.Keeps ESLint away from nested swc_snapshots under transform/**.
packages/react/.dprint.jsonc (1)
16-16: LGTM: align formatter excludes with ESLint.Avoids formatting churn in nested SWC snapshots.
packages/react/transform/crates/swc_plugin_css_scope/napi.rs (2)
77-86: Config N-API shape looks correct.ts_type on mode yields the intended union type in d.ts; filename as String is fine.
122-131: Thin wrapper delegating to CoreVisitor is clean.Good separation of N-API surface from core logic.
Cargo.toml (1)
6-7: Workspace wiring OK — napi4 compatible with CI Node targets
- Adding members under crates/* is fine.
- CI workflows target Node 22 and 24; Node‑API v4 is supported from Node.js v10.16.0 onward, so those CI targets are compatible. (nodejs.org)
packages/react/transform/crates/swc_plugin_css_scope/Cargo.toml (1)
9-14: Reconsider/remove internal swc_core features: __visit & __testing_transformrg only finds these in the crate's Cargo.toml (packages/react/transform/crates/swc_plugin_css_scope/Cargo.toml:13); no references in the crate source. Drop them unless required by macros/build-scripts or transitive usage — verify by removing and running a local cargo check or tools like cargo-shear / cargo-udeps.
packages/react/transform/src/css.rs (1)
10-10: LGTM: switched to sharedjsonify.Matches the shared crate API; no functional change.
packages/react/transform/src/lib.rs (3)
65-65: LGTM: CSS Scope imports now come from the new crate module.
swc_plugin_css_scope::napi::{CSSScopeVisitor, CSSScopeVisitorConfig}matches the extracted layout.
75-75: LGTM: centralizedcalc_hash.Consistent with other modules pulling from
swc_plugins_shared.
238-270: Public N-API surface: css_scope TypeScript typing present.
Confirmed packages/react/transform/crates/swc_plugin_css_scope/napi.rs includes #[napi(ts_type = "'all' | 'none' | 'modules'")] on CSSScopeVisitorConfig.mode — no breaking API change.packages/react/transform/crates/swc_plugin_css_scope/lib.rs (1)
12-15: Good split: sharedcalc_hashandpub mod napi.Separation of N-API glue is clean and aligns with the new shared utils crate.
packages/react/transform/src/swc_plugin_dynamic_import/mod.rs (1)
19-19: Move to shared utils — verifiedrg shows only swc_plugins_shared::utils::jsonify imports at packages/react/transform/src/css.rs:10 and packages/react/transform/src/swc_plugin_dynamic_import/mod.rs:19; no crate::utils::jsonify remains.
packages/react/transform/Cargo.toml (1)
14-15: Approve — workspace deps & new crates wired correctly.
- Verified: transform/Cargo.toml uses napi = { workspace = true } and napi-derive = { workspace = true }.
- Found crates swc_plugin_css_scope and swc_plugins_shared.
- swc_plugins_shared/utils.rs exposes pub fn jsonify and pub fn calc_hash.
- packages/react/transform/Cargo.toml keeps serde = { workspace = true, features = ["derive"] }.
Web Explorer#5255 Bundle Size — 365.07KiB (-0.03%).f59fcae(current) vs 75693e4 main#5246(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch refactor/extract-swc-plugin-css-... Project dashboard Generated by RelativeCI Documentation Report issue |
React Example#5262 Bundle Size — 237.52KiB (0%).f59fcae(current) vs 75693e4 main#5253(baseline) Bundle metrics
|
| Current #5262 |
Baseline #5253 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
162 |
162 |
|
65 |
65 |
|
46.71% |
46.71% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #5262 |
Baseline #5253 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.76KiB |
91.76KiB |
Bundle analysis report Branch refactor/extract-swc-plugin-css-... Project dashboard
Generated by RelativeCI Documentation Report issue
CodSpeed Performance ReportMerging #1762 will degrade performances by 7.96%Comparing Summary
Benchmarks breakdown
Footnotes |
colinaaa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Summary by CodeRabbit
Checklist