Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/codegen/generate-js2native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ const rustIdentifierPaths: Record<string, string> = {
"parse_args.rs": "runtime/node/util/parse_args.rs",
"patch.rs": "patch/patch.rs",
"postgres.rs": "sql_jsc/postgres.rs",
"resolver_jsc.rs": "jsc/resolver_jsc.rs",
"runtime/dns_jsc/dns.rs": "runtime/dns_jsc/dns.rs",
"runtime/node/types.rs": "runtime/node/types.rs",
"runtime/socket/socket.rs": "runtime/socket/socket.rs",
Expand Down
23 changes: 23 additions & 0 deletions src/js/internal-for-testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,3 +422,26 @@ export const fetchH3Internals = {
export const fileSinkInternals = {
liveCount: $newRustFunction("runtime/webcore/FileSink.rs", "TestingAPIs.fileSinkLiveCount", 0) as () => number,
};

export const packageJsonInternals = {
/**
* Drives `SideEffects::has_side_effects` on a synthetic `(package_dir,
* patterns, runtime_path)` triple so tests can exercise the Windows-path
* matching fix for #30320 on any platform.
*
* Pass a Windows-style `dir` (e.g. `C:\\pkg\\`) and `path` to simulate
* Windows behavior — on Linux `path.text` is all `/`, so the bug can
* only be reproduced by feeding synthesised strings directly.
*
* `usePreFix: true` routes through the pre-#30320 `r_fs.join` code path
* so tests can assert the bug actually regressed before the fix.
*
* Returns `true` iff `path` matches any pattern.
*/
sideEffectsHasSideEffects: $newRustFunction("resolver_jsc.rs", "sideEffectsTesting.sideEffectsHasSideEffects", 3) as (
dir: string,
patterns: string[],
path: string,
usePreFix?: boolean,
) => boolean,
};
123 changes: 123 additions & 0 deletions src/jsc/resolver_jsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,126 @@
})
}
}

