Skip to content
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.

Sign up for GitHub

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

bun pm pack + bundleDependencies with scoped packages #16407

Merged
merged 6 commits into from
Jan 16, 2025
Merged

bun pm pack + bundleDependencies with scoped packages #16407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
379f0b6
pack scoped packages
RiskyMH Jan 15, 2025
b33c4f7
Merge branch 'main' into riskymh/scoped-pack-bundled
RiskyMH Jan 15, 2025
385aed5
Update src/cli/pack_command.zig
dylan-conway Jan 15, 2025
a50a8f9
`bun run zig-format`
dylan-conway Jan 15, 2025
d7171e0
close
dylan-conway Jan 15, 2025
4a9cbe2
!todo
dylan-conway Jan 15, 2025
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
90 changes: 67 additions & 23 deletions src/cli/pack_command.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ pub const PackCommand = struct {
var bundled_pack_queue = PackQueue.init(ctx.allocator, {});
if (ctx.bundled_deps.items.len == 0) return bundled_pack_queue;

const dir = root_dir.openDirZ("node_modules", .{ .iterate = true }) catch |err| {
var dir = root_dir.openDirZ("node_modules", .{ .iterate = true }) catch |err| {
switch (err) {
// ignore node_modules if it isn't a directory, or doesn't exist
error.NotDir, error.FileNotFound => return bundled_pack_queue,
Expand All @@ -519,6 +519,7 @@ pub const PackCommand = struct {
},
}
};
defer dir.close();

// A set of bundled dependency locations
// - node_modules/is-even
Expand All @@ -535,34 +536,77 @@ pub const PackCommand = struct {
while (iter.next().unwrap() catch null) |entry| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change this function to instead loop through the bundled_deps list, try to open each, and send the dir through addBundledDep if it exists, like you suggested. This way we can avoid opening both directories of a scoped package

if (entry.kind != .directory) continue;

const entry_name = entry.name.slice();
const _entry_name = entry.name.slice();

for (ctx.bundled_deps.items) |*dep| {
bun.assertWithLocation(dep.from_root_package_json, @src());
if (!strings.eqlLong(entry_name, dep.name, true)) continue;
if (strings.startsWithChar(_entry_name, '@')) {
const concat = try entrySubpath(ctx.allocator, "node_modules", _entry_name);

const entry_subpath = try entrySubpath(ctx.allocator, "node_modules", entry_name);
var scoped_dir = root_dir.openDirZ(concat, .{ .iterate = true }) catch {
continue;
};
defer scoped_dir.close();

const dedupe_entry = try dedupe.getOrPut(entry_subpath);
if (dedupe_entry.found_existing) {
// already got to it in `addBundledDep` below
dep.was_packed = true;
break;
var scoped_iter = DirIterator.iterate(scoped_dir, .u8);
while (scoped_iter.next().unwrap() catch null) |sub_entry| {
const entry_name = try entrySubpath(ctx.allocator, _entry_name, sub_entry.name.slice());

for (ctx.bundled_deps.items) |*dep| {
bun.assertWithLocation(dep.from_root_package_json, @src());
if (!strings.eqlLong(entry_name, dep.name, true)) continue;

const entry_subpath = try entrySubpath(ctx.allocator, "node_modules", entry_name);

const dedupe_entry = try dedupe.getOrPut(entry_subpath);
if (dedupe_entry.found_existing) {
// already got to it in `addBundledDep` below
dep.was_packed = true;
break;
}

const subdir = openSubdir(dir, entry_name, entry_subpath);
dep.was_packed = true;
try addBundledDep(
ctx,
root_dir,
.{ subdir, entry_subpath, 2 },
&bundled_pack_queue,
&dedupe,
&additional_bundled_deps,
log_level,
);

break;
}
}
} else {
const entry_name = _entry_name;
for (ctx.bundled_deps.items) |*dep| {
bun.assertWithLocation(dep.from_root_package_json, @src());
if (!strings.eqlLong(entry_name, dep.name, true)) continue;

const subdir = openSubdir(dir, entry_name, entry_subpath);
dep.was_packed = true;
try addBundledDep(
ctx,
root_dir,
.{ subdir, entry_subpath, 2 },
&bundled_pack_queue,
&dedupe,
&additional_bundled_deps,
log_level,
);
const entry_subpath = try entrySubpath(ctx.allocator, "node_modules", entry_name);

break;
const dedupe_entry = try dedupe.getOrPut(entry_subpath);
if (dedupe_entry.found_existing) {
// already got to it in `addBundledDep` below
dep.was_packed = true;
break;
}

const subdir = openSubdir(dir, entry_name, entry_subpath);
dep.was_packed = true;
try addBundledDep(
ctx,
root_dir,
.{ subdir, entry_subpath, 2 },
&bundled_pack_queue,
&dedupe,
&additional_bundled_deps,
log_level,
);

break;
}
}
}

Expand Down
33 changes: 32 additions & 1 deletion test/cli/install/bun-pack.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,37 @@ describe("bundledDependnecies", () => {
]);
});

test(`scoped bundledDependencies`, async () => {
await Promise.all([
write(
join(packageDir, "package.json"),
JSON.stringify({
name: "pack-bundled",
version: "4.4.4",
dependencies: {
"@oven/bun": "1.1.1",
},
bundledDependencies: ["@oven/bun"],
}),
),
write(
join(packageDir, "node_modules", "@oven", "bun", "package.json"),
JSON.stringify({
name: "@oven/bun",
version: "1.1.1",
}),
),
]);

await pack(packageDir, bunEnv);

const tarball = readTarball(join(packageDir, "pack-bundled-4.4.4.tgz"));
expect(tarball.entries).toMatchObject([
{ "pathname": "package/package.json" },
{ "pathname": "package/node_modules/@oven/bun/package.json" },
]);
});

test(`invalid bundledDependencies value should throw`, async () => {
await Promise.all([
write(
Expand Down Expand Up @@ -733,7 +764,7 @@ describe("bundledDependnecies", () => {
]);
});

test.todo("scoped names", async () => {
test("scoped names", async () => {
await Promise.all([
write(
join(packageDir, "package.json"),
Expand Down
Loading