From 99fb7fa89a6ff696a0f6eb9c62c0493e9c08b80b Mon Sep 17 00:00:00 2001 From: jdalton Date: Wed, 29 Jul 2026 02:50:26 -0400 Subject: [PATCH] fix(resolve): walk every ancestor node_modules for bare specifiers (pnpm layouts) --- .../7001-ancestor-node-modules-walk.md | 1 + crates/perry/src/commands/compile/resolve.rs | 134 +++++++++++------- .../src/commands/compile/resolve/tests.rs | 39 +++++ 3 files changed, 120 insertions(+), 54 deletions(-) create mode 100644 changelog.d/7001-ancestor-node-modules-walk.md diff --git a/changelog.d/7001-ancestor-node-modules-walk.md b/changelog.d/7001-ancestor-node-modules-walk.md new file mode 100644 index 00000000000..26de653328b --- /dev/null +++ b/changelog.d/7001-ancestor-node-modules-walk.md @@ -0,0 +1 @@ +fix(resolve): consult every ancestor `node_modules` directory when resolving a bare specifier, matching Node's algorithm. The walk previously stopped at the first `node_modules` found — under pnpm a package's own `node_modules` holds only `.bin`, so every transitive dependency of a compiled package (living in the `.pnpm/@/node_modules` sibling) was unresolvable and its call sites linked as undefined raw symbols. diff --git a/crates/perry/src/commands/compile/resolve.rs b/crates/perry/src/commands/compile/resolve.rs index 48fb62e87d3..cb9dd5df4f0 100644 --- a/crates/perry/src/commands/compile/resolve.rs +++ b/crates/perry/src/commands/compile/resolve.rs @@ -338,6 +338,33 @@ pub(super) fn find_node_modules(start: &Path) -> Option { } } +/// Every `node_modules` directory on the ancestor chain of `start`, nearest +/// first. Node's resolution algorithm consults each one until the package is +/// found — not only the nearest. pnpm layouts hit the difference constantly: +/// a package's own `node_modules` usually holds only `.bin`, while its real +/// dependencies live in the `.pnpm/@/node_modules` sibling one +/// level further up. Stopping at the first directory made every transitive +/// dependency of a compiled pnpm package unresolvable, so their call sites +/// lowered to raw extern symbols and the link failed (undefined `_toNumber` / +/// `_isUnsafe` across the fast-xml-parser graph). +pub(super) fn ancestor_node_modules_dirs(start: &Path) -> Vec { + let mut dirs = Vec::new(); + let mut current = start.to_path_buf(); + loop { + // A dir literally named `node_modules` hosts packages itself; its + // `node_modules/node_modules` join below would never exist. + if current.file_name().is_some_and(|n| n != "node_modules") { + let node_modules = current.join("node_modules"); + if node_modules.is_dir() { + dirs.push(node_modules); + } + } + if !current.pop() { + return dirs; + } + } +} + /// Look up a bare package name in the nearest package.json's `dependencies` / /// `devDependencies` sections and, if the entry has a `file:` prefix, return the /// resolved directory path (NOT canonicalized — caller does that). @@ -1316,63 +1343,62 @@ pub(super) fn resolve_import( }; for start in search_paths.iter().flatten() { - if let Some(node_modules) = find_node_modules(start) { + for node_modules in ancestor_node_modules_dirs(start) { let package_dir = node_modules.join(&package_name); - if package_dir.is_dir() { - if let Some(entry) = resolve_package_entry(&package_dir, subpath.as_deref()) { - // Packages with perry.nativeLibrary are compiled natively (Rust FFI) - if has_perry_native_library(&package_dir) { - return Some((entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); - } - // Packages with perry.nativeModule: true contain Perry-compatible - // TypeScript that must be compiled natively (e.g. perry-react). - if has_perry_native_module(&package_dir) { - return Some((entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); - } - // Packages listed in perry.compilePackages are compiled natively - if compile_packages.contains(&package_name) { - // Deduplicate: if we've already resolved this package from a - // different node_modules location, use the first-found directory - // to avoid duplicate symbols from identical package copies - let effective_dir = compile_package_dirs - .get(&package_name) - .unwrap_or(&package_dir); - // Prefer TypeScript source over compiled JS - if let Some(src_entry) = - resolve_package_source_entry(effective_dir, subpath.as_deref()) - { - return Some(( - src_entry.canonicalize().ok()?, - ModuleKind::NativeCompiled, - )); - } - // Fall back to normal resolution but still mark as NativeCompiled - if let Some(fallback_entry) = - resolve_package_entry(effective_dir, subpath.as_deref()) - { - return Some(( - fallback_entry.canonicalize().ok()?, - ModuleKind::NativeCompiled, - )); - } - // If effective_dir failed (shouldn't happen), try the local dir - return Some((entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); - } - // For other node_modules packages, classify by file - // extension. `.ts` / `.tsx` sources are compiled natively. - // `.js` / `.mjs` / `.cjs` and other shapes stay Interpreted; - // since runtime-JS (V8) support was removed, reaching one of - // these is a hard error surfaced by the V8-free gate after - // module collection. - let canonical = entry.canonicalize().ok()?; - let kind = if is_ts_file(&canonical) { - ModuleKind::NativeCompiled - } else { - ModuleKind::Interpreted - }; - return Some((canonical, kind)); + if !package_dir.is_dir() { + continue; + } + let Some(entry) = resolve_package_entry(&package_dir, subpath.as_deref()) else { + continue; + }; + // Packages with perry.nativeLibrary are compiled natively (Rust FFI) + if has_perry_native_library(&package_dir) { + return Some((entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); + } + // Packages with perry.nativeModule: true contain Perry-compatible + // TypeScript that must be compiled natively (e.g. perry-react). + if has_perry_native_module(&package_dir) { + return Some((entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); + } + // Packages listed in perry.compilePackages are compiled natively + if compile_packages.contains(&package_name) { + // Deduplicate: if we've already resolved this package from a + // different node_modules location, use the first-found directory + // to avoid duplicate symbols from identical package copies + let effective_dir = compile_package_dirs + .get(&package_name) + .unwrap_or(&package_dir); + // Prefer TypeScript source over compiled JS + if let Some(src_entry) = + resolve_package_source_entry(effective_dir, subpath.as_deref()) + { + return Some((src_entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); } + // Fall back to normal resolution but still mark as NativeCompiled + if let Some(fallback_entry) = + resolve_package_entry(effective_dir, subpath.as_deref()) + { + return Some(( + fallback_entry.canonicalize().ok()?, + ModuleKind::NativeCompiled, + )); + } + // If effective_dir failed (shouldn't happen), try the local dir + return Some((entry.canonicalize().ok()?, ModuleKind::NativeCompiled)); } + // For other node_modules packages, classify by file + // extension. `.ts` / `.tsx` sources are compiled natively. + // `.js` / `.mjs` / `.cjs` and other shapes stay Interpreted; + // since runtime-JS (V8) support was removed, reaching one of + // these is a hard error surfaced by the V8-free gate after + // module collection. + let canonical = entry.canonicalize().ok()?; + let kind = if is_ts_file(&canonical) { + ModuleKind::NativeCompiled + } else { + ModuleKind::Interpreted + }; + return Some((canonical, kind)); } } diff --git a/crates/perry/src/commands/compile/resolve/tests.rs b/crates/perry/src/commands/compile/resolve/tests.rs index 26f5254f7fe..d1734216092 100644 --- a/crates/perry/src/commands/compile/resolve/tests.rs +++ b/crates/perry/src/commands/compile/resolve/tests.rs @@ -1937,3 +1937,42 @@ mod exports_over_module_precedence_tests { assert_eq!(resolved, pkg.join("dist/index.js")); } } + +/// pnpm layout regression — a package's own `node_modules` usually holds +/// only `.bin`, while its real dependencies live in the +/// `.pnpm/@/node_modules` sibling one level further up. The +/// resolver must consult EVERY ancestor `node_modules` (Node's algorithm), +/// not stop at the first one found. +mod ancestor_node_modules_tests { + use super::super::ancestor_node_modules_dirs; + + #[test] + fn walk_lists_every_ancestor_node_modules_nearest_first() { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + + // /node_modules/.pnpm/fxp@1/node_modules/fxp/node_modules/.bin + // └── strnum lives in the + // .pnpm sibling dir + let pnpm_nm = root.join("node_modules/.pnpm/fxp@1/node_modules"); + let pkg_local_nm = pnpm_nm.join("fxp/node_modules"); + std::fs::create_dir_all(pkg_local_nm.join(".bin")).expect("mkdir .bin"); + std::fs::create_dir_all(pnpm_nm.join("strnum")).expect("mkdir strnum"); + let start = pnpm_nm.join("fxp/src/xmlparser"); + std::fs::create_dir_all(&start).expect("mkdir src"); + + // Ancestors of the temp dir itself (e.g. /tmp) are outside the + // fixture; keep only the chain under `root` so the exact-order + // assertion cannot flake on a host machine's own node_modules. + let dirs: Vec<_> = ancestor_node_modules_dirs(&start) + .into_iter() + .filter(|d| d.starts_with(root)) + .collect(); + assert_eq!( + dirs, + vec![pkg_local_nm, pnpm_nm, root.join("node_modules")], + "every ancestor node_modules, nearest first: the package-local `.bin` \ + host, then the .pnpm sibling hosting the real deps, then the project root" + ); + } +}