/// Testing bridges for `bun.resolver.package_json.SideEffects`. Exposed
/// via `bun:internal-for-testing` so the #30320 regression test can drive
/// the glob/exact matchers with synthetic Windows-style paths on any
/// host. Not used by production code.
pub mod side_effects_testing {
use super::*;
use bun_resolver::package_json::{
FileSystemPackageJsonExt, MixedPatterns, SideEffects, StringHashMapUnownedKey,
};

/// Mirrors `PackageJSON::normalize_path_for_glob` — backslashes → slashes.
/// Inlined here so the testing helper stays compilable even when the
/// resolver's private helper changes signature (it's intentionally private
/// in production to keep the matcher's surface small).
fn normalize(path: &[u8]) -> Vec<u8> {
let mut v = path.to_vec();
bun_paths::slashes_to_posix_in_place(&mut v[..]);
v
}

/// `sideEffectsHasSideEffects(dir, patterns, path, usePreFix) -> bool`
///
/// - `dir` — absolute directory the package.json "lives in",
/// with trailing separator. Pass `C:\pkg\` to simulate
/// a Windows package root on a Linux host.
/// - `patterns` — `sideEffects` array, e.g. `["adapters/**/*.js"]`.
/// - `path` — runtime path the resolver would hand to
/// `has_side_effects`, e.g. `C:\pkg\adapters\foo.js`.
/// - `usePreFix` — when truthy, build patterns through the old
/// `r_fs.join` path so tests can assert the bug
/// actually regressed. Default false (fixed path).
///
/// Returns `true` iff `path` matches any pattern. Before the fix,
/// Windows-shaped inputs always returned `false` because the stored
/// pattern carried a leading `/` that the runtime path never did.
pub fn side_effects_has_side_effects(
global: &JSGlobalObject,
frame: &CallFrame,
) -> JsResult<JSValue> {
let [dir_val, patterns_val, path_val, use_pre_fix_val] = frame.arguments_as_array::<4>();
if dir_val.is_undefined() || patterns_val.is_undefined() || path_val.is_undefined() {
return Err(global.throw(format_args!(
"sideEffectsHasSideEffects(dir, patterns, path, usePreFix?) takes 3 or 4 arguments"
)));
}

// `to_bun_string` returns a +1 ref; `bun_core::String` is `Copy` (no
// `Drop`), so wrap in `OwnedString` for the scope-exit `deref()`,
// including on `?`-early-return paths.
let dir_bunstr = OwnedString::new(dir_val.to_bun_string(global)?);
let dir_utf8 = dir_bunstr.to_utf8();
let path_bunstr = OwnedString::new(path_val.to_bun_string(global)?);
let path_utf8 = path_bunstr.to_utf8();
let use_pre_fix = use_pre_fix_val.to_boolean();

if !patterns_val.is_array() {
return Err(global.throw_type_error(format_args!(
"sideEffectsHasSideEffects: patterns must be an array"
)));
}

// SAFETY: `bun_vm()` is non-null on a Bun-owned global; `transpiler.resolver`
// is initialized at VM init. We only use the FileSystem field through the
// raw pointer; no aliasing with other host fns (JS single-threaded).
let vm = global.bun_vm().as_mut();
let r_fs: &mut bun_resolver::fs::FileSystem = unsafe { &mut *vm.transpiler.resolver.fs };

Check failure on line 207 in src/jsc/resolver_jsc.rs

View workflow job for this annotation

GitHub Actions / cargo clippy

unsafe block missing a safety comment

let dir = dir_utf8.slice();
let len = patterns_val.get_length(global)? as u32;

// Build a `SideEffects` value from the patterns just like `parse` would.
let mut map = bun_resolver::package_json::SideEffectsMap::with_capacity(len as usize);
let mut glob_list = bun_resolver::package_json::GlobList::with_capacity(len as usize);
let mut has_globs = false;
let mut has_exact = false;

for i in 0..len {
let item = patterns_val.get_index(global, i)?;
let item_bunstr = OwnedString::new(item.to_bun_string(global)?);
let item_utf8 = item_bunstr.to_utf8();
let name = item_utf8.slice();

// Build the pattern through the same helper production code uses,
// OR through the pre-fix `r_fs.join` so the test can observe both.
// The fixed path strips package-root-relative leading separators
// before `r_fs.abs` (matches `PackageJSON::strip_pattern_root`).
let pattern_vec: Vec<u8> = if use_pre_fix {
FileSystemPackageJsonExt::join(r_fs, &[dir, name]).to_vec()
} else {
let mut stripped = name;
while let [b'/' | b'\\', rest @ ..] = stripped {
stripped = rest;
}
r_fs.abs(&[dir, stripped]).to_vec()
};
let normalized_pattern = normalize(&pattern_vec);

let is_glob = name.iter().any(|&b| matches!(b, b'*' | b'?' | b'[' | b'{'));
if is_glob {
glob_list.push(normalized_pattern.into_boxed_slice());
has_globs = true;
} else {
let _ = map.insert(StringHashMapUnownedKey::init(&normalized_pattern), ());
has_exact = true;
}
}

let side_effects = if has_globs && has_exact {
SideEffects::Mixed(MixedPatterns {
exact: map,
globs: glob_list,
})
} else if has_globs {
SideEffects::Glob(glob_list)
} else {
SideEffects::Map(map)
};

let matched = side_effects.has_side_effects(path_utf8.slice());
Ok(JSValue::js_boolean(matched))
}
}
76 changes: 51 additions & 25 deletions src/resolver/package_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,18 @@ impl PackageJSON {
bun_paths::slashes_to_posix_in_place(&mut normalized[..]);
Ok(normalized)
}

/// A leading `/` or `\` in a `sideEffects` entry is package-root-relative,
/// not filesystem-absolute. Strip any leading separators so `r_fs.abs`
/// joins against the package dir instead of discarding it (`path.resolve`
/// treats an absolute later component as a new root, and `//x` is still
/// absolute after one strip). Matches esbuild's `filepath.Join`. #30320
fn strip_pattern_root(mut name: &[u8]) -> &[u8] {
while let [b'/' | b'\\', rest @ ..] = name {
name = rest;
}
name
}
}

