Skip to content

Commit

Permalink
fix(bundler) fix pretty path resolution (#15119)
Browse files Browse the repository at this point in the history
  • Loading branch information
cirospaciari authored Nov 13, 2024
1 parent 873b0a7 commit e945146
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 23 deletions.
48 changes: 27 additions & 21 deletions src/bundler/bundle_v2.zig
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,23 @@ pub const BakeEntryPoint = struct {
return .{ .path = path, .graph = .client, .css = true };
}
};

fn genericPathWithPrettyInitialized(path: Fs.Path, target: options.Target, top_level_dir: string, allocator: std.mem.Allocator) !Fs.Path {
// TODO: outbase
var buf: bun.PathBuffer = undefined;
const rel = bun.path.relativePlatform(top_level_dir, path.text, .loose, false);
var path_clone = path;

// stack-allocated temporary is not leaked because dupeAlloc on the path will
// move .pretty into the heap. that function also fixes some slash issues.
if (target == .bake_server_components_ssr) {
// the SSR graph needs different pretty names or else HMR mode will
// confuse the two modules.
path_clone.pretty = std.fmt.bufPrint(&buf, "ssr:{s}", .{rel}) catch buf[0..];
} else {
path_clone.pretty = rel;
}
return path_clone.dupeAllocFixPretty(allocator);
}
pub const BundleV2 = struct {
bundler: *Bundler,
/// When Server Component is enabled, this is used for the client bundles
Expand Down Expand Up @@ -2274,25 +2290,7 @@ pub const BundleV2 = struct {
}

fn pathWithPrettyInitialized(this: *BundleV2, path: Fs.Path, target: options.Target) !Fs.Path {
if (path.pretty.ptr != path.text.ptr) {
// TODO(@paperdave): there is a high chance this dupe is no longer required
return path.dupeAlloc(this.graph.allocator);
}

// TODO: outbase
var buf: bun.PathBuffer = undefined;
const rel = bun.path.relativePlatform(this.bundler.fs.top_level_dir, path.text, .loose, false);
var path_clone = path;
// stack-allocated temporary is not leaked because dupeAlloc on the path will
// move .pretty into the heap. that function also fixes some slash issues.
if (target == .bake_server_components_ssr) {
// the SSR graph needs different pretty names or else HMR mode will
// confuse the two modules.
path_clone.pretty = std.fmt.bufPrint(&buf, "ssr:{s}", .{rel}) catch buf[0..];
} else {
path_clone.pretty = rel;
}
return path_clone.dupeAllocFixPretty(this.graph.allocator);
return genericPathWithPrettyInitialized(path, target, this.bundler.fs.top_level_dir, this.graph.allocator);
}

fn reserveSourceIndexesForBake(this: *BundleV2) !void {
Expand Down Expand Up @@ -5017,6 +5015,10 @@ pub const LinkerContext = struct {
dev_server: ?*bun.bake.DevServer = null,
framework: ?*const bake.Framework = null,

fn pathWithPrettyInitialized(this: *LinkerContext, path: Fs.Path) !Fs.Path {
return genericPathWithPrettyInitialized(path, this.options.target, this.resolver.fs.top_level_dir, this.graph.allocator);
}

pub const LinkerOptions = struct {
output_format: options.Format = .esm,
ignore_dce_annotations: bool = false,
Expand Down Expand Up @@ -9581,13 +9583,17 @@ pub const LinkerContext = struct {
if (chunk.content == .javascript) {
const sources = c.parse_graph.input_files.items(.source);
for (chunk.content.javascript.parts_in_chunk_in_order) |part_range| {
const source: Logger.Source = sources[part_range.source_index.get()];
const source: *Logger.Source = &sources[part_range.source_index.get()];

const file_path = brk: {
if (source.path.isFile()) {
// Use the pretty path as the file name since it should be platform-
// independent (relative paths and the "/" path separator)
if (source.path.text.ptr == source.path.pretty.ptr) {
source.path = c.pathWithPrettyInitialized(source.path) catch bun.outOfMemory();
}
source.path.assertPrettyIsValid();

break :brk source.path.pretty;
} else {
// If this isn't in the "file" namespace, just use the full path text
Expand Down
96 changes: 94 additions & 2 deletions test/bundler/cli.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tmpdirSync } from "harness";
import fs from "node:fs";
import path from "node:path";
import fs, { mkdirSync, realpathSync, rmSync, writeFileSync } from "node:fs";
import path, { join } from "node:path";
import { isWindows } from "harness";

describe("bun build", () => {
test("warnings dont return exit code 1", () => {
Expand Down Expand Up @@ -78,4 +79,95 @@ describe("bun build", () => {
expect(stdout.toString()).toContain(path.join(baseDir, "我") + "\n");
expect(stdout.toString()).toContain(path.join(baseDir, "我", "我.ts") + "\n");
});

test.skipIf(!isWindows)("should be able to handle pretty path when using pnpm + #14685", async () => {
// this test code follows the same structure as and
// is based on the code for testing issue 4893

let testDir = tmpdirSync();

// Clean up from prior runs if necessary
rmSync(testDir, { recursive: true, force: true });

// Create a directory with our test file
mkdirSync(testDir, { recursive: true });

writeFileSync(
join(testDir, "index.ts"),
"import chalk from \"chalk\"; export function main() { console.log(chalk.red('Hello, World!')); }",
);
writeFileSync(
join(testDir, "package.json"),
`
{
"dependencies": {
"chalk": "^5.3.0"
}
}`,
);
testDir = realpathSync(testDir);

Bun.spawnSync({
cmd: [bunExe(), "x", "pnpm@9", "i"],
env: bunEnv,
stderr: "pipe",
cwd: testDir,
});
// bun build --entrypoints ./index.ts --outdir ./dist --target node
const { stderr, exitCode } = Bun.spawnSync({
cmd: [
bunExe(),
"build",
"--entrypoints",
join(testDir, "index.ts"),
"--outdir",
join(testDir, "dist"),
"--target",
"node",
],
env: bunEnv,
stderr: "pipe",
stdout: "pipe",
});
expect(stderr.toString()).toBe("");
expect(exitCode).toBe(0);
});
}, 10_000);

test.skipIf(!isWindows)("should be able to handle pretty path on windows #13897", async () => {
// this test code follows the same structure as and
// is based on the code for testing issue 4893

let testDir = tmpdirSync();

// Clean up from prior runs if necessary
rmSync(testDir, { recursive: true, force: true });

// Create a directory with our test file
mkdirSync(testDir, { recursive: true });

writeFileSync(
join(testDir, "index.ts"),
"import chalk from \"chalk\"; export function main() { console.log(chalk.red('Hello, World!')); }",
);

writeFileSync(join(testDir, "chalk.ts"), "function red(value){ consol.error(value); } export default { red };");
testDir = realpathSync(testDir);

// bun build --entrypoints ./index.ts --outdir ./dist --target node
const buildOut = await Bun.build({
entrypoints: [join(testDir, "index.ts")],
outdir: join(testDir, "dist"),
minify: true,
sourcemap: "linked",
plugins: [
{
name: "My windows plugin",
async setup(build) {
build.onResolve({ filter: /chalk/ }, () => ({ path: join(testDir, "chalk.ts").replaceAll("/", "\\") }));
},
},
],
});
expect(buildOut?.success).toBe(true);
});

0 comments on commit e945146

Please sign in to comment.