Skip to content
Open
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
33 changes: 21 additions & 12 deletions src/install/PackageManager/PackageJSONEditor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,8 @@ pub(crate) fn edit_update_no_args_in(
let version_literal = value
.as_utf8_string_literal()
.unwrap_or_else(|| bun_core::out_of_memory());
let mut tag = dependency::Tag::infer(version_literal);
let mut tag =
dependency::Tag::infer(dependency::trim_literal(version_literal));

// npm versions only (and dist-tags with --latest); `catalog:` is handled by edit_catalogs_*.
if tag != dependency::Tag::Npm
Expand Down Expand Up @@ -403,7 +404,9 @@ pub(crate) fn edit_update_no_args_in(
let value_literal = value
.as_utf8_string_literal()
.unwrap_or_else(|| bun_core::out_of_memory());
if dependency::Tag::infer(value_literal) == dependency::Tag::Catalog {
if dependency::Tag::infer(dependency::trim_literal(value_literal))
== dependency::Tag::Catalog
{
continue;
}

Expand Down Expand Up @@ -499,8 +502,9 @@ pub(crate) fn edit_update_no_args_in(
};

if is_alias {
let dep_literal =
workspace_dep.version.literal.slice(string_buf);
let dep_literal = dependency::trim_literal(
workspace_dep.version.literal.slice(string_buf),
);

// negative because the real package might have a scope
// e.g. "dep": "npm:@foo/bar@1.2.3"
Expand Down Expand Up @@ -621,7 +625,7 @@ pub(crate) fn edit_catalogs_before_update(
let version_literal = value
.as_utf8_string_literal()
.unwrap_or_else(|| bun_core::out_of_memory());
let mut tag = dependency::Tag::infer(version_literal);
let mut tag = dependency::Tag::infer(dependency::trim_literal(version_literal));

let mut alias_at_index: Option<usize> = None;
if strings::trim(version_literal, &strings::WHITESPACE_CHARS).starts_with(b"npm:") {
Expand Down Expand Up @@ -781,7 +785,7 @@ pub(crate) fn edit_catalogs_after_update(
};

new_literals[index] = Some(if info.is_alias {
let dep_literal = &info.original_version_literal;
let dep_literal = dependency::trim_literal(&info.original_version_literal);
if let Some(at_index) = strings::last_index_of_char(dep_literal, b'@') {
let mut v = Vec::new();
write!(
Expand Down Expand Up @@ -917,8 +921,9 @@ pub(crate) fn edit(
== Subcommand::Update
&& value.expr.as_utf8_string_literal().is_some_and(
|version_literal| {
dependency::Tag::infer(version_literal)
== dependency::Tag::Catalog
dependency::Tag::infer(dependency::trim_literal(
version_literal,
)) == dependency::Tag::Catalog
},
);

Expand All @@ -937,8 +942,9 @@ pub(crate) fn edit(
else {
break 'add_packages_to_update;
};
let mut tag =
dependency::Tag::infer(version_literal);
let mut tag = dependency::Tag::infer(
dependency::trim_literal(version_literal),
);

if tag != dependency::Tag::Npm
&& tag != dependency::Tag::DistTag
Expand Down Expand Up @@ -1423,7 +1429,8 @@ pub(crate) fn edit(
let e_string = unsafe { &mut *e_string };
// `bun update <pkg>` keeps a `catalog:` reference; `bun add` still replaces it.
if manager.subcommand == Subcommand::Update
&& dependency::Tag::infer(e_string.data.slice()) == dependency::Tag::Catalog
&& dependency::Tag::infer(dependency::trim_literal(e_string.data.slice()))
== dependency::Tag::Catalog
{
continue;
}
Expand Down Expand Up @@ -1518,7 +1525,9 @@ pub(crate) fn edit(
};

if entry.value.is_alias {
let dep_literal = &entry.value.original_version_literal;
let dep_literal = dependency::trim_literal(
&entry.value.original_version_literal,
);

if let Some(at_index) =
strings::last_index_of_char(dep_literal, b'@')
Expand Down
14 changes: 10 additions & 4 deletions src/install/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ impl DependencyExt for Dependency {
Some(Semver::string::Builder::string_hash(
new_name.slice(out_slice),
)),
new_literal.slice(out_slice),
trim_literal(new_literal.slice(out_slice)),
self.version.tag,
&sliced,
None,
Expand Down Expand Up @@ -693,7 +693,7 @@ impl VersionExt for Version {
parse_with_tag(
alias,
Some(alias_hash),
sliced.slice,
trim_literal(sliced.slice),
tag,
&sliced,
Some(ctx.log),
Expand Down Expand Up @@ -1164,6 +1164,12 @@ pub(crate) fn is_windows_abs_path_with_leading_slashes(dep: &[u8]) -> Option<&[u
None
}

/// Literals may carry leading whitespace; classify and parse these bytes, not the raw literal.
#[inline]
pub fn trim_literal(literal: &[u8]) -> &[u8] {
strings::trim_left(literal, b" \t\n\r")
}

#[inline]
pub fn parse<'a, 'b>(
alias: String,
Expand All @@ -1173,7 +1179,7 @@ pub fn parse<'a, 'b>(
log: impl Into<Option<&'a mut bun_ast::Log>>,
manager: impl Into<Option<&'b mut PackageManager>>,
) -> Option<Version> {
let dep = strings::trim_left(dependency, b" \t\n\r");
let dep = trim_literal(dependency);
parse_with_tag(
alias,
alias_hash.into(),
Expand All @@ -1194,7 +1200,7 @@ pub(crate) fn parse_with_optional_tag<'a, 'b>(
log: impl Into<Option<&'a mut bun_ast::Log>>,
package_manager: impl Into<Option<&'b mut PackageManager>>,
) -> Option<Version> {
let dep = strings::trim_left(dependency, b" \t\n\r");
let dep = trim_literal(dependency);
parse_with_tag(
alias,
alias_hash.into(),
Expand Down
11 changes: 7 additions & 4 deletions src/install/lockfile/Package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1631,7 +1631,9 @@ impl Package<u64> {
) -> crate::Result<Option<Dependency>> {
#[cfg(windows)]
let external_version = 'brk: {
match tag.unwrap_or_else(|| dependency::version::Tag::infer(version)) {
match tag.unwrap_or_else(|| {
dependency::version::Tag::infer(dependency::trim_literal(version))
}) {
dependency::version::Tag::Workspace
| dependency::version::Tag::Folder
| dependency::version::Tag::Symlink
Expand Down Expand Up @@ -1683,9 +1685,10 @@ impl Package<u64> {
semver::string::Builder::string_hash(npm_name.slice(buf))
}
dependency::version::Tag::Workspace => {
if strings::has_prefix(sliced.slice, b"workspace:") {
let literal = dependency::trim_literal(sliced.slice);
if strings::has_prefix(literal, b"workspace:") {
'brk: {
let input = &sliced.slice[b"workspace:".len()..];
let input = &literal[b"workspace:".len()..];
let trimmed = strings::trim(input, &strings::WHITESPACE_CHARS);
if trimmed.len() != 1
|| (trimmed[0] != b'*' && trimmed[0] != b'^' && trimmed[0] != b'~')
Expand Down Expand Up @@ -2298,7 +2301,7 @@ impl Package<u64> {
string_builder.count(value);

// If it's a folder or workspace, pessimistically assume we will need a maximum path
match dependency::version::Tag::infer(value) {
match dependency::version::Tag::infer(dependency::trim_literal(value)) {
dependency::version::Tag::Folder
| dependency::version::Tag::Workspace => {
string_builder.cap += MAX_PATH_BYTES;
Expand Down
1 change: 1 addition & 0 deletions src/runtime/cli/pack_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3315,6 +3315,7 @@ fn edit_root_package_json(
else {
continue;
};
let package_spec = bun_install::dependency::trim_literal(package_spec);
if let Some(without_workspace_protocol) =
strings::without_prefix_if_possible_comptime(package_spec, b"workspace:")
{
Expand Down
1 change: 1 addition & 0 deletions src/runtime/cli/update_interactive_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2395,6 +2395,7 @@ fn preserve_version_prefix(
original_version: &[u8],
new_version: &[u8],
) -> crate::Result<Box<[u8]>> {
let original_version = dependency::trim_literal(original_version);
if original_version.len() > 1 {
let mut orig_version: &[u8] = original_version;
let mut alias: Option<&[u8]> = None;
Expand Down
121 changes: 121 additions & 0 deletions test/cli/install/bun-install-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5405,6 +5405,127 @@ describe("update", () => {
});
});
});
describe("leading whitespace in the version literal", () => {
// The installer ignores leading whitespace in a package.json version, so each of these must
// end up exactly where the same command takes the value without the whitespace.
const cases: [dependency: string, version: string, args: string[], updated: string, installed: string][] = [
["aliased-dep", " npm:no-deps@^1.0.0", ["aliased-dep"], "npm:no-deps@^1.1.0", "1.1.0"],
["aliased-dep", " npm:no-deps@^1.0.0", [], "npm:no-deps@^1.1.0", "1.1.0"],
["aliased-dep", " npm:no-deps@^1.0.0", ["--latest"], "npm:no-deps@^2.0.0", "2.0.0"],
["aliased-dep", "\tnpm:no-deps@~1.0.0", [], "npm:no-deps@~1.0.1", "1.0.1"],
["aliased-dep", "\tnpm:no-deps@~1.0.0", ["--latest"], "npm:no-deps@~2.0.0", "2.0.0"],
["no-deps", " ^1.0.0", [], "^1.1.0", "1.1.0"],
["no-deps", " ^1.0.0", ["no-deps"], "^1.1.0", "1.1.0"],
["no-deps", " ^1.0.0", ["--latest"], "^2.0.0", "2.0.0"],
["no-deps", "\n~1.0.0", [], "~1.0.1", "1.0.1"],
];

for (const [dependency, version, args, updated, installed] of cases) {
test(`${JSON.stringify(version)} with \`bun update${args.map(arg => ` ${arg}`).join("")}\``, async () => {
await write(
packageJson,
JSON.stringify({
name: "foo",
dependencies: {
[dependency]: version,
},
}),
);

await runBunUpdate(env, packageDir, args);
assertManifestsPopulated(join(packageDir, ".bun-cache"), registryUrl());

expect(await file(packageJson).json()).toEqual({
name: "foo",
dependencies: {
[dependency]: updated,
},
});
expect(await file(join(packageDir, "node_modules", dependency, "package.json")).json()).toMatchObject({
name: "no-deps",
version: installed,
});
});
}

test("bun update --interactive keeps the alias and the range prefix", async () => {
await write(
packageJson,
JSON.stringify({
name: "foo",
dependencies: {
"aliased-dep": " npm:no-deps@^1.0.0",
"no-deps": "\t~1.0.0",
},
}),
);
await runBunInstall(env, packageDir);

// `a` selects every package, enter confirms. bunEnv lets the prompt read keys from a pipe.
await using proc = spawn({
cmd: [bunExe(), "update", "--interactive", "--latest"],
cwd: packageDir,
env,
stdin: new Blob(["a\r"]),
stdout: "pipe",
stderr: "pipe",
});
const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ out, err, exitCode }).toMatchObject({ exitCode: 0 });

expect(await file(packageJson).json()).toEqual({
name: "foo",
dependencies: {
"aliased-dep": "npm:no-deps@^2.0.0",
"no-deps": "~2.0.0",
},
});
expect(await file(join(packageDir, "node_modules", "aliased-dep", "package.json")).json()).toMatchObject({
name: "no-deps",
version: "2.0.0",
});
});

test("a catalog reference in an earlier dependency group is left alone", async () => {
// devDependencies are visited before dependencies. The entry recorded for `dependencies`
// must not be spent rewriting the `catalog:` reference.
await write(
packageJson,
JSON.stringify({
name: "foo",
workspaces: {
catalog: {
"no-deps": "^1.0.0",
},
},
devDependencies: {
"no-deps": " catalog:",
},
dependencies: {
"no-deps": " ^1.0.0",
},
}),
);

await runBunUpdate(env, packageDir);
assertManifestsPopulated(join(packageDir, ".bun-cache"), registryUrl());

expect(await file(packageJson).json()).toEqual({
name: "foo",
workspaces: {
catalog: {
"no-deps": "^1.1.0",
},
},
devDependencies: {
"no-deps": " catalog:",
},
dependencies: {
"no-deps": "^1.1.0",
},
});
});
});
test("--no-save will update packages in node_modules and not save to package.json", async () => {
await write(
packageJson,
Expand Down
51 changes: 51 additions & 0 deletions test/cli/install/bun-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9883,6 +9883,57 @@ it("installs the transitive file: dependency of a file: dependency", async () =>
}
});

for (const lockfile of ["bun.lock", "bun.lockb"]) {
it(`installs a file: dependency whose version literal has leading whitespace (${lockfile})`, async () => {
// The folder path is longer than an inline string so that it has to be appended to the
// lockfile's string buffer, which only has room for it if the literal was classified as a folder.
const literal = " file:./vendor/some-long-directory-name/lib";
using dir = tempDir("whitespace-file-dep", {
"package.json": JSON.stringify({
name: "my-app",
version: "1.0.0",
dependencies: {
lib: literal,
},
}),
"vendor/some-long-directory-name/lib/package.json": JSON.stringify({
name: "lib",
version: "1.0.0",
}),
"bunfig.toml": `install.saveTextLockfile = ${lockfile === "bun.lock"}`,
});

// The first pass resolves from package.json; the second installs from the lockfile the first
// pass wrote, which stores the literal as written and has to parse it as a folder again.
for (const args of [["install"], ["install", "--frozen-lockfile"]]) {
await rm(join(String(dir), "node_modules"), { recursive: true, force: true });

const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), ...args],
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
env,
});
const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]);

expect(err).not.toContain("error:");
expect(out).toContain("1 package installed");
expect(exitCode).toBe(0);

expect(await file(join(String(dir), "node_modules", "lib", "package.json")).json()).toEqual({
name: "lib",
version: "1.0.0",
});
if (lockfile === "bun.lock") {
expect(await file(join(String(dir), "bun.lock")).text()).toContain(`"lib": ${JSON.stringify(literal)}`);
} else {
expect(await exists(join(String(dir), "bun.lockb"))).toBe(true);
}
}
});
}

it("fails when a transitive file: dependency's folder does not exist", async () => {
using dir = tempDir("transitive-file-dep-missing", {
"package.json": JSON.stringify({
Expand Down
2 changes: 2 additions & 0 deletions test/cli/install/bun-pack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,8 @@ describe("workspaces", () => {
{ input: "workspace:1.1.x", expected: "1.1.x" },
{ input: "workspace:*", expected: "1.1.1" },
{ input: "workspace:-", expected: "-" },
// leading whitespace is not part of the specifier
{ input: " workspace:^", expected: "^1.1.1" },
];

for (const { input, expected } of withLockfileWorkspaceProtocolTests) {
Expand Down
Loading