#[derive(Default)]
Expand Down Expand Up @@ -254,7 +266,15 @@ impl SideEffects {
match self {
SideEffects::Unspecified => true,
SideEffects::False => false,
SideEffects::Map(map) => map.contains_key(&StringHashMapUnownedKey::init(path)),
// Stored patterns are always slash-normalized (see `parse`); the
// runtime path may contain native separators on Windows, so we
// must normalize it the same way before hashing. See #30320.
SideEffects::Map(map) => {
let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else {
return true;
};
map.contains_key(&StringHashMapUnownedKey::init(&normalized_path))
}
SideEffects::Glob(glob_list) => {
// Normalize path for cross-platform glob matching
let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else {
Expand All @@ -269,18 +289,18 @@ impl SideEffects {
false
}
SideEffects::Mixed(mixed) => {
let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else {
return true;
};

// First check exact matches
if mixed
.exact
.contains_key(&StringHashMapUnownedKey::init(path))
.contains_key(&StringHashMapUnownedKey::init(&normalized_path))
{
return true;
}
// Then check glob patterns with normalized path
let Ok(normalized_path) = PackageJSON::normalize_path_for_glob(path) else {
return true;
};

for pattern in mixed.globs.iter() {
if glob::r#match(pattern, &normalized_path).matches() {
return true;
Expand Down Expand Up @@ -749,30 +769,36 @@ impl PackageJSON {
map.reserve(items.len());
glob_list.reserve(items.len());

let dir = json_source.path.name().dir_with_trailing_slash();
for item in items {
if let Some(name) = item.as_str() {
// Skip CSS files as they're not relevant for tree-shaking
if bun_paths::extension(name) == b".css" {
continue;
}

// Store the pattern relative to the package directory
let joined: [&[u8]; 2] =
[json_source.path.name().dir_with_trailing_slash(), name];

let pattern = r_fs.join(&joined);
// Build the absolute pattern using the same shape as runtime paths
// (`r_fs.abs` / `join_abs_string`) so they compare equal after
// normalization. `r_fs.join` here would produce a different shape
// on Windows (leading `/` before the drive letter) that wouldn't
// match the runtime path. See #30320.
let joined: [&[u8]; 2] = [dir, Self::strip_pattern_root(name)];
let pattern = r_fs.abs(&joined);
let normalized_pattern = Self::normalize_path_for_glob(pattern)
.unwrap_or_else(|_| pattern.to_vec());

if strings::contains_char(name, b'*')
|| strings::contains_char(name, b'?')
|| strings::contains_char(name, b'[')
|| strings::contains_char(name, b'{')
{
// Normalize pattern to use forward slashes for cross-platform compatibility
let normalized_pattern = Self::normalize_path_for_glob(pattern)
.unwrap_or_else(|_| pattern.to_vec());
glob_list.push(normalized_pattern.into_boxed_slice());
} else {
let _ = map.insert(StringHashMapUnownedKey::init(pattern), ());
// Exact-match keys must be normalized to match runtime paths
// (callers feed `hasSideEffects` a path that is also run through
// `normalize_path_for_glob` on lookup).
let _ = map
.insert(StringHashMapUnownedKey::init(&normalized_pattern), ());
}
}
}
Expand All @@ -783,19 +809,17 @@ impl PackageJSON {
} else if has_globs {
// Only glob patterns
glob_list.reserve(items.len());
let dir = json_source.path.name().dir_with_trailing_slash();
for item in items {
if let Some(name) = item.as_str() {
// Skip CSS files as they're not relevant for tree-shaking
if bun_paths::extension(name) == b".css" {
continue;
}

// Store the pattern relative to the package directory
let joined: [&[u8]; 2] =
[json_source.path.name().dir_with_trailing_slash(), name];

let pattern = r_fs.join(&joined);
// Normalize pattern to use forward slashes for cross-platform compatibility
// See comment above about `r_fs.abs` vs `r_fs.join`.
let joined: [&[u8]; 2] = [dir, Self::strip_pattern_root(name)];
let pattern = r_fs.abs(&joined);
let normalized_pattern = Self::normalize_path_for_glob(pattern)
.unwrap_or_else(|_| pattern.to_vec());
glob_list.push(normalized_pattern.into_boxed_slice());
Expand All @@ -805,13 +829,15 @@ impl PackageJSON {
} else {
// Only exact matches
map.reserve(items.len());
let dir = json_source.path.name().dir_with_trailing_slash();
for item in items {
if let Some(name) = item.as_str() {
let joined: [&[u8]; 2] =
[json_source.path.name().dir_with_trailing_slash(), name];

let joined: [&[u8]; 2] = [dir, Self::strip_pattern_root(name)];
let pattern = r_fs.abs(&joined);
let normalized_pattern = Self::normalize_path_for_glob(pattern)
.unwrap_or_else(|_| pattern.to_vec());
let _ =
map.insert(StringHashMapUnownedKey::init(r_fs.join(&joined)), ());
map.insert(StringHashMapUnownedKey::init(&normalized_pattern), ());
}
}
package_json.side_effects = SideEffects::Map(map);
Expand Down
44 changes: 10 additions & 34 deletions src/resolver/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1636,23 +1636,13 @@ impl<'a> Resolver<'a> {
result.primary_side_effects_data = match &existing.side_effects {
PJSideEffects::Unspecified => SideEffects::HasSideEffects,
PJSideEffects::False => SideEffects::NoSideEffectsPackageJson,
PJSideEffects::Map(map) => {
if map.contains_key(&crate::package_json::StringHashMapUnownedKey::init(
path.text(),
)) {
SideEffects::HasSideEffects
} else {
SideEffects::NoSideEffectsPackageJson
}
}
PJSideEffects::Glob(_) => {
if existing.side_effects.has_side_effects(path.text()) {
SideEffects::HasSideEffects
} else {
SideEffects::NoSideEffectsPackageJson
}
}
PJSideEffects::Mixed(_) => {
// Route all three Map/Glob/Mixed branches through
// `has_side_effects`. Stored pattern keys are slash-
// normalized at parse time, so the runtime lookup must
// normalize `path.text` the same way — bypassing that
// call was the bug that dropped every matched file on
// Windows. See #30320.
PJSideEffects::Map(_) | PJSideEffects::Glob(_) | PJSideEffects::Mixed(_) => {
if existing.side_effects.has_side_effects(path.text()) {
SideEffects::HasSideEffects
} else {
Expand All @@ -1676,23 +1666,9 @@ impl<'a> Resolver<'a> {
result.primary_side_effects_data = match &package_json.side_effects {
PJSideEffects::Unspecified => SideEffects::HasSideEffects,
PJSideEffects::False => SideEffects::NoSideEffectsPackageJson,
PJSideEffects::Map(map) => {
if map.contains_key(
&crate::package_json::StringHashMapUnownedKey::init(path.text()),
) {
SideEffects::HasSideEffects
} else {
SideEffects::NoSideEffectsPackageJson
}
}
PJSideEffects::Glob(_) => {
if package_json.side_effects.has_side_effects(path.text()) {
SideEffects::HasSideEffects
} else {
SideEffects::NoSideEffectsPackageJson
}
}
PJSideEffects::Mixed(_) => {
PJSideEffects::Map(_)
| PJSideEffects::Glob(_)
| PJSideEffects::Mixed(_) => {
if package_json.side_effects.has_side_effects(path.text()) {
SideEffects::HasSideEffects
} else {
Expand Down
5 changes: 5 additions & 0 deletions src/runtime/dispatch_js2native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ pub use bun_patch_jsc::testing::patch_apply as patch_patch_testing_ap_is_apply;
pub use bun_patch_jsc::testing::patch_make_diff as patch_patch_testing_ap_is_make_diff;
pub use bun_patch_jsc::testing::patch_parse as patch_patch_testing_ap_is_parse;

// Drives `SideEffects::has_side_effects` with synthetic Windows-style paths
// from `bun:internal-for-testing` so #30320's regression test can reproduce
// the Windows-path mismatch on any host. See `jsc/resolver_jsc.rs`.
pub use bun_jsc::resolver_jsc::side_effects_testing::side_effects_has_side_effects as jsc_resolver_jsc_side_effects_testing_side_effects_has_side_effects;

pub use bun_sourcemap_jsc::internal_jsc::testing_find as sourcemap_internal_source_map_testing_ap_is_find;
pub use bun_sourcemap_jsc::internal_jsc::testing_from_vlq as sourcemap_internal_source_map_testing_ap_is_from_vlq;
pub use bun_sourcemap_jsc::internal_jsc::testing_to_vlq as sourcemap_internal_source_map_testing_ap_is_to_vlq;
Expand Down
Loading
Loading