diff --git a/.changeset/fix-preserve-symlinks.md b/.changeset/fix-preserve-symlinks.md new file mode 100644 index 000000000000..7081e881cc51 --- /dev/null +++ b/.changeset/fix-preserve-symlinks.md @@ -0,0 +1,7 @@ +--- +swc_ecma_transforms_module: patch +swc: patch +swc_core: patch +--- + +fix(transforms/module): replace `canonicalize()` with `path_clean` to avoid resolving symlinks during module resolution diff --git a/Cargo.lock b/Cargo.lock index 6e5e9d3b1bd6..511cdf643d2c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5545,6 +5545,7 @@ dependencies = [ "par-core", "par-iter", "parking_lot", + "path-clean 1.0.1", "regex", "rustc-hash 2.1.1", "serde", @@ -6684,6 +6685,7 @@ dependencies = [ "swc_ecma_transforms_typescript", "swc_ecma_utils", "swc_ecma_visit", + "tempfile", "testing", "tracing", ] diff --git a/crates/swc/Cargo.toml b/crates/swc/Cargo.toml index 1ea8dbcf8dab..e4da119bd14b 100644 --- a/crates/swc/Cargo.toml +++ b/crates/swc/Cargo.toml @@ -71,6 +71,7 @@ indexmap = { workspace = true, features = ["serde"] } jsonc-parser = { workspace = true, features = ["serde"] } once_cell = { workspace = true } par-core = { workspace = true } +path-clean = { workspace = true } par-iter = { workspace = true } parking_lot = { workspace = true } regex = { workspace = true } diff --git a/crates/swc/src/config/mod.rs b/crates/swc/src/config/mod.rs index 36a2a09f498d..34b4063a8ca5 100644 --- a/crates/swc/src/config/mod.rs +++ b/crates/swc/src/config/mod.rs @@ -16,6 +16,8 @@ use dashmap::DashMap; use either::Either; use indexmap::IndexMap; use once_cell::sync::Lazy; +#[cfg(feature = "module")] +use path_clean::PathClean; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use serde::{Deserialize, Serialize}; use swc_atoms::Atom; @@ -1612,9 +1614,55 @@ impl ModuleConfig { return None; } + // Normalize the base path without resolving symlinks. + // Using `.clean()` instead of `.canonicalize()` keeps symlinked + // paths intact, which is required for correct relative-path + // computation in `diff_paths` (both base and target must live + // in the same "path space"). + // + // On Windows, we still canonicalize absolute paths to keep the base + // path in UNC form. `build_resolver` canonicalizes `jsc.baseUrl` to + // UNC as well, and `diff_paths` requires both paths to be in the same + // form to produce relative paths consistently. + // + // https://github.com/swc-project/swc/issues/8265 + // https://github.com/swc-project/swc/issues/11584 let base = match base { FileName::Real(v) if !skip_resolver => { - FileName::Real(v.canonicalize().unwrap_or_else(|_| v.to_path_buf())) + let cleaned = if v.is_absolute() { + v.clean() + } else { + let relative = v.clean(); + + // If the relative input filename points to an existing file from + // cwd (CLI/manual use-cases), keep it in cwd path space. + // Otherwise (virtual/in-memory filenames), keep it relative so + // resolver logic can rebase through `jsc.baseUrl`. + env::current_dir() + .ok() + .map(|cwd| cwd.join(&relative).clean()) + .filter(|abs| abs.exists()) + .unwrap_or(relative) + }; + + #[cfg(target_os = "windows")] + let cleaned = if cleaned.is_absolute() + && !matches!( + cleaned.components().next(), + Some(std::path::Component::Prefix(prefix)) + if matches!( + prefix.kind(), + std::path::Prefix::Verbatim(_) + | std::path::Prefix::VerbatimDisk(_) + | std::path::Prefix::VerbatimUNC(_, _) + ) + ) { + cleaned.canonicalize().unwrap_or(cleaned) + } else { + cleaned + }; + + FileName::Real(cleaned) } _ => base.clone(), }; diff --git a/crates/swc/src/config/tests.rs b/crates/swc/src/config/tests.rs index cb7eae53af1f..f7bef9f4f01d 100644 --- a/crates/swc/src/config/tests.rs +++ b/crates/swc/src/config/tests.rs @@ -1,3 +1,8 @@ +#[cfg(feature = "module")] +use swc_common::FileName; + +#[cfg(feature = "module")] +use super::ModuleConfig; use crate::parse_swcrc; #[test] @@ -30,3 +35,163 @@ fn issue_6996() { let rc = parse_swcrc(include_str!("issue-6996.json")).expect("failed to parse"); dbg!(&rc); } + +#[cfg(feature = "module")] +#[test] +fn issue_11584_relative_base_is_rebased_against_base_url() { + use std::{ + env, fs, + path::PathBuf, + time::{SystemTime, UNIX_EPOCH}, + }; + + let uniq = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock should be monotonic") + .as_nanos(); + let tmp_root = env::temp_dir().join(format!("swc-issue-11584-{}-{}", std::process::id(), uniq)); + let base_url = tmp_root.join("project"); + + fs::create_dir_all(base_url.join("src")).expect("should create fixture directories"); + fs::write( + base_url.join("src").join("foo.ts"), + "export const foo = 1;\n", + ) + .expect("should create fixture file"); + + let base = FileName::Real(PathBuf::from("virtual/index.ts")); + let paths = vec![("@app/*".to_string(), vec!["src/*".to_string()])]; + + let (normalized_base, resolver) = ModuleConfig::get_resolver(&base_url, paths, &base, None) + .expect("resolver should be created"); + + let base_path = match &normalized_base { + FileName::Real(path) => path, + other => panic!("unexpected base filename: {other:?}"), + }; + assert!( + !base_path.is_absolute(), + "relative input filename should stay relative so resolver can rebase against jsc.baseUrl" + ); + + let resolved = resolver + .resolve_import(&normalized_base, "@app/foo") + .expect("import should resolve"); + assert_eq!( + &*resolved, "../src/foo", + "resolved import should stay in the jsc.baseUrl path space" + ); + + let _ = fs::remove_dir_all(&tmp_root); +} + +#[cfg(feature = "module")] +#[test] +fn issue_11584_existing_relative_base_uses_cwd_path_space() { + use std::{ + env, fs, + time::{SystemTime, UNIX_EPOCH}, + }; + + let cwd = env::current_dir().expect("should get current_dir"); + let uniq = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock should be monotonic") + .as_nanos(); + let tmp_root = cwd.join(format!( + "swc-issue-11584-existing-{}-{}", + std::process::id(), + uniq + )); + let base_url = tmp_root.join("src"); + let base_file = base_url.join("index.ts"); + let dep_file = base_url.join("modules").join("moduleA").join("index.ts"); + + fs::create_dir_all(dep_file.parent().expect("dep parent should exist")) + .expect("should create fixture directories"); + fs::write(&base_file, "import { moduleA } from '@modules/moduleA';\n") + .expect("should create base file"); + fs::write(&dep_file, "export const moduleA = () => {};\n").expect("should create dep file"); + + let base_relative = base_file + .strip_prefix(&cwd) + .expect("fixture path should be under cwd") + .to_path_buf(); + let base = FileName::Real(base_relative); + let paths = vec![("@modules/*".to_string(), vec!["./modules/*".to_string()])]; + + let (normalized_base, resolver) = ModuleConfig::get_resolver(&base_url, paths, &base, None) + .expect("resolver should be created"); + + let normalized_path = match &normalized_base { + FileName::Real(path) => path, + other => panic!("unexpected base filename: {other:?}"), + }; + assert!( + normalized_path.is_absolute(), + "existing relative filename should be normalized into cwd path space" + ); + + let resolved = resolver + .resolve_import(&normalized_base, "@modules/moduleA") + .expect("import should resolve"); + assert_eq!( + &*resolved, "./modules/moduleA", + "resolved import should stay relative to src/ for existing relative filenames" + ); + + let _ = fs::remove_dir_all(&tmp_root); +} + +#[cfg(all(feature = "module", target_os = "windows"))] +#[test] +fn issue_11584_windows_absolute_base_is_unc() { + use std::{ + env, fs, + time::{SystemTime, UNIX_EPOCH}, + }; + + let uniq = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("clock should be monotonic") + .as_nanos(); + let tmp_root = env::temp_dir().join(format!( + "swc-issue-11584-win-{}-{}", + std::process::id(), + uniq + )); + let base_url = tmp_root.join("project"); + let src_dir = base_url.join("src"); + let entry = src_dir.join("index.ts"); + + fs::create_dir_all(&src_dir).expect("should create fixture directories"); + fs::write(&entry, "export const value = 1;\n").expect("should create fixture file"); + + let base = FileName::Real(entry); + let paths = vec![("@app/*".to_string(), vec!["src/*".to_string()])]; + let (normalized_base, _) = ModuleConfig::get_resolver(&base_url, paths, &base, None) + .expect("resolver should be created"); + + let normalized_path = match &normalized_base { + FileName::Real(path) => path, + other => panic!("unexpected base filename: {other:?}"), + }; + + let is_unc = matches!( + normalized_path.components().next(), + Some(std::path::Component::Prefix(prefix)) + if matches!( + prefix.kind(), + std::path::Prefix::Verbatim(_) + | std::path::Prefix::VerbatimDisk(_) + | std::path::Prefix::VerbatimUNC(_, _) + ) + ); + assert!( + is_unc, + "normalized Windows base path should be UNC, got: {}", + normalized_path.display() + ); + + let _ = fs::remove_dir_all(&tmp_root); +} diff --git a/crates/swc_ecma_transforms_module/Cargo.toml b/crates/swc_ecma_transforms_module/Cargo.toml index a681628b773e..3463517665d9 100644 --- a/crates/swc_ecma_transforms_module/Cargo.toml +++ b/crates/swc_ecma_transforms_module/Cargo.toml @@ -46,6 +46,7 @@ swc_ecma_visit = { version = "20.0.0", path = "../swc_ecma_visit" } [dev-dependencies] indexmap = { workspace = true, features = ["serde"] } serde_json = { workspace = true } +tempfile = { workspace = true } swc_ecma_loader = { version = "18.0.0", path = "../swc_ecma_loader", features = [ "node", diff --git a/crates/swc_ecma_transforms_module/src/path.rs b/crates/swc_ecma_transforms_module/src/path.rs index 708138be7ecf..60e85629e35d 100644 --- a/crates/swc_ecma_transforms_module/src/path.rs +++ b/crates/swc_ecma_transforms_module/src/path.rs @@ -1,7 +1,6 @@ use std::{ borrow::Cow, env::current_dir, - fs::canonicalize, io, path::{Component, Path, PathBuf}, sync::Arc, @@ -254,13 +253,15 @@ where } }; - // Bazel uses symlink + // Clean the resolved path to normalize `.` and `..` components + // without resolving symlinks. Previously this used `canonicalize()` + // which resolved symlinks, breaking setups where symlinked source + // files need imports resolved relative to the symlink location. // // https://github.com/swc-project/swc/issues/8265 - if let FileName::Real(resolved) = &target.filename { - if let Ok(orig) = canonicalize(resolved) { - target.filename = FileName::Real(orig); - } + // https://github.com/swc-project/swc/issues/11584 + if let FileName::Real(resolved) = &mut target.filename { + *resolved = resolved.clean(); } let Resolution { diff --git a/crates/swc_ecma_transforms_module/tests/path_node.rs b/crates/swc_ecma_transforms_module/tests/path_node.rs index d4dc3eea5dd6..78e76d4b3abe 100644 --- a/crates/swc_ecma_transforms_module/tests/path_node.rs +++ b/crates/swc_ecma_transforms_module/tests/path_node.rs @@ -13,6 +13,7 @@ use swc_ecma_transforms_module::{ rewriter::import_rewriter, }; use swc_ecma_transforms_testing::{test_fixture, FixtureTestConfig}; +use tempfile::TempDir; use testing::run_test2; type TestProvider = NodeImportResolver; @@ -157,3 +158,119 @@ fn fixture(input_dir: PathBuf) { }, ); } + +/// Test for https://github.com/swc-project/swc/issues/11584 +/// +/// `NodeImportResolver` should not resolve symlinks when computing +/// relative import paths. This ensures that symlinked source files +/// resolve imports relative to the symlink location, not the real +/// file location. +/// +/// Directory structure: +/// tmpdir/ +/// real/ +/// lib/ +/// dep.js <- real file +/// project/ +/// lib/ <- symlink -> ../../real/lib +/// src/ +/// index.js <- real file, imports ../lib/dep +#[cfg(unix)] +#[test] +fn issue_11584_symlink_not_canonicalized() { + use std::{fs, os::unix::fs as unix_fs}; + + let tmpdir = TempDir::new().unwrap(); + let base_dir = tmpdir.path().canonicalize().unwrap(); + + let real_lib = base_dir.join("real").join("lib"); + let project_dir = base_dir.join("project"); + let project_src = project_dir.join("src"); + + fs::create_dir_all(&real_lib).unwrap(); + fs::create_dir_all(&project_src).unwrap(); + + // Create the real dep file + fs::write( + real_lib.join("dep.js"), + "module.exports.VALUE = \"hello\";\n", + ) + .unwrap(); + + // Create source file in project/src/ + fs::write( + project_src.join("index.js"), + "import { VALUE } from \"../lib/dep\";\n", + ) + .unwrap(); + + // Create symlink: project/lib -> ../real/lib + unix_fs::symlink(&real_lib, project_dir.join("lib")).unwrap(); + + // The base filename is in project/src/ (a real file, not a symlink). + // The import ../lib/dep resolves to project/lib/dep.js through the symlink. + let base = FileName::Real(project_src.join("index.js")); + + let resolver = NodeImportResolver::with_config( + NodeModulesResolver::new(swc_ecma_loader::TargetEnv::Node, Default::default(), true), + swc_ecma_transforms_module::path::Config { + base_dir: Some(base_dir.clone()), + ..Default::default() + }, + ); + + let result = resolver.resolve_import(&base, "../lib/dep").unwrap(); + // The resolved path should stay relative to the symlink location + // (project/lib/) instead of being canonicalized to the real location + // (real/lib/), which would produce ../../real/lib/dep.js. + assert_eq!( + &*result, "../lib/dep.js", + "Symlink path should be preserved, not canonicalized to real path" + ); +} + +/// Test for use cases discussed in +/// https://github.com/swc-project/swc/pull/11585#issuecomment-3993466331 +/// +/// In a pnpm-like layout, `node_modules/@a/pkg` may be a symlink to +/// `node_modules/.pnpm/.../node_modules/@a/pkg`. Both a package import and +/// an explicit `./node_modules` import should preserve the original specifier. +#[cfg(unix)] +#[test] +fn issue_11585_pnpm_node_modules_symlink() { + use std::{fs, os::unix::fs as unix_fs}; + + let tmpdir = TempDir::new().unwrap(); + let project_dir = tmpdir.path().join("project"); + let node_modules = project_dir.join("node_modules"); + let pnpm_pkg = node_modules + .join(".pnpm") + .join("@a+pkg@1.0.0") + .join("node_modules") + .join("@a") + .join("pkg"); + + fs::create_dir_all(&pnpm_pkg).unwrap(); + fs::create_dir_all(node_modules.join("@a")).unwrap(); + fs::write(pnpm_pkg.join("index.js"), "export const value = 1;\n").unwrap(); + fs::write(project_dir.join("index.js"), "import '@a/pkg';\n").unwrap(); + + unix_fs::symlink(&pnpm_pkg, node_modules.join("@a").join("pkg")).unwrap(); + + let base = FileName::Real(project_dir.join("index.js")); + let resolver = NodeImportResolver::with_config( + NodeModulesResolver::new(swc_ecma_loader::TargetEnv::Node, Default::default(), true), + swc_ecma_transforms_module::path::Config { + base_dir: Some(project_dir.clone()), + ..Default::default() + }, + ); + + let package_import = resolver.resolve_import(&base, "@a/pkg").unwrap(); + assert_eq!(&*package_import, "@a/pkg"); + + let explicit_node_modules_import = resolver + .resolve_import(&base, "./node_modules/@a/pkg") + .unwrap(); + assert_eq!(&*explicit_node_modules_import, "./node_modules/@a/pkg"); +}