-
Notifications
You must be signed in to change notification settings - Fork 8
feat(rust-plugins): 🎸 update farm v2-beta #136
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
|
Caution Review failedThe pull request is closed. WalkthroughWorkspace dependency upgrades and multiple plugin migrations to Farm v2-beta: import path relocations, Arc-wrapped module content for SWC parsing, SWC codegen/sourcemap and Ident SyntaxContext updates, adoption of TransformOutputFileNameParams, SVGR/jsx-runtime adjustments, icons cache switched to bincode, Cargo.toml workspace refactors, playground package bumps, and toolchain update. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Plugin as Plugin (url/wasm/worker)
participant FS as farmfe_toolkit::fs
Note over Plugin,FS: Structured filename generation (new TransformOutputFileNameParams)
Plugin->>FS: transform_output_filename(TransformOutputFileNameParams{ filename_config, name, name_hash, bytes, ext, special_placeholders })
FS-->>Plugin: output_filename (string)
sequenceDiagram
autonumber
actor Plugin as Plugin (react-components/strip)
participant CM as create_swc_source_map
participant PARSE as parse_module
participant CODEGEN as swc_ecma_codegen
participant SM as cm.build_source_map
Note over Plugin: SWC parse & codegen flow with Arc content
Plugin->>CM: create_swc_source_map(module_id, Arc<content>)
CM-->>Plugin: (cm, src_map)
Plugin->>PARSE: parse_module(module_id.into(), Arc<content>, ts/js options)
PARSE-->>Plugin: { ast, .. }
Plugin->>CODEGEN: codegen_module(ast, Config::default(), Config::default(), Some(&mut src_map))
CODEGEN-->>Plugin: { code, map }
Plugin->>SM: cm.build_source_map(&src_map, None, DefaultSourceMapGenConfig)
SM-->>Plugin: sourcemap
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (32)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
rust-plugins/dsv/src/lib.rs (1)
81-95: Remove unwraps to prevent panics on malformed CSV/TSV
A bad record currently crashes the build. Handle errors gracefully.- let reader = get_reader(&binding); - - if reader.is_err() { - return Ok(None); - } - - let mut records = vec![]; - for result in reader.unwrap().records() { - let record = result.unwrap(); + let mut reader = match get_reader(&binding) { + Ok(r) => r, + // Unsupported type or open error: opt out silently (consistent with current behavior) + Err(_) => return Ok(None), + }; + + let mut records = Vec::new(); + for result in reader.records() { + let record = match result { + Ok(r) => r, + // Malformed content: opt out instead of panicking; consider emitting a warning if available + Err(_) => return Ok(None), + }; let json_record: Value = record .iter() .map(|field| Value::String(field.to_string())) .collect(); records.push(json_record); }rust-plugins/auto-import/src/parser/parse.rs (3)
399-409: Fix lifetime bug when reading file content; avoid referencing temporaries.You’re taking
&of a temporaryStringfromfs::read_to_stringand then using it in the next statement. That reference doesn’t live long enough. Also,&file_path.into()takes a reference to a temporaryString. Make both owned first, then wrap withArc.Apply this diff:
- let content = if content.is_none() { - &fs::read_to_string(file_path) - .unwrap_or_else(|_| panic!("Unable to read file: {:?}", file_path)) - } else { - content.unwrap() - }; - let content = Arc::new(content.to_string()); - let ParseScriptModuleResult { ast, .. } = match parse_module( - &file_path.into(), - content, + let content_owned: String = match content { + Some(c) => c.to_owned(), + None => fs::read_to_string(file_path) + .unwrap_or_else(|_| panic!("Unable to read file: {:?}", file_path)), + }; + let content = Arc::new(content_owned); + let file_path_owned = file_path.to_string(); + let ParseScriptModuleResult { ast, .. } = match parse_module( + &file_path_owned, + content,
439-449: Same lifetime/temporary issue inparse_esm_imports.Mirror the fix here to avoid
&of a temporary and reference to&file_path.into().Apply this diff:
- let content = if content.is_none() { - &fs::read_to_string(file_path) - .unwrap_or_else(|_| panic!("Unable to read file: {:?}", file_path)) - } else { - content.unwrap() - }; - let content = Arc::new(content.to_string()); - let ParseScriptModuleResult { ast, .. } = match parse_module( - &file_path.into(), - content, + let content_owned: String = match content { + Some(c) => c.to_owned(), + None => fs::read_to_string(file_path) + .unwrap_or_else(|_| panic!("Unable to read file: {:?}", file_path)), + }; + let content = Arc::new(content_owned); + let file_path_owned = file_path.to_string(); + let ParseScriptModuleResult { ast, .. } = match parse_module( + &file_path_owned, + content,
112-116: Type-only import classification is inverted.For
ImportSpecifier::Named, type-only specifiers should populatetype_named_imports; non-type should populatenamed_imports. Current logic is reversed.Apply this diff:
- if !named.is_type_only { - type_named_imports.insert(named.local.sym.to_string(), imported_name); - } else { - named_imports.insert(named.local.sym.to_string(), imported_name); - } + if named.is_type_only { + type_named_imports.insert(named.local.sym.to_string(), imported_name); + } else { + named_imports.insert(named.local.sym.to_string(), imported_name); + }rust-plugins/auto-import/src/parser/scan_exports.rs (1)
27-52: Escape import specifiers to generate valid JS on all platformsCurrent stringify emits raw paths inside single quotes; Windows backslashes and quotes can break the string. Prior learning recommends using {:?} to ensure proper escaping.
Using the retrieved learnings noted earlier, update the format strings:
- ExportType::DefaultDecl => format!("import {} from '{}';\n", self.name, self.from), + ExportType::DefaultDecl => format!("import {} from {:?};\n", self.name, self.from), @@ - format!("import {{ {} }} from '{}';\n", self.name, self.from) + format!("import {{ {} }} from {:?};\n", self.name, self.from) @@ - format!( - "import {{ {} as {} }} from '{}';\n", - self.name, as_name, self.from - ) + format!( + "import {{ {} as {} }} from {:?};\n", + self.name, as_name, self.from + ) @@ - format!("import {{ {} }} from '{}';\n", self.name, self.from) + format!("import {{ {} }} from {:?};\n", self.name, self.from) @@ - ExportType::Type => format!("import {{ type {} }} from '{}';\n", self.name, self.from), + ExportType::Type => format!("import {{ type {} }} from {:?};\n", self.name, self.from),rust-plugins/wasm/src/lib.rs (1)
78-83: Fix filename split, bytes type, and temporary reference in params
- split_once('.') breaks on names with multiple dots; use Path::file_stem/extension.
- bytes: ¶m.module_id.as_bytes() yields &&[u8]; pass param.module_id.as_bytes().
- special_placeholders takes a reference to a temporary Default; inline the params in the call or bind a local to extend lifetime.
Apply this diff:
- let file_name_ext = Path::new(wasm_file_path) - .file_name() - .map(|x| x.to_string_lossy().to_string()) - .unwrap(); - let (file_name, ext) = file_name_ext.split_once('.').unwrap(); + let path = Path::new(wasm_file_path); + let file_name = path + .file_stem() + .and_then(|x| x.to_str()) + .expect("wasm file_stem should be valid UTF-8"); + let ext = path + .extension() + .and_then(|x| x.to_str()) + .unwrap_or("wasm"); @@ - let transform_output_file_name_params = TransformOutputFileNameParams { - filename_config: assets_filename_config, - name: file_name, - name_hash: "", - bytes: ¶m.module_id.as_bytes(), - ext, - special_placeholders: &Default::default(), - }; - let output_file_name = transform_output_filename(transform_output_file_name_params); + let output_file_name = transform_output_filename(TransformOutputFileNameParams { + filename_config: assets_filename_config, + name: file_name, + name_hash: "", + bytes: param.module_id.as_bytes(), + ext, + special_placeholders: &Default::default(), + });Also applies to: 84-93
rust-plugins/worker/src/lib.rs (2)
301-310: Validate compiler config parsingThe compiler config parsing silently ignores errors with
.ok(), which could lead toNonebeing used later when the code expectsSome.Instead of silently ignoring parsing errors, consider logging them or handling them appropriately:
- compiler_config: serde_json::from_value(compiler_config).ok(), + compiler_config: match serde_json::from_value(compiler_config) { + Ok(config) => Some(config), + Err(e) => { + eprintln!("Warning: Failed to parse compiler config: {}", e); + None + } + },
199-200: The cache lookup at line 199 usesresolved_path, but earlier in the same function onlyfile_nameis ever used as the key when inserting intoworker_cache. This mismatch meansunwrap()here can indeed panic because that key may never have been inserted. Replace the unwrap with an explicit error or use the same key (file_name) consistently:- let content_bytes = worker_cache.get(resolved_path).unwrap(); + // Use the same key as in insertion, or handle missing entry explicitly + let content_bytes = worker_cache + .get(&file_name) + .unwrap_or_else(|| panic!("Expected worker_cache to contain `{}`", file_name));
🧹 Nitpick comments (36)
rust-plugins/react-components/tests/mod.rs (1)
76-79: Use Default::default for consistency and easier future type changes.Keeps style uniform and decouples construction from the concrete map/vector types.
- meta: FxHashMap::default(), + meta: Default::default(), source_map_chain: vec![], - query: vec![], + query: Default::default(),rust-plugins/react-components/playground/src/types/components.d.ts (3)
19-19: Normalize naming for lowercase antd utilities.For consistency with PascalCase globals, consider renaming:
- Antversion → AntVersion
- Antnotification → AntNotification
- Antmessage → AntMessage
- Anttheme → AntTheme
Apply locally (adjust your generator):
-const Antversion: typeof import('antd')['version'] +const AntVersion: typeof import('antd')['version'] -const Antnotification: typeof import('antd')['notification'] +const AntNotification: typeof import('antd')['notification'] -const Antmessage: typeof import('antd')['message'] +const AntMessage: typeof import('antd')['message'] -const Anttheme: typeof import('antd')['theme'] +const AntTheme: typeof import('antd')['theme']Also applies to: 81-81, 98-98, 145-145
127-127: Verify Arco Trigger export path.Check that '@arco-design/web-react' actually re-exports Trigger at the root. If not, point the type to its module entry (commonly '/es/Trigger' or '/Trigger') or drop it.
Possible fallback:
-const ArcoTrigger: typeof import('@arco-design/web-react')['Trigger'] +const ArcoTrigger: typeof import('@arco-design/web-react/es/Trigger')['default'](Adjust path per the installed version.)
12-144: Reduce global surface area (optional).To cut churn and collisions, consider exposing just two globals and deprecate per-component names:
declare global { const Ant: typeof import('antd'); const Arco: typeof import('@arco-design/web-react'); } export {};This keeps usage ergonomic (Ant.Button, Arco.Modal) and future-proof as libraries add/remove exports.
rust-plugins/worker/playground/package.json (1)
21-27: Align CLI/dev tool versions (optional).CLI is ^1.0.4 here but ^1.0.2 in other playbooks; consider standardizing to reduce drift.
rust-plugins/auto-import/playground-vue/package.json (1)
16-21: Optional: unify CLI version across playgrounds.Consider aligning @farmfe/cli with other examples for consistency.
rustfmt.toml (1)
2-2: No functional change; OK to keep.Key reordering is harmless. If unintentional churn, consider reverting to minimize diffs.
rust-plugins/wasm/playground/package.json (1)
21-27: Optional: normalize React deps across examples.This playground uses ^18.3.x while others pin "18". Consider aligning policy.
rust-plugins/react-components/Cargo.toml (1)
21-21: Document/centralize rustc-hash usage.If multiple crates use rustc-hash, consider moving it to
[workspace.dependencies]to keep versions in sync; add a short comment on why FxHasher is chosen.rust-plugins/worker/Cargo.toml (1)
19-20: rkyv 0.8 bump: confirm feature flags; FxHash caution if keys are persisted
- If you rely on zero-copy validation or runtime checks, enable rkyv’s validation/bytecheck features explicitly.
- FxHash (rustc-hash) is great for perf, but its hash is not stable across versions/platforms—avoid using it for cache keys persisted to disk or across processes.
-rkyv = "0.8.10" +// enable if you need archived type validation; otherwise keep as-is +rkyv = { version = "0.8.10", features = ["validation", "bytecheck"] }rust-plugins/dsv/src/lib.rs (2)
41-55: Handle extensions case-insensitively and set has_headers explicitly
This avoids surprises on uppercased extensions and makes header handling deterministic.fn get_reader(param: &Param) -> Result<Reader<&[u8]>, Box<dyn Error>> { - let extname = Path::new(¶m.module_id) + let extname = Path::new(¶m.module_id) .extension() .ok_or("No file extension found")? .to_str() .ok_or("File extension cannot be converted to string")?; - let reader = match extname { - "csv" => ReaderBuilder::new().from_reader(param.content.as_bytes()), - "tsv" => ReaderBuilder::new() - .delimiter(b'\t') - .from_reader(param.content.as_bytes()), + let ext = extname.to_ascii_lowercase(); + let reader = match ext.as_str() { + "csv" => ReaderBuilder::new() + .has_headers(false) + .from_reader(param.content.as_bytes()), + "tsv" => ReaderBuilder::new() + .has_headers(false) + .delimiter(b'\t') + .from_reader(param.content.as_bytes()), _ => return Err("Unsupported file type".into()), }; Ok(reader) }
31-34: Avoid startup panic on invalid options
Parsing options withunwrap()will crash initialization. Prefer a fall-back or a clear error.impl FarmPluginDsv { fn new(_config: &Config, options: String) -> Self { - let options: Options = serde_json::from_str(&options).unwrap(); + let options: Options = serde_json::from_str(&options) + .unwrap_or(Options { include: None, exclude: None }); Self { options } } }rust-plugins/image/src/lib.rs (2)
55-57: Surface I/O errors instead of emitting an empty data URI.Encoding an empty vec yields
data:...;base64,and hides real file-read failures. Prefer propagating the error or returningOk(None)with a diagnostic.Example (sketch):
let bytes = read_file_raw(¶m.resolved_path) .map_err(|e| farmfe_core::error::Error::new(e.to_string()))?; let file_base64 = general_purpose::STANDARD.encode(bytes);
46-47: Minor: simplify withunwrap_or_default().let include = options.include.unwrap_or_default(); let exclude = options.exclude.unwrap_or_default();rust-plugins/worker/playground/farm.config.ts (1)
22-24: Makeruntime.pathenv-configurable to support subpath deployments.Hard-coding "/" breaks when served under a base path.
- runtime: { - path: "/" - } + runtime: { + path: process.env.BASE_URL ?? "/" + }Please confirm this matches the 2.x config shape for
compilation.runtime.path.Cargo.toml (1)
7-12: Consider loosening to caret/major ranges to pick up patch releases automatically.Keeps reproducibility while allowing security/bugfix patches.
-farmfe_core = { version = "2.0.0" } -farmfe_utils = { version = "2.0.0" } -farmfe_toolkit_plugin_types = { version = "2.0.0" } -farmfe_macro_plugin = { version = "2.0.0" } -farmfe_toolkit = "2.0.0" -farmfe_compiler = { version = "2.0.0" } +farmfe_core = { version = "2" } +farmfe_utils = { version = "2" } +farmfe_toolkit_plugin_types = { version = "2" } +farmfe_macro_plugin = { version = "2" } +farmfe_toolkit = "2" +farmfe_compiler = { version = "2" }rust-plugins/svgr/src/react_compiler.rs (1)
12-21: Avoidunwrap(); return a Result to bubble up transform errors.Example:
pub fn react_compiler(param: CompilerParams) -> anyhow::Result<String> { let CompilerParams { svg, .. } = param; let code = _react_compiler( svg, Config { jsx_runtime: JSXRuntime::Classic, ..Default::default() }, Default::default(), )?; Ok(code) }rust-plugins/modular-import/Cargo.toml (1)
15-15: Optional: lift regex to workspace.dependencies to avoid duplicatesMultiple crates may use
regex; centralizing in[workspace.dependencies]keeps a single semver across the repo.rust-plugins/url/Cargo.toml (1)
16-17: LGTM; consider moving serde to workspace for uniform version/feature flagsUsing
serdewith derive is fine here; for consistency and to prevent duplicates, consider[workspace.dependencies] serde = { version = "1.x", features=["derive"] }.rust-plugins/icons/src/compiler/preact.rs (2)
15-15: Avoid unwrap: surface transform errors with contextPrevent panics in plugin code paths by adding context or propagating the error.
- ) - .unwrap(); + ) + .unwrap_or_else(|e| panic!("svgr transform (preact) failed: {e}"));If you can change the signature, prefer returning Result<String, E> and bubble up.
1-1: Nit: “complier” → “compiler” (typo in alias and function name)Renaming improves readability and searchability.
-pub use svgr_rs::{transform as react_complier, Config, JSXRuntime}; +pub use svgr_rs::{transform as react_compiler, Config, JSXRuntime}; -pub fn preact_complier(param: CompilerParams) -> String { +pub fn preact_compiler(param: CompilerParams) -> String { - let code = react_complier( + let code = react_compiler(Also applies to: 5-5, 7-7
rust-plugins/auto-import/src/parser/parse.rs (1)
258-276: SimplifyTsModuleid extraction (readability).This chain clones and unwraps multiple times. A simple
if letavoids redundant clones and unwraps.Suggested refactor (outside changed lines):
let name = if let Some(s) = module_decl.id.clone().str() { s.value.to_string() } else if let Some(id) = module_decl.id.clone().ident() { id.sym.to_string() } else { "Anonymous".to_string() };rust-plugins/icons/src/cache/mod.rs (2)
53-58: Avoidunwrap()on encode; propagate an error instead.If encoding fails, fail the request rather than panic.
Apply this diff:
- let serialized_data = bincode::encode_to_vec(&cache_value, config).unwrap(); - self - .cache - .connection() - .insert(url.to_string(), serialized_data) - .unwrap(); + let serialized_data = bincode::encode_to_vec(&cache_value, config) + .map_err(|e| { + loading.fail(format!("{} icon cache encode err: {:?}", url, e)); + e + })?; + let _ = self.cache.connection().insert(url.to_string(), serialized_data);
41-46: Reuse an HTTP client for connection pooling.
reqwest::getcreates a new client each time. Consider injecting areqwest::ClientintoHttpClientto reuse connections.If desired, I can draft the change.
rust-plugins/icons/src/compiler/react.rs (1)
1-5: Nit: fix “complier” typo in local alias for clarityRename the alias to reduce confusion; safe, local-only change.
-pub use svgr_rs::{transform as _react_complier, Config, JSXRuntime}; +pub use svgr_rs::{transform as _react_compiler, Config, JSXRuntime}; @@ - let code = _react_complier( + let code = _react_compiler(rust-plugins/auto-import/src/parser/scan_dirs_exports.rs (2)
19-19: Harden path/extension handling (avoid panics; handle case-insensitive extensions)to_str().unwrap() can panic on non-UTF8 paths, and ext matching is case-sensitive. Prefer lossy conversion and lowercase ext.
Apply this diff:
- let exports_names = scan_exports(&normalize_path(file_path.to_str().unwrap()), None); + let path_str = file_path.to_string_lossy(); + let exports_names = scan_exports(&normalize_path(&path_str), None); @@ - let normalized_path = normalize_path(e.path().to_str().unwrap()); - if filter.execute(&normalized_path) - && e.path().extension().is_some() - && file_exts.contains(&e.path().extension().unwrap().to_str().unwrap()) + let normalized_path = normalize_path(&e.path().to_string_lossy()); + let ext_lc = e + .path() + .extension() + .and_then(|s| s.to_str()) + .map(|s| s.to_ascii_lowercase()); + if filter.execute(&normalized_path) + && ext_lc + .as_deref() + .is_some_and(|ext| file_exts.iter().any(|e| e.eq_ignore_ascii_case(ext)))Also applies to: 41-45
30-30: Prefer slices over &Vec in paramsUse &[ConfigRegex] to avoid forcing callers to allocate a Vec.
-pub fn scan_dirs_exports(root_path: &str, dirs: &Vec<ConfigRegex>) -> Vec<Import> { +pub fn scan_dirs_exports(root_path: &str, dirs: &[ConfigRegex]) -> Vec<Import> {rust-plugins/react-components/src/find_local_components.rs (2)
330-335: Avoid referencing a temporary PathBuf; pass a Path directly&components_path.clone().into() borrows a temporary. Use Path::new(&components_path) (or prebind a PathBuf) for clarity and to avoid accidental lifetime pitfalls.
- let ParseScriptModuleResult { ast,..} = match parse_module( - &components_path.clone().into(), - file_content, + let ParseScriptModuleResult { ast, .. } = match parse_module( + std::path::Path::new(&components_path), + file_content,
382-385: Normalize path using lossy conversion to avoid panicsMinor robustness improvement when normalizing OS paths.
- let normalized_path = normalize_path(e.path().to_str().unwrap()); + let normalized_path = normalize_path(&e.path().to_string_lossy());rust-plugins/wasm/src/lib.rs (1)
101-121: String formatting is fine; consider trimming shared indentNot critical, but the emitted JS contains leading spaces. Using indoc! or a raw string without indentation could reduce bytes.
rust-plugins/strip/src/lib.rs (3)
113-114: Potential Arc usage inefficiencyThe content is being cloned and wrapped in Arc twice - once for
create_swc_source_mapand again forparse_module. Consider reusing the Arc-wrapped content.- let (cm, _) = - create_swc_source_map(¶m.resolved_path.into(), Arc::new(param.content.clone())); + let content = Arc::new(param.content.clone()); + let (cm, _) = create_swc_source_map(¶m.resolved_path.into(), content.clone());Then update line 119:
- Arc::new(param.content.clone()), + content,
82-84: Simplify boolean comparison logicThe source_map check can be simplified by using
unwrap_or(false)directly.- let source_map = match options.source_map { - Some(s) => s != false, - None => false, - }; + let source_map = options.source_map.unwrap_or(false);
86-89: Simplify debugger option handlingSimilar to the source_map option, this can be simplified.
- let remove_debugger_statements = match options.debugger { - Some(s) => s != false, - None => false, - }; + let remove_debugger_statements = options.debugger.unwrap_or(false);rust-plugins/react-components/src/lib.rs (1)
164-170: Redundant Arc wrapping of contentSimilar to the strip plugin, the content is being cloned and wrapped in Arc twice. Consider reusing the Arc-wrapped content.
- let content = Arc::new(param.content.clone()); - let (cm, _) = create_swc_source_map(¶m.module_id.clone().into(), content); - let ParseScriptModuleResult { - mut ast, comments, .. - } = match parse_module( - ¶m.module_id.clone().into(), - Arc::new(param.content.clone()), + let content = Arc::new(param.content.clone()); + let (cm, _) = create_swc_source_map(¶m.module_id.clone().into(), content.clone()); + let ParseScriptModuleResult { + mut ast, comments, .. + } = match parse_module( + ¶m.module_id.clone().into(), + content,rust-plugins/worker/src/lib.rs (2)
113-120: Fix indentation inconsistencyThe TransformOutputFileNameParams initialization has inconsistent indentation.
- let transform_output_file_name_params = TransformOutputFileNameParams { - filename_config: assets_filename_config, - name: file_name, - name_hash: "", - bytes: &module_id.as_bytes(), - ext, - special_placeholders: &Default::default(), - }; + let transform_output_file_name_params = TransformOutputFileNameParams { + filename_config: assets_filename_config, + name: file_name, + name_hash: "", + bytes: &module_id.as_bytes(), + ext, + special_placeholders: &Default::default(), + };
55-57: Consider using more descriptive variable nameThe variable name
_worker_urlis prefixed with underscore but it's actually being used (file_name is extracted from get_worker_url). Consider renaming for clarity.- let (_worker_url, full_file_name) = get_worker_url(resolved_path, module_id, compiler_config); + let (worker_url, full_file_name) = get_worker_url(resolved_path, module_id, compiler_config); + let _ = worker_url; // Explicitly mark as unused if not needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (5)
Cargo.lockis excluded by!**/*.lockrust-plugins/auto-import/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlrust-plugins/react-components/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlrust-plugins/wasm/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlrust-plugins/worker/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (37)
Cargo.toml(1 hunks)rust-plugins/auto-import/playground-react/package.json(1 hunks)rust-plugins/auto-import/playground-vue/package.json(1 hunks)rust-plugins/auto-import/src/lib.rs(1 hunks)rust-plugins/auto-import/src/parser/parse.rs(4 hunks)rust-plugins/auto-import/src/parser/scan_dirs_exports.rs(1 hunks)rust-plugins/auto-import/src/parser/scan_exports.rs(1 hunks)rust-plugins/compress/Cargo.toml(1 hunks)rust-plugins/dsv/src/lib.rs(1 hunks)rust-plugins/icons/Cargo.toml(1 hunks)rust-plugins/icons/src/cache/mod.rs(3 hunks)rust-plugins/icons/src/compiler/preact.rs(1 hunks)rust-plugins/icons/src/compiler/react.rs(1 hunks)rust-plugins/icons/src/lib.rs(1 hunks)rust-plugins/image/src/lib.rs(1 hunks)rust-plugins/modular-import/Cargo.toml(1 hunks)rust-plugins/react-components/Cargo.toml(1 hunks)rust-plugins/react-components/playground/package.json(2 hunks)rust-plugins/react-components/playground/src/types/components.d.ts(1 hunks)rust-plugins/react-components/src/find_local_components.rs(2 hunks)rust-plugins/react-components/src/insert_import.rs(3 hunks)rust-plugins/react-components/src/lib.rs(5 hunks)rust-plugins/react-components/tests/mod.rs(2 hunks)rust-plugins/strip/src/lib.rs(6 hunks)rust-plugins/svgr/Cargo.toml(1 hunks)rust-plugins/svgr/src/react_compiler.rs(1 hunks)rust-plugins/url/Cargo.toml(1 hunks)rust-plugins/url/src/lib.rs(3 hunks)rust-plugins/wasm/Cargo.toml(1 hunks)rust-plugins/wasm/playground/package.json(1 hunks)rust-plugins/wasm/src/lib.rs(2 hunks)rust-plugins/worker/Cargo.toml(1 hunks)rust-plugins/worker/playground/farm.config.ts(1 hunks)rust-plugins/worker/playground/package.json(1 hunks)rust-plugins/worker/src/lib.rs(5 hunks)rust-toolchain.toml(1 hunks)rustfmt.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-02-24T06:59:13.009Z
Learnt from: fu050409
PR: farm-fe/plugins#115
File: rust-plugins/wasm/src/utils.rs:71-80
Timestamp: 2025-02-24T06:59:13.009Z
Learning: When generating JavaScript import statements from file paths in Rust, use `{:?}` format specifier instead of `{}` to ensure proper escaping of special characters in paths (e.g., `"".txt` → `\"\"`).
Applied to files:
rust-plugins/auto-import/src/parser/scan_exports.rs
🧬 Code graph analysis (5)
rust-plugins/auto-import/src/parser/scan_dirs_exports.rs (1)
rust-plugins/virtual/src/utils.rs (1)
normalize_path(57-80)
rust-plugins/auto-import/src/parser/scan_exports.rs (1)
rust-plugins/virtual/src/utils.rs (1)
normalize_path(57-80)
rust-plugins/wasm/src/lib.rs (1)
rust-plugins/wasm/src/utils.rs (1)
generate_glue_code(64-125)
rust-plugins/worker/src/lib.rs (1)
rust-plugins/worker/tests/mod.rs (1)
Regex(90-90)
rust-plugins/react-components/src/find_local_components.rs (1)
rust-plugins/virtual/src/utils.rs (1)
normalize_path(57-80)
🔇 Additional comments (33)
rust-plugins/react-components/playground/src/types/components.d.ts (2)
12-18: LGTM on ambient typings pattern.Using
declare globalwithtypeof import(...)keeps globals aligned with library types. No issues spotted in this block.
12-12: Deps and tsconfig include verified:antdand@arco-design/web-reactare listed in package.json, and tsconfig.json’s"include": ["src"]coverssrc/types/components.d.ts.rust-plugins/auto-import/playground-vue/package.json (1)
17-17: LGTM on Farm v2 bump.Version string is correct ("beta"); no issues spotted.
rust-plugins/dsv/src/lib.rs (2)
3-3: PathFilter import migration looks good
No functional change; aligns with toolkit’s new module path.
98-103: Verify module_type for Farm 2.x
Confirm thatModuleType::Custom("json")is still the intended type for “export default ” modules in v2. If the loader expects JS, switch toModuleType::Js.rust-toolchain.toml (1)
2-2: Toolchain bump acknowledged; ensure CI/dev envs are pinned
Looks good. Please confirm CI installs nightly-2025-05-06 to avoid drift.rust-plugins/auto-import/playground-react/package.json (1)
20-21: Align prerelease version spec across playgrounds and verify CLI compatibility- "@farmfe/core": "^2.0.0-beta.0", - "@farmfe/plugin-react": "^2.0.0-beta.0", + "@farmfe/core": "2.0.0-beta.0", + "@farmfe/plugin-react": "2.0.0-beta.0",Manually confirm that
@farmfe/cli@^1.xis compatible with Core 2.x.rust-plugins/react-components/playground/package.json (1)
14-14: Unify Farm plugin version channels
js-plugin-less is pinned to nightly (2.0.0-nightly-20250827162746) but there’s no 2.0.0-beta publish and the stable “latest” is 1.13.0. Align all Farm plugins in package.json to a single channel—either nightly (^2.0.0-nightly-…) across the board or stable (^1.13.0)—to prevent drift.rust-plugins/image/src/lib.rs (1)
11-12: Import path relocation to toolkit 2.x looks correct.Reads clean and aligns with the new module layout.
rust-plugins/svgr/Cargo.toml (1)
17-19: Loosen Cargo.toml pins for svgr-rs and xmltree--- rust-plugins/svgr/Cargo.toml -svgr-rs = "0.2.0" +svgr-rs = "0.2" -xmltree = "0.11.0" +xmltree = "0.11"Verify JSXRuntime::Classic and xmltree::Element APIs remain unchanged in patch releases.
rust-plugins/svgr/src/react_compiler.rs (2)
15-15: JSXRuntime enum update is correct for svgr-rs 0.2.Matches the non-optional API.
15-15: Consider exposing runtime selection.If consumers use React 17+ with automatic runtime, allow opting into
JSXRuntime::Automatic.rust-plugins/icons/Cargo.toml (4)
17-17: All JSXRuntime callers updated to non-Option NoOption<JSXRuntime>remains in preact, react, or any tests/fixtures.
14-14: No duplicate farmfe_utils versions found All workspace crates useworkspace = true, inheriting version 2.0.0 from the rootCargo.toml.
23-23: No xmltree 0.11 API changes impact svg_builder.rs
Scan ofrust-plugins/icons/src/loader/svg_builder.rsshows onlyuse xmltree::Element, theElementtype, and theElement::parsemethod—both of which remain unchanged in xmltree 0.11.0 (eminence.github.io)
27-27: No action required:disk_storefeature correct and no MSRV change
Upstreamcachedv0.56.0 defines thedisk_storefeature exactly as used and does not declare arust-version, so bumping to 0.56.0 introduces no MSRV impact.rust-plugins/wasm/Cargo.toml (2)
10-13: Workspace dep switch LGTMMoving core farm crates to
workspace = trueis consistent and reduces version drift.
10-15: Remove validation/bytecheck suggestion: noArchive/Deserializederives detectedSearch for
#[derive(Archive)]andbytecheckinrust-plugins/wasmreturned no matches—enabling rkyv validation features is unnecessary.rust-plugins/modular-import/Cargo.toml (1)
10-12: Workspace alignment LGTMGood consolidation to workspace-managed versions.
rust-plugins/icons/src/compiler/preact.rs (1)
10-11: JSX runtime update LGTM; mirrors svgr-rs 0.2 non-Option fieldChange looks correct and consistent with the new API.
rust-plugins/react-components/src/insert_import.rs (2)
1-1: SWC API migration toIdent::new(..., SyntaxContext::empty())looks correct.Calls are updated consistently for default and named specifiers and re-exports.
Also applies to: 133-145
62-69: Mutex lock scope is short and cloned before release — good.This avoids holding the lock while mutating
used_components.rust-plugins/auto-import/src/lib.rs (1)
21-21: Import path update to plugin_utils is correct for v2Looks consistent with the toolkit re-org. Please ensure all sibling modules use the same path to avoid mixed imports.
rust-plugins/auto-import/src/parser/scan_exports.rs (1)
6-7: Fixed normalize_path import pathCorrecting to plugin_utils aligns with the new layout. No further changes needed here.
rust-plugins/icons/src/compiler/react.rs (1)
10-10: JSX runtime no longer optional: confirm svgr_rs API matchSetting jsx_runtime directly implies svgr_rs Config changed. Ensure the workspace pins svgr_rs to the version where jsx_runtime is non-Option, or this will fail to compile.
rust-plugins/icons/src/lib.rs (1)
47-47: Avoid unwrappingcache_dirto prevent panics when persistent cache is disabled
Replace theunwrap()with a safe fallback, for example:- let cache_dir = config.persistent_cache.as_obj(&config.root).cache_dir.unwrap(); + let cache_dir = config + .persistent_cache + .as_obj(&config.root) + .cache_dir + .unwrap_or_else(|| { + let fallback = std::env::temp_dir().join("farmfe-icons-cache"); + let _ = std::fs::create_dir_all(&fallback); + fallback + });If you’re confident v2 always provides a directory, use
expect("cache_dir must be set")instead.
Verify thatPersistentCacheConfig::Bool(false)leads toNoneforcache_dirinas_obj, and audit the repo for otherpersistent_cache…unwrap()calls.rust-plugins/auto-import/src/parser/scan_dirs_exports.rs (1)
3-3: Import path migration looks correctUsing plugin_utils::{normalize_path, PathFilter} matches the v2 layout.
rust-plugins/react-components/src/find_local_components.rs (2)
8-9: Import reorg aligns with toolkit v2plugin_utils::{normalize_path, PathFilter} and script::{parse_module, ParseScriptModuleResult} look good.
11-11: Arc import LGTMArc-wrapping file content matches the new parse_module API.
rust-plugins/wasm/src/lib.rs (2)
12-13: Required traits importedBringing Cacheable into scope is necessary for #[cache_item]. Good.
18-18: API migration acknowledgedSwitch to TransformOutputFileNameParams matches the new toolkit fs API.
rust-plugins/url/src/lib.rs (2)
24-26: Imports updated correctlyfs::{TransformOutputFileNameParams, read_file_raw, transform_output_filename} and plugin_utils::PathFilter are correct for v2.
167-168: Signature update acknowledgedfinalize_resources now uses PluginFinalizeResourcesHookParam; change looks consistent.
| farmfe_core = { version = "2.0.0" } | ||
| farmfe_utils = { version = "2.0.0" } | ||
| farmfe_toolkit_plugin_types = { version = "2.0.0" } | ||
| farmfe_macro_plugin = { version = "2.0.0" } | ||
| farmfe_toolkit = "2.0.0" | ||
| farmfe_compiler = { version = "2.0.0" } |
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.
💡 Verification agent
🧩 Analysis chain
Verify no stale pre-2.x imports/usages remain.
Run:
🏁 Script executed:
#!/bin/bash
# old PathFilter location and other likely renames
rg -n 'farmfe_toolkit::common::PathFilter|normalize_path\(|TransformOutputFileNameParams|PluginTransformHookParam\s*\{|PluginLoadHookParam\s*\{' -S -g '!**/target/**'Length of output: 2103
Update or remove all stale pre-2.x FarmFE API usages
Stale imports and calls remain across the rust-plugins crates (wasm, virtual, url, worker, react-components, auto-import). You must replace or remove references to the old APIs—e.g.
farmfe_toolkit::fs::{transform_output_filename, TransformOutputFileNameParams}normalize_path(...)PluginTransformHookParam { … }/PluginLoadHookParam { … }
🤖 Prompt for AI Agents
Cargo.toml lines 7-12: the project is pinned to FarmFE 2.0.0 but the
rust-plugins crates still reference pre-2.x FarmFE APIs; update or remove those
stale imports and usages across the rust-plugins crates (wasm, virtual, url,
worker, react-components, auto-import). Search for and replace removed APIs such
as farmfe_toolkit::fs::{transform_output_filename,
TransformOutputFileNameParams}, normalize_path(...), and old
PluginTransformHookParam/PluginLoadHookParam structures with their 2.x
equivalents or new utility functions/types provided by
farmfe_toolkit/farmfe_utils, refactor hook signatures to match the new
trait/type names, and remove any no-longer-needed helper wrappers; ensure
compilation by running cargo check and update Cargo.toml dependency entries only
if you need to change feature flags or versions for the new APIs.
| let content = Arc::new(content.to_string()); | ||
| let ParseScriptModuleResult { ast, .. } = match parse_module( | ||
| &file_path.into(), | ||
| content, | ||
| Syntax::Typescript(TsSyntax { |
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.
Same lifetime/temporary issue in parse_esm_exports.
Repeat the ownership fix here as well.
Apply this diff:
- let content = Arc::new(content.to_string());
- let ParseScriptModuleResult { ast, .. } = match parse_module(
- &file_path.into(),
- content,
+ let content_owned: String = match content {
+ Some(c) => c.to_owned(),
+ None => fs::read_to_string(file_path)
+ .unwrap_or_else(|_| panic!("Unable to read file: {:?}", file_path)),
+ };
+ let content = Arc::new(content_owned);
+ let file_path_owned = file_path.to_string();
+ let ParseScriptModuleResult { ast, .. } = match parse_module(
+ &file_path_owned,
+ content,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In rust-plugins/auto-import/src/parser/parse.rs around lines 485-489,
parse_esm_exports suffers the same temporary/lifetime ownership issue as
earlier: create an Arc<String> for the file content (e.g. let content =
Arc::new(content.to_string());) and pass that Arc (or content.clone()) into
parse_module instead of the temporary string slice so the AST retains a valid
owned reference; update the call site to use the Arc value and adjust any
parameter types if necessary to accept Arc<String>.
| farmfe_core = { workspace = true } | ||
| farmfe_toolkit_plugin_types = { workspace = true } | ||
| farmfe_macro_plugin = { workspace = true } |
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.
💡 Verification agent
🧩 Analysis chain
Workspace deps switch: verify workspace pins exist and match farm v2
Looks good, but compilation will fail if these crates aren’t declared in [workspace.dependencies] at the root or versions drift from 2.x. Please verify.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Workspace root and dependency pins:"
rg -nP '^\[workspace\]' -n Cargo.toml || true
rg -nP '^\[workspace\.dependencies\]' -n Cargo.toml -C3 || true
echo
echo "Looking for farmfe workspace deps:"
rg -nP '^\s*(farmfe_core|farmfe_toolkit_plugin_types|farmfe_macro_plugin)\s*=' **/Cargo.toml -g '!**/target/**' -C1Length of output: 674
Add missing farmfe_macro_plugin to root [workspace.dependencies] and align versions
The root Cargo.toml’s [workspace.dependencies] currently pins farmfe_core, farmfe_utils, and farmfe_toolkit_plugin_types at version 2.0.0 but omits farmfe_macro_plugin. Add:
[workspace.dependencies]
…
farmfe_macro_plugin = { version = "2.0.0" }and ensure all farmfe_* deps remain on 2.x to prevent build breaks.
🤖 Prompt for AI Agents
In rust-plugins/compress/Cargo.toml around lines 10 to 12, the workspace
dependencies list includes farmfe_core and farmfe_toolkit_plugin_types but omits
farmfe_macro_plugin and the versions must be aligned to 2.x; add
farmfe_macro_plugin to the [workspace.dependencies] section with version "2.0.0"
(and verify all other farmfe_* entries are set to version "2.0.0") so all
farmfe_* dependencies remain on the 2.x line to prevent build breaks.
| cached = { version = "0.56.0", features = ["disk_store"] } | ||
| bincode = "2.0.1" |
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.
💡 Verification agent
🧩 Analysis chain
Disk cache compatibility: bincode v2 breaks v1 cache format—bump cache version or handle decode fallback
Upgrading bincode 1.x → 2.x will make previously persisted cache entries unreadable. To avoid hard failures and stale cache reuse, version your on-disk cache keys or add a graceful v1→v2 fallback.
If you already did this elsewhere, ignore. Otherwise, I can generate a small helper (CacheHeader { version: 2, ... }) and wire it into the read/write path—want me to send a patch?
🏁 Script executed:
#!/bin/bash
# Verify whether cache versioning exists or bincode is used directly.
rg -nS 'bincode::' rust-plugins/icons
rg -nS 'version' rust-plugins/icons | rg -nS 'cache|icon'Length of output: 4709
Version on-disk cache entries for bincode v2 compatibility
bincode v2 (bincode = "2.0.1" in Cargo.toml) changes the wire format and will break reads of any files written with v1. In rust-plugins/icons/src/cache/mod.rs, both
let cached_value: (CacheValue, usize) = bincode::decode_from_slice(&entry, config).unwrap();
let serialized_data = bincode::encode_to_vec(&cache_value, config).unwrap();assume a headerless payload. Add a small version header (e.g. a u8 or u16 before the bincode blob), bump it to 2 for new entries, and implement fallback logic to decode v1 when the header indicates version 1.
🤖 Prompt for AI Agents
In rust-plugins/icons/Cargo.toml around lines 27-28 and corresponding
rust-plugins/icons/src/cache/mod.rs, the on-disk bincode wire format changed in
bincode v2 so existing headerless v1 payloads will fail to decode; modify the
cache read/write to prepend a small version header (use a u8, write 2 for new
entries) before the bincode blob when serializing, and when deserializing first
read the header byte: if it equals 2 decode the following bytes with the current
bincode v2 config, if it indicates legacy (missing header or value 1) fall back
to the v1 decode path (attempt decode without header or use a v1-compatible
config) so existing files remain readable; ensure writers always write the
version byte 2 and update error handling/logging for both branches.
| let config = config::standard(); | ||
| if let Ok(Some(entry)) = self.cache.connection().get(url) { | ||
| let cached_value: CacheValue = bincode::deserialize(&entry).unwrap(); | ||
| if cached_value.expiration > SystemTime::now() { | ||
| let cached_value:(CacheValue, usize) = bincode::decode_from_slice(&entry, config).unwrap(); | ||
| if cached_value.0.expiration > SystemTime::now() { | ||
| // Return cached value if not expired | ||
| loading.success(format!("{} icon fetched from cache", url)); | ||
| loading.end(); | ||
| return Ok(cached_value.data); | ||
| return Ok(cached_value.0.data); | ||
| } else { |
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.
🛠️ Refactor suggestion
Don’t panic on corrupt/old cache; handle bincode decode errors gracefully.
unwrap() on decode_from_slice will crash the process if bytes are malformed (e.g., version bump, partial writes). Treat it as a cache miss and evict.
Apply this diff:
- let config = config::standard();
- if let Ok(Some(entry)) = self.cache.connection().get(url) {
- let cached_value:(CacheValue, usize) = bincode::decode_from_slice(&entry, config).unwrap();
- if cached_value.0.expiration > SystemTime::now() {
+ let config = config::standard();
+ if let Ok(Some(entry)) = self.cache.connection().get(url) {
+ match bincode::decode_from_slice::<CacheValue, _>(&entry, config) {
+ Ok((cached_value, _)) if cached_value.expiration > SystemTime::now() => {
// Return cached value if not expired
loading.success(format!("{} icon fetched from cache", url));
loading.end();
- return Ok(cached_value.0.data);
- } else {
- // Remove expired cache
- self.cache.connection().remove(url).unwrap();
- }
+ return Ok(cached_value.data);
+ }
+ Ok((_cached_value, _)) => {
+ // Remove expired cache
+ let _ = self.cache.connection().remove(url);
+ }
+ Err(_) => {
+ // Corrupt/incompatible cache; evict
+ let _ = self.cache.connection().remove(url);
+ }
+ }
}📝 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.
| let config = config::standard(); | |
| if let Ok(Some(entry)) = self.cache.connection().get(url) { | |
| let cached_value: CacheValue = bincode::deserialize(&entry).unwrap(); | |
| if cached_value.expiration > SystemTime::now() { | |
| let cached_value:(CacheValue, usize) = bincode::decode_from_slice(&entry, config).unwrap(); | |
| if cached_value.0.expiration > SystemTime::now() { | |
| // Return cached value if not expired | |
| loading.success(format!("{} icon fetched from cache", url)); | |
| loading.end(); | |
| return Ok(cached_value.data); | |
| return Ok(cached_value.0.data); | |
| } else { | |
| let config = config::standard(); | |
| if let Ok(Some(entry)) = self.cache.connection().get(url) { | |
| match bincode::decode_from_slice::<CacheValue, _>(&entry, config) { | |
| Ok((cached_value, _)) if cached_value.expiration > SystemTime::now() => { | |
| // Return cached value if not expired | |
| loading.success(format!("{} icon fetched from cache", url)); | |
| loading.end(); | |
| return Ok(cached_value.data); | |
| } | |
| Ok((_cached_value, _)) => { | |
| // Remove expired cache | |
| let _ = self.cache.connection().remove(url); | |
| } | |
| Err(_) => { | |
| // Corrupt/incompatible cache; evict | |
| let _ = self.cache.connection().remove(url); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In rust-plugins/icons/src/cache/mod.rs around lines 27 to 35, the code currently
calls bincode::decode_from_slice(...).unwrap() which will panic on
malformed/corrupt cached bytes; change this to handle decode errors gracefully
by matching the Result returned by decode_from_slice and treating any Err as a
cache miss: log a warning about corrupt cache for the given url, evict/delete
the cache entry (call the same removal path used for expired entries), and
continue as if the entry was not present (do not return or propagate a panic);
only proceed to check expiration and return the cached data on Ok(decoded) with
a valid expiration. Ensure no unwraps remain and use safe error handling so
corrupt/old cache entries are removed and do not crash the process.
| const ComponentC: typeof import('./../components/ComponentC')['ComponentC'] | ||
| const ComponentA: typeof import('./../components/ComponentA')['default'] | ||
| const ComponentE: typeof import('./../components/ComponentE')['ComponentE'] | ||
| const ComponentG: typeof import('./../components/ComponentE')['ComponentG'] | ||
| const ComponentD: typeof import('./../components/ComponentD')['default'] | ||
| const ComponentX: typeof import('./../components/ComponentD')['ComponentX'] | ||
| const ComponentB: typeof import('./../components/ComponentB')['ComponentB'] | ||
| const ComponentC: typeof import('./../components/ComponentC')['ComponentC'] | ||
| const ComponentD: typeof import('./../components/ComponentD')['default'] | ||
| const ComponentG: typeof import('./../components/ComponentE')['ComponentG'] | ||
| const ComponentF: typeof import('./../components/ComponentE')['ComponentF'] |
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.
💡 Verification agent
🧩 Analysis chain
Confirm cross-file component exports (ComponentG, ComponentX).
These entries assume:
- ComponentE exports ComponentE, ComponentF, ComponentG.
- ComponentD exports default and named ComponentX.
Please verify to avoid ambient type drift at compile time.
Run to check files/exports:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
base="rust-plugins/react-components/playground/src/components"
# Resolve module files
find_mod() {
fd -a -t f -e ts -e tsx -e js -e jsx "^$1\.(t|j)sx?$" "$base" | head -n1
}
check_named() {
local file="$1" name="$2"
rg -nP "export\s+(const|function|class)\s+$name\b|export\s*{\s*[^}]*\b$name\b" "$file" >/dev/null || {
echo "MISSING named export $name in $file"
}
}
check_default() {
local file="$1"
rg -nP 'export\s+default\s+' "$file" >/dev/null || {
echo "MISSING default export in $file"
}
}
declare -a checks=(
"ComponentA:default"
"ComponentB:ComponentB"
"ComponentC:ComponentC"
"ComponentD:default"
"ComponentD:ComponentX"
"ComponentE:ComponentE"
"ComponentE:ComponentF"
"ComponentE:ComponentG"
)
for entry in "${checks[@]}"; do
module="${entry%%:*}"
exportName="${entry##*:}"
file="$(find_mod "$module" || true)"
if [ -z "${file:-}" ]; then echo "MISSING FILE: $module.* under $base"; continue; fi
if [ "$exportName" = "default" ]; then check_default "$file"; else check_named "$file" "$exportName"; fi
doneLength of output: 7643
Export missing named components or update type definitions
ComponentC.tsxdoesn’t export a namedComponentCbut is referenced as such incomponents.d.tsComponentE.tsxdoesn’t export a namedComponentFbut is referenced as such incomponents.d.ts
Add the missing named exports in those modules or adjust the type definitions to use their default exports to prevent ambient type drift.
🤖 Prompt for AI Agents
In rust-plugins/react-components/playground/src/types/components.d.ts around
lines 4 to 11, the type declarations reference named exports that don't exist
(ComponentC and ComponentF) causing ambient type drift; update the modules to
export those named components or change the type declarations to reference the
modules' default exports. Specifically either add named exports ComponentC in
src/components/ComponentC.tsx and ComponentF in src/components/ComponentE.tsx
(export const ComponentC = ... / export const ComponentF = ... or export {
ComponentC, ComponentF } as appropriate), or modify the corresponding lines in
components.d.ts to use typeof import('./../components/ComponentC')['default']
and typeof import('./../components/ComponentE')['default'] so the types match
the actual default exports.
| use farmfe_plugin_react_components::Dts; | ||
| use farmfe_plugin_react_components::ImportMode; | ||
| use farmfe_plugin_react_components::{FarmPluginReactComponents, Options}; | ||
| use rustc_hash::FxHashMap; |
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.
💡 Verification agent
🧩 Analysis chain
FxHashMap import aligns with Farm v2 types; check dev-deps.
Looks good. Ensure rustc_hash is declared for this crate (deps or dev-deps), otherwise the test won’t compile.
Run to verify:
🏁 Script executed:
#!/bin/bash
# Verify rustc_hash is declared for react-components crate
rg -n -C2 -g 'rust-plugins/react-components/**/Cargo.toml' $'^(\\[dev-dependencies\\]|\\[dependencies\\])|^\\s*rustc_hash\\s*='Length of output: 460
Add rustc_hash to dev-dependencies
The tests in rust-plugins/react-components/tests/mod.rs use rustc_hash::FxHashMap, but rustc_hash isn’t declared in Cargo.toml. Add it under [dev-dependencies] in rust-plugins/react-components/Cargo.toml so the tests compile.
🤖 Prompt for AI Agents
In rust-plugins/react-components/tests/mod.rs around line 21, the test imports
rustc_hash::FxHashMap but rustc_hash is not declared in the crate's Cargo.toml;
open rust-plugins/react-components/Cargo.toml and add rustc_hash under the
[dev-dependencies] section (use the project/workspace preferred version or a
compatible version), save and run cargo test to confirm the tests compile.
| let transform_output_file_name_params = TransformOutputFileNameParams { | ||
| filename_config, | ||
| name: filename, | ||
| name_hash: "", | ||
| bytes: &raw_bytes, | ||
| ext, | ||
| special_placeholders: &Default::default() | ||
| }; | ||
| let output_file_name = transform_output_filename(transform_output_file_name_params); | ||
| res = format!("{}{}", &public_path, &output_file_name); |
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.
🛠️ Refactor suggestion
Avoid referencing a temporary Default and inline the params
&Default::default() creates a short-lived temporary; inline the struct in the call (or bind a local) to satisfy lifetimes. Also, you can reuse raw_bytes to avoid a second read below.
- let transform_output_file_name_params = TransformOutputFileNameParams {
- filename_config,
- name: filename,
- name_hash: "",
- bytes: &raw_bytes,
- ext,
- special_placeholders: &Default::default()
- };
- let output_file_name = transform_output_filename(transform_output_file_name_params);
+ let output_file_name = transform_output_filename(TransformOutputFileNameParams {
+ filename_config,
+ name: filename,
+ name_hash: "",
+ bytes: &raw_bytes,
+ ext,
+ special_placeholders: &Default::default(),
+ });
@@
- let content = read_file_raw(param.resolved_path).unwrap();
+ let content = raw_bytes;📝 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.
| let transform_output_file_name_params = TransformOutputFileNameParams { | |
| filename_config, | |
| name: filename, | |
| name_hash: "", | |
| bytes: &raw_bytes, | |
| ext, | |
| special_placeholders: &Default::default() | |
| }; | |
| let output_file_name = transform_output_filename(transform_output_file_name_params); | |
| res = format!("{}{}", &public_path, &output_file_name); | |
| let output_file_name = transform_output_filename(TransformOutputFileNameParams { | |
| filename_config, | |
| name: filename, | |
| name_hash: "", | |
| bytes: &raw_bytes, | |
| ext, | |
| special_placeholders: &Default::default(), | |
| }); | |
| res = format!("{}{}", &public_path, &output_file_name); | |
| let content = raw_bytes; |
🤖 Prompt for AI Agents
In rust-plugins/url/src/lib.rs around lines 130 to 139, avoid passing a
temporary reference &Default::default() into TransformOutputFileNameParams
(which creates a short-lived temporary and can cause lifetime issues) and stop
re-reading the file bytes; instead create a local variable let
special_placeholders = Default::default() and pass &special_placeholders to
TransformOutputFileNameParams (or inline the struct into the function call but
bind special_placeholders to a local), and reuse the already-read raw_bytes
variable for the bytes field so you don’t read the file a second time.
| "@farmfe/core": "^2.0.0-bate.0", | ||
| "@farmfe/plugin-react": "^2.0.0-bate.0", |
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.
Fix version typo: "bate" → "beta".
Same prerelease misspelling as other playground; will break installs.
- "@farmfe/core": "^2.0.0-bate.0",
- "@farmfe/plugin-react": "^2.0.0-bate.0",
+ "@farmfe/core": "^2.0.0-beta.0",
+ "@farmfe/plugin-react": "^2.0.0-beta.0",📝 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.
| "@farmfe/core": "^2.0.0-bate.0", | |
| "@farmfe/plugin-react": "^2.0.0-bate.0", | |
| "@farmfe/core": "^2.0.0-beta.0", | |
| "@farmfe/plugin-react": "^2.0.0-beta.0", |
🤖 Prompt for AI Agents
In rust-plugins/wasm/playground/package.json around lines 22 to 23, the
prerelease tag "bate" is misspelled; replace "bate" with "beta" in the version
strings for both "@farmfe/core" and "@farmfe/plugin-react" so the dependencies
use the correct prerelease tag and installs won't fail.
| "@farmfe/core": "^2.0.0-bate.0", | ||
| "@farmfe/plugin-react": "^2.0.0-bate.0", |
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.
Fix version typo: "bate" → "beta".
The prerelease tag is misspelled and will fail resolution.
- "@farmfe/core": "^2.0.0-bate.0",
- "@farmfe/plugin-react": "^2.0.0-bate.0",
+ "@farmfe/core": "^2.0.0-beta.0",
+ "@farmfe/plugin-react": "^2.0.0-beta.0",📝 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.
| "@farmfe/core": "^2.0.0-bate.0", | |
| "@farmfe/plugin-react": "^2.0.0-bate.0", | |
| "@farmfe/core": "^2.0.0-beta.0", | |
| "@farmfe/plugin-react": "^2.0.0-beta.0", |
🤖 Prompt for AI Agents
In rust-plugins/worker/playground/package.json around lines 22 to 23 the
prerelease tag "bate" is misspelled in the dependency versions; update both
occurrences ("^2.0.0-bate.0") to the correct prerelease tag ("^2.0.0-beta.0") so
the package resolver can match the intended beta versions.
update farm v2-beta
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
rust-plugins/worker/src/lib.rs (3)
116-132: Compute name/extension robustly and use a stable hash base.
split_once(".")breaks on multi-dot filenames; usePathhelpers.- Hashing with
module_id.as_bytes()makes output depend on the importer; useresolved_path(or content) for deterministic worker file names.- Prefer normalizing to
.mjsfor both.jsand TS-like extensions.- let file_name_ext = Path::new(resolved_path) - .file_name() - .map(|x| x.to_string_lossy().to_string()) - .unwrap_or_else(|| "".to_string()); - let (file_name, ext) = file_name_ext.split_once(".").unwrap(); - let assets_filename_config = compiler_config.output.assets_filename.clone(); - let transform_output_file_name_params = TransformOutputFileNameParams { - filename_config: assets_filename_config, - name: file_name, - name_hash: "", - bytes: &module_id.as_bytes(), - ext, - special_placeholders: &Default::default(), - }; + let path = Path::new(resolved_path); + let file_stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or_default(); + let ext = path.extension().and_then(|s| s.to_str()).unwrap_or_default(); + let assets_filename_config = compiler_config.output.assets_filename.clone(); + let transform_output_file_name_params = TransformOutputFileNameParams { + filename_config: assets_filename_config, + name: file_stem, + name_hash: "", + // Use resolved_path for stable, content-agnostic hashing; consider using bytes after build if content hashing is desired. + bytes: resolved_path.as_bytes(), + ext, + special_placeholders: &Default::default(), + }; // hash_bytes = resolved_path + file_name_ext bytes ,make sure that the files of the same name in different directory will not be covered; - let file_name = transform_output_filename(transform_output_file_name_params); - // worker.ts -> worker.js - let file_name = if file_name.ends_with(".ts") { - file_name.replace(".ts", ".js") - } else { - file_name - }; + let file_name = transform_output_filename(transform_output_file_name_params); + // Normalize to .mjs for esm workers + let file_name = match Path::new(&file_name).extension().and_then(|s| s.to_str()) { + Some("ts") | Some("tsx") | Some("mts") | Some("cts") | Some("js") => { + let (stem, _) = file_name.rsplit_once('.').unwrap(); + format!("{stem}.mjs") + } + _ => file_name, + };
210-213: Cache key mismatch in inline mode (panic risk).You insert/fetch with
file_nameelsewhere, but here you read withresolved_path. This willunwrap()onNone.- let content_bytes = worker_cache.get(resolved_path).unwrap(); + let content_bytes = worker_cache.get(&file_name).unwrap();
391-401: Wrong capture group for string insidenew URL(...).
captures[1]spans the entirenew URL(...); you then slice off quotes incorrectly. Use the inner string literal capture instead.- let args = &m.captures[0].clone().unwrap(); - let worker_url = &m.captures[1].clone().unwrap(); - let arg_code = ¶m.content[args.start..args.end]; - let worker_url_code = ¶m.content[worker_url.start..worker_url.end]; + let args = &m.captures[0].clone().unwrap(); // full `new Worker(...` + let url_cap = &m.captures[2].clone().unwrap(); // string literal inside new URL(...) + let arg_code = ¶m.content[args.start..args.end]; + let worker_url_code = ¶m.content[url_cap.start..url_cap.end]; if arg_code.contains("`") && arg_code.contains("${") { println!("new URL(url, import.meta.url) is not supported in dynamic template string.") } else { let compiler_config = self.options.compiler_config.as_ref().unwrap(); - let worker_url = &worker_url_code[1..worker_url_code.len() - 1]; + let worker_url = &worker_url_code[1..worker_url_code.len() - 1]; // strip quotes
🧹 Nitpick comments (8)
rust-plugins/worker/playground/src/index.tsx (2)
3-7: Drop committed commented-out code (or guard behind a flag).These debug/toggle lines add noise. If you need them for local switching, wrap behind a dev flag; otherwise remove.
-// import TestWorker from "./worker/test.worker?worker" - -// console.log(TestWorker); -// const worker = new TestWorker();
17-19: Mirror options and basic error handling for worker2; logging label is good.Align instantiation with worker1 and add error hooks; the labeled log helps triage.
-const worker2 = new Worker(new URL("./worker/vue.worker.ts",import.meta.url)) +const worker2 = new Worker(new URL('./worker/vue.worker.ts', import.meta.url), { type: 'module' }) worker2.postMessage([2, 3]); worker2.onmessage = (e) => { console.log('vue worker', e.data); } +worker2.onerror = (err) => console.error('vue worker error', err); +worker2.onmessageerror = (e) => console.warn('vue worker messageerror', e);rust-plugins/compress/playground/package.json (1)
1-27: Consider declaring Node.js engine to match Farm v2 toolchain.If v2 expects a minimum Node (e.g., >=18.18), add an engines field to prevent accidental installs on older Node.
{ "name": "playground", "version": "1.0.0", "type": "module", + "engines": { + "node": ">=18.18" + }, "scripts": {rust-plugins/compress/src/lib.rs (2)
96-101: Tighten regex match and avoid needless borrowing.
filter.is_match(&resource_id)adds an extra layer of referencing; pass&strdirectly. Also, consider short-circuiting onthresholdfirst to avoid a regex call on small files (micro).- if !filter.is_match(&resource_id) || resource.bytes.len() < self.options.threshold { + if resource.bytes.len() < self.options.threshold || !filter.is_match(resource_id) { return None; }
136-136: Remove needless borrow in sha256 call.Avoid
&&[u8].- name_hash: sha256(&name.as_bytes(), 8), + name_hash: sha256(name.as_bytes(), 8),rust-plugins/worker/src/lib.rs (3)
73-85: Config threading looks right; minor suggestion oninputkey.Feeding
full_file_nameas the virtual entry key ties downstream lookups to that string. If you replace the extension later, prefer the stem as the key to reduce surprises.- input.insert(full_file_name.clone(), resolved_path.to_string()); + input.insert( + Path::new(&full_file_name) + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or(&full_file_name) + .to_string(), + resolved_path.to_string(), + );
404-417: Avoid.unwrap()chain on resolve.Downstream panics on resolution failures are hard to debug. Propagate an error or early-return
Ok(None).- .unwrap() - .unwrap() - .resolved_path + .ok_or_else(|| anyhow::anyhow!("Worker resolve failed for {worker_url}"))? + .ok_or_else(|| anyhow::anyhow!("Worker unresolved for {worker_url}"))? + .resolved_path
451-487: Cache (de)serialization with expandedResourcestill compatible.Emitting cached assets back is fine; consider logging count for observability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockrust-plugins/compress/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
rust-plugins/compress/Cargo.toml(1 hunks)rust-plugins/compress/playground/package.json(1 hunks)rust-plugins/compress/src/lib.rs(3 hunks)rust-plugins/worker/playground/src/index.tsx(1 hunks)rust-plugins/worker/src/lib.rs(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- rust-plugins/compress/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (2)
rust-plugins/compress/src/lib.rs (1)
rust-plugins/compress/src/utils.rs (1)
get_ext_name(90-96)
rust-plugins/worker/src/lib.rs (1)
rust-plugins/worker/src/cache.rs (1)
new(10-14)
🔇 Additional comments (5)
rust-plugins/compress/src/lib.rs (1)
80-83: API alignment looks good.
PluginFinalizeResourcesHookParamupdate matches v2 API changes.rust-plugins/worker/src/lib.rs (4)
171-183: Good: dedup emit via worker cache.The up-front cache check and conditional re-emit are sound. Fixing the inline cache key will complete this path.
198-201: Format switch aligns with v2 API.Using
ModuleFormatConfig::Single(ModuleFormat::EsModule)here looks correct.
291-301: Options wiring: host_config defaulting toconfig.clone()is sensible.No issues spotted.
122-132: Unify hashing base across paths.Currently, the load-path uses
module_idas hash bytes while the transform-path passesresolved_path. Pick one (recommendresolved_path) to avoid duplicate filenames for the same worker across importers.I recommend the
resolved_path.as_bytes()change proposed above; please confirm no tests rely on importer-scoped filenames.Also applies to: 171-173, 419-425
| "@farmfe/cli": "^1.0.4", | ||
| "@farmfe/core": "^1.6.6", | ||
| "@farmfe/plugin-react": "^1.2.6", | ||
| "@farmfe/core": "^2.0.0-beta.2", | ||
| "@farmfe/plugin-react": "^2.0.0-beta.2", |
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.
💡 Verification agent
🧩 Analysis chain
Align CLI to v2 to avoid runtime incompatibilities with Core v2.
Core and React plugin are upgraded to 2.0.0-beta.2, but CLI remains on 1.0.4. If Farm v2 requires CLI v2, farm start/build in this playground may break or use incompatible flags.
Proposed change:
"devDependencies": {
- "@farmfe/cli": "^1.0.4",
+ "@farmfe/cli": "^2.0.0-beta.2",
"@farmfe/core": "^2.0.0-beta.2",
"@farmfe/plugin-react": "^2.0.0-beta.2",To verify consistency and availability, run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "— Scan repo for Farm CLI/Core versions —"
fd -t f package.json | while read -r f; do
jq -r --arg file "$f" '
"FILE=" + $file
+ " NAME=" + (.name // "unknown")
+ " CLI=" + (.devDependencies["@farmfe/cli"] // .dependencies["@farmfe/cli"] // "N/A")
+ " CORE=" + (.devDependencies["@farmfe/core"] // .dependencies["@farmfe/core"] // "N/A")
' "$f"
done
echo; echo "— NPM dist-tags for @farmfe/cli and @farmfe/core —"
for pkg in cli core; do
echo "@farmfe/$pkg dist-tags:"
curl -s https://registry.npmjs.org/@farmfe/$pkg | jq -r '."dist-tags"'
doneLength of output: 84828
Bump CLI to available v2 pre-release
In rust-plugins/compress/playground/package.json, update the CLI to the latest v2 beta (0) since 2.0.0-beta.2 doesn’t exist:
rust-plugins/compress/playground/package.json
"devDependencies": {
- "@farmfe/cli": "^1.0.4",
+ "@farmfe/cli": "^2.0.0-beta.0",
"@farmfe/core": "^2.0.0-beta.2",
"@farmfe/plugin-react": "^2.0.0-beta.2",📝 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.
| "@farmfe/cli": "^1.0.4", | |
| "@farmfe/core": "^1.6.6", | |
| "@farmfe/plugin-react": "^1.2.6", | |
| "@farmfe/core": "^2.0.0-beta.2", | |
| "@farmfe/plugin-react": "^2.0.0-beta.2", | |
| "devDependencies": { | |
| "@farmfe/cli": "^2.0.0-beta.0", | |
| "@farmfe/core": "^2.0.0-beta.2", | |
| "@farmfe/plugin-react": "^2.0.0-beta.2", |
🤖 Prompt for AI Agents
In rust-plugins/compress/playground/package.json around lines 19 to 21, the
dependency "@farmfe/cli" is pinned to "^1.0.4" but the PR attempted to set it to
a non-existent "2.0.0-beta.2"; update the "@farmfe/cli" entry to the correct v2
pre-release (for example "^2.0.0-beta.0") so the package.json references an
available beta release, leaving other dependencies unchanged.
| saved += origin_file_size - bytes.len(); | ||
| param.resources_map.insert( |
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.
🛠️ Refactor suggestion
Fix underflow when compressed size > original.
saved is usize; origin_file_size - bytes.len() will wrap if compression inflates. Use saturating_sub (and optionally skip emitting larger artifacts).
Apply:
- saved += origin_file_size - bytes.len();
+ let delta = origin_file_size.saturating_sub(bytes.len());
+ saved += delta;
+ // Optional: only emit if compression helps
+ // if delta == 0 { continue; }📝 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.
| saved += origin_file_size - bytes.len(); | |
| param.resources_map.insert( | |
| let delta = origin_file_size.saturating_sub(bytes.len()); | |
| saved += delta; | |
| // Optional: only emit if compression helps | |
| // if delta == 0 { continue; } | |
| param.resources_map.insert( |
🤖 Prompt for AI Agents
In rust-plugins/compress/src/lib.rs around lines 127-128, the code does `saved
+= origin_file_size - bytes.len()` which can underflow if the compressed size
(bytes.len()) is larger than origin_file_size; change the subtraction to use
`origin_file_size.saturating_sub(bytes.len())` (or compute a `saved_delta =
origin_file_size.saturating_sub(bytes.len())` and add that) and consider
skipping or not emitting artifacts when compressed size >= original to avoid
counting negative savings and producing larger outputs.
| const worker = new Worker(new URL("/src/worker/test.worker.ts",import.meta.url)); | ||
| worker.postMessage([5, 5]); | ||
| worker.onmessage = (e) => { | ||
| console.log(e.data); | ||
| console.log('test worker', e.data); | ||
| } |
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.
🛠️ Refactor suggestion
Avoid absolute “/src/…” in Worker URL; treat worker as ESM.
Using an absolute path ties to dev server root and can break in build/prod. Prefer relative URL and pass { type: 'module' } for ESM workers.
-const worker = new Worker(new URL("/src/worker/test.worker.ts",import.meta.url));
+const worker = new Worker(new URL('./worker/test.worker.ts', import.meta.url), { type: 'module' });
worker.postMessage([5, 5]);
worker.onmessage = (e) => {
console.log('test worker', e.data);
}
+worker.onerror = (err) => console.error('test worker error', err);
+worker.onmessageerror = (e) => console.warn('test worker messageerror', e);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const worker = new Worker(new URL("/src/worker/test.worker.ts",import.meta.url)); | |
| worker.postMessage([5, 5]); | |
| worker.onmessage = (e) => { | |
| console.log(e.data); | |
| console.log('test worker', e.data); | |
| } | |
| const worker = new Worker( | |
| new URL('./worker/test.worker.ts', import.meta.url), | |
| { type: 'module' } | |
| ); | |
| worker.postMessage([5, 5]); | |
| worker.onmessage = (e) => { | |
| console.log('test worker', e.data); | |
| } | |
| worker.onerror = (err) => console.error('test worker error', err); | |
| worker.onmessageerror = (e) => console.warn('test worker messageerror', e); |
🤖 Prompt for AI Agents
In rust-plugins/worker/playground/src/index.tsx around lines 8 to 12, the Worker
is created with an absolute URL ("/src/...") which ties it to the dev server
root and can break in production; change the URL to a relative path (for example
"./worker/test.worker.ts" resolved against import.meta.url, i.e. new
URL("./worker/test.worker.ts", import.meta.url)) and instantiate the Worker with
the ESM option by passing { type: 'module' } as the second argument so the
worker is loaded as an ESM module.
| let resource_name = format!("{}.mjs", full_file_name); | ||
| let resource = resources_map.get(&resource_name).unwrap(); | ||
| let content_bytes = resource.bytes.clone(); |
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.
Don't append .mjs to a name that may already have an extension.
format!("{}.mjs", full_file_name) risks *.js.mjs. Replace the extension instead.
- let resource_name = format!("{}.mjs", full_file_name);
+ let resource_name = match full_file_name.rsplit_once('.') {
+ Some((stem, _)) => format!("{stem}.mjs"),
+ None => format!("{full_file_name}.mjs"),
+ };🤖 Prompt for AI Agents
In rust-plugins/worker/src/lib.rs around lines 90 to 92, you're naively
appending ".mjs" which can produce filenames like "foo.js.mjs"; instead replace
the file extension using Path utilities: build resource_name by taking
Path::new(&full_file_name).with_extension("mjs") and converting that PathBuf
back to a String (e.g., via to_string_lossy().into_owned()), then use that
resource_name to look up the resource; this ensures existing extensions are
replaced and files without extensions get ".mjs".
Summary by CodeRabbit
New Features
Improvements
Breaking Changes
Chores