From e58cf69f94f1426a32c340d7d415f839d719fe04 Mon Sep 17 00:00:00 2001 From: dave caruso Date: Thu, 20 Jun 2024 13:46:49 -0700 Subject: [PATCH] feat(bundler): Add `--sourcemap=linked` for `//# sourceMappingURL` comments (#11983) Co-authored-by: Jarred Sumner --- docs/bundler/index.md | 27 ++++++++++++----- packages/bun-types/bun.d.ts | 2 +- src/api/schema.zig | 9 ++++-- src/bun.js/api/JSBundler.zig | 29 +++++++++++++----- src/bun.js/api/JSTranspiler.zig | 6 ++-- src/bundler/bundle_v2.zig | 45 +++++++++++++++++++++++++--- src/cli.zig | 16 +++++++--- src/feature_flags.zig | 2 ++ src/options.zig | 23 ++++++++++---- src/sourcemap/sourcemap.zig | 2 +- test/bundler/esbuild/default.test.ts | 25 +++++++++++++++- test/bundler/expectBundled.ts | 2 +- 12 files changed, 149 insertions(+), 39 deletions(-) diff --git a/docs/bundler/index.md b/docs/bundler/index.md index 780e401343..514be87a9e 100644 --- a/docs/bundler/index.md +++ b/docs/bundler/index.md @@ -563,12 +563,12 @@ Specifies the type of sourcemap to generate. await Bun.build({ entrypoints: ['./index.tsx'], outdir: './out', - sourcemap: "external", // default "none" + sourcemap: 'linked', // default 'none' }) ``` ```bash#CLI -$ bun build ./index.tsx --outdir ./out --sourcemap=external +$ bun build ./index.tsx --outdir ./out --sourcemap=linked ``` {% /codetabs %} @@ -582,19 +582,19 @@ $ bun build ./index.tsx --outdir ./out --sourcemap=external --- -- `"inline"` -- A sourcemap is generated and appended to the end of the generated bundle as a base64 payload. +- `"linked"` +- A separate `*.js.map` file is created alongside each `*.js` bundle using a `//# sourceMappingURL` comment to link the two. Requires `--outdir` to be set. The base URL of this can be customized with `--public-path`. ```ts // - //# sourceMappingURL=data:application/json;base64, + //# sourceMappingURL=bundle.js.map ``` --- - `"external"` -- A separate `*.js.map` file is created alongside each `*.js` bundle. +- A separate `*.js.map` file is created alongside each `*.js` bundle without inserting a `//# sourceMappingURL` comment. {% /table %} @@ -608,7 +608,18 @@ Generated bundles contain a [debug id](https://sentry.engineering/blog/the-case- //# debugId= ``` -The associated `*.js.map` sourcemap will be a JSON file containing an equivalent `debugId` property. +--- + +- `"inline"` +- A sourcemap is generated and appended to the end of the generated bundle as a base64 payload. + + ```ts + // + + //# sourceMappingURL=data:application/json;base64, + ``` + + The associated `*.js.map` sourcemap will be a JSON file containing an equivalent `debugId` property. {% /callout %} @@ -1246,7 +1257,7 @@ interface BuildOptions { loader?: { [k in string]: Loader }; // See https://bun.sh/docs/bundler/loaders manifest?: boolean; // false external?: string[]; // [] - sourcemap?: "none" | "inline" | "external"; // "none" + sourcemap?: "none" | "inline" | "linked" | "external" | boolean; // "none" root?: string; // computed from entrypoints naming?: | string diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index efa69e06a7..7d97b0d08a 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -1520,7 +1520,7 @@ declare module "bun" { define?: Record; // origin?: string; // e.g. http://mydomain.com loader?: { [k in string]: Loader }; - sourcemap?: "none" | "inline" | "external"; // default: "none" + sourcemap?: "none" | "linked" | "inline" | "external"; // default: "none", true -> "inline" /** * package.json `exports` conditions used when resolving imports * diff --git a/src/api/schema.zig b/src/api/schema.zig index 30c416e153..666b9adc1f 100644 --- a/src/api/schema.zig +++ b/src/api/schema.zig @@ -1890,13 +1890,16 @@ pub const Api = struct { }; pub const SourceMapMode = enum(u8) { - _none, - /// inline_into_file - inline_into_file, + none, + + /// inline + @"inline", /// external external, + linked, + _, pub fn jsonStringify(self: @This(), writer: anytype) !void { diff --git a/src/bun.js/api/JSBundler.zig b/src/bun.js/api/JSBundler.zig index fe8a473e84..4998ba2996 100644 --- a/src/bun.js/api/JSBundler.zig +++ b/src/bun.js/api/JSBundler.zig @@ -199,8 +199,28 @@ pub const JSBundler = struct { this.target = target; } - if (try config.getOptionalEnum(globalThis, "sourcemap", options.SourceMapOption)) |source_map| { - this.source_map = source_map; + var has_out_dir = false; + if (try config.getOptional(globalThis, "outdir", ZigString.Slice)) |slice| { + defer slice.deinit(); + this.outdir.appendSliceExact(slice.slice()) catch unreachable; + has_out_dir = true; + } + + if (config.getTruthy(globalThis, "sourcemap")) |source_map_js| { + if (bun.FeatureFlags.breaking_changes_1_2 and config.isBoolean()) { + if (source_map_js == .true) { + this.source_map = if (has_out_dir) + .linked + else + .@"inline"; + } + } else if (!source_map_js.isEmptyOrUndefinedOrNull()) { + this.source_map = try source_map_js.toEnum( + globalThis, + "sourcemap", + options.SourceMapOption, + ); + } } if (try config.getOptionalEnum(globalThis, "format", options.Format)) |format| { @@ -221,11 +241,6 @@ pub const JSBundler = struct { this.code_splitting = hot; } - if (try config.getOptional(globalThis, "outdir", ZigString.Slice)) |slice| { - defer slice.deinit(); - this.outdir.appendSliceExact(slice.slice()) catch unreachable; - } - if (config.getTruthy(globalThis, "minify")) |hot| { if (hot.isBoolean()) { const value = hot.coerce(bool, globalThis); diff --git a/src/bun.js/api/JSTranspiler.zig b/src/bun.js/api/JSTranspiler.zig index f84ed4d490..9eda467a5e 100644 --- a/src/bun.js/api/JSTranspiler.zig +++ b/src/bun.js/api/JSTranspiler.zig @@ -573,15 +573,15 @@ fn transformOptionsFromJSC(globalObject: JSC.C.JSContextRef, temp_allocator: std if (object.get(globalThis, "sourcemap")) |flag| { if (flag.isBoolean() or flag.isUndefinedOrNull()) { if (flag.toBoolean()) { - transpiler.transform.source_map = Api.SourceMapMode.external; + transpiler.transform.source_map = .@"inline"; } else { - transpiler.transform.source_map = Api.SourceMapMode.inline_into_file; + transpiler.transform.source_map = .none; } } else { if (options.SourceMapOption.Map.fromJS(globalObject, flag)) |source| { transpiler.transform.source_map = source.toAPI(); } else { - JSC.throwInvalidArguments("sourcemap must be one of \"inline\", \"external\", or \"none\"", .{}, globalObject, exception); + JSC.throwInvalidArguments("sourcemap must be one of \"inline\", \"linked\", \"external\", or \"none\"", .{}, globalObject, exception); return transpiler; } } diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig index e0905423f3..948f07dd0a 100644 --- a/src/bundler/bundle_v2.zig +++ b/src/bundler/bundle_v2.zig @@ -759,6 +759,7 @@ pub const BundleV2 = struct { generator.linker.options.minify_whitespace = bundler.options.minify_whitespace; generator.linker.options.source_maps = bundler.options.source_map; generator.linker.options.tree_shaking = bundler.options.tree_shaking; + generator.linker.options.public_path = bundler.options.public_path; var pool = try generator.graph.allocator.create(ThreadPool); if (enable_reloading) { @@ -3801,7 +3802,7 @@ const LinkerContext = struct { /// We may need to refer to the CommonJS "module" symbol for exports unbound_module_ref: Ref = Ref.None, - options: LinkerOptions = LinkerOptions{}, + options: LinkerOptions = .{}, wait_group: ThreadPoolLib.WaitGroup = undefined, @@ -9293,7 +9294,7 @@ const LinkerContext = struct { var output_files = std.ArrayList(options.OutputFile).initCapacity( bun.default_allocator, - (if (c.options.source_maps == .external) chunks.len * 2 else chunks.len) + @as( + (if (c.options.source_maps.hasExternalFiles()) chunks.len * 2 else chunks.len) + @as( usize, @intFromBool(react_client_components_manifest.len > 0) + c.parse_graph.additional_output_files.items.len, ), @@ -9336,12 +9337,31 @@ const LinkerContext = struct { ); switch (c.options.source_maps) { - .external => { + .external, .linked => |tag| { const output_source_map = chunk.output_source_map.finalize(bun.default_allocator, code_result.shifts) catch @panic("Failed to allocate memory for external source map"); var source_map_final_rel_path = default_allocator.alloc(u8, chunk.final_rel_path.len + ".map".len) catch unreachable; bun.copy(u8, source_map_final_rel_path, chunk.final_rel_path); bun.copy(u8, source_map_final_rel_path[chunk.final_rel_path.len..], ".map"); + if (tag == .linked) { + const a, const b = if (c.options.public_path.len > 0) + cheapPrefixNormalizer(c.options.public_path, source_map_final_rel_path) + else + .{ "", std.fs.path.basename(source_map_final_rel_path) }; + + const source_map_start = "//# sourceMappingURL="; + const total_len = code_result.buffer.len + source_map_start.len + a.len + b.len + "\n".len; + var buf = std.ArrayList(u8).initCapacity(Chunk.IntermediateOutput.allocatorForSize(total_len), total_len) catch @panic("Failed to allocate memory for output file with inline source map"); + buf.appendSliceAssumeCapacity(code_result.buffer); + buf.appendSliceAssumeCapacity(source_map_start); + buf.appendSliceAssumeCapacity(a); + buf.appendSliceAssumeCapacity(b); + buf.appendAssumeCapacity('\n'); + + Chunk.IntermediateOutput.allocatorForSize(code_result.buffer.len).free(code_result.buffer); + code_result.buffer = buf.items; + } + sourcemap_output_file = options.OutputFile.init( options.OutputFile.Options{ .data = .{ @@ -9520,13 +9540,30 @@ const LinkerContext = struct { ); switch (c.options.source_maps) { - .external => { + .external, .linked => |tag| { const output_source_map = chunk.output_source_map.finalize(source_map_allocator, code_result.shifts) catch @panic("Failed to allocate memory for external source map"); const source_map_final_rel_path = strings.concat(default_allocator, &.{ chunk.final_rel_path, ".map", }) catch @panic("Failed to allocate memory for external source map path"); + if (tag == .linked) { + const a, const b = if (c.options.public_path.len > 0) + cheapPrefixNormalizer(c.options.public_path, source_map_final_rel_path) + else + .{ "", std.fs.path.basename(source_map_final_rel_path) }; + + const source_map_start = "//# sourceMappingURL="; + const total_len = code_result.buffer.len + source_map_start.len + a.len + b.len + "\n".len; + var buf = std.ArrayList(u8).initCapacity(Chunk.IntermediateOutput.allocatorForSize(total_len), total_len) catch @panic("Failed to allocate memory for output file with inline source map"); + buf.appendSliceAssumeCapacity(code_result.buffer); + buf.appendSliceAssumeCapacity(source_map_start); + buf.appendSliceAssumeCapacity(a); + buf.appendSliceAssumeCapacity(b); + buf.appendAssumeCapacity('\n'); + code_result.buffer = buf.items; + } + switch (JSC.Node.NodeFS.writeFileWithPathBuffer( &pathbuf, JSC.Node.Arguments.WriteFile{ diff --git a/src/cli.zig b/src/cli.zig index 36a6d3e24b..2be67169ef 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -770,12 +770,20 @@ pub const Arguments = struct { } if (args.option("--sourcemap")) |setting| { - if (setting.len == 0 or strings.eqlComptime(setting, "inline")) { - opts.source_map = Api.SourceMapMode.inline_into_file; + if (setting.len == 0) { + // In the future, Bun is going to make this default to .linked + opts.source_map = if (bun.FeatureFlags.breaking_changes_1_2) + .linked + else + .@"inline"; + } else if (strings.eqlComptime(setting, "inline")) { + opts.source_map = .@"inline"; } else if (strings.eqlComptime(setting, "none")) { - opts.source_map = Api.SourceMapMode._none; + opts.source_map = .none; } else if (strings.eqlComptime(setting, "external")) { - opts.source_map = Api.SourceMapMode.external; + opts.source_map = .external; + } else if (strings.eqlComptime(setting, "linked")) { + opts.source_map = .linked; } else { Output.prettyErrorln("error: Invalid sourcemap setting: \"{s}\"", .{setting}); Global.crash(); diff --git a/src/feature_flags.zig b/src/feature_flags.zig index 93bb8f0309..a53ad08ed1 100644 --- a/src/feature_flags.zig +++ b/src/feature_flags.zig @@ -173,6 +173,8 @@ pub const runtime_transpiler_cache = true; /// order to isolate your bug. pub const windows_bunx_fast_path = true; +/// Enable breaking changes for the next major release of Bun +// TODO: Make this a CLI flag / runtime var so that we can verify disabled code paths can compile pub const breaking_changes_1_2 = false; // This causes strange bugs where writing via console.log (sync) has a different diff --git a/src/options.zig b/src/options.zig index 260130b21d..b872c2561e 100644 --- a/src/options.zig +++ b/src/options.zig @@ -1360,11 +1360,13 @@ pub const SourceMapOption = enum { none, @"inline", external, + linked, pub fn fromApi(source_map: ?Api.SourceMapMode) SourceMapOption { - return switch (source_map orelse Api.SourceMapMode._none) { - Api.SourceMapMode.external => .external, - Api.SourceMapMode.inline_into_file => .@"inline", + return switch (source_map orelse .none) { + .external => .external, + .@"inline" => .@"inline", + .linked => .linked, else => .none, }; } @@ -1372,8 +1374,16 @@ pub const SourceMapOption = enum { pub fn toAPI(source_map: ?SourceMapOption) Api.SourceMapMode { return switch (source_map orelse .none) { .external => .external, - .@"inline" => .inline_into_file, - else => ._none, + .@"inline" => .@"inline", + .linked => .linked, + .none => .none, + }; + } + + pub fn hasExternalFiles(mode: SourceMapOption) bool { + return switch (mode) { + .linked, .external => true, + else => false, }; } @@ -1381,6 +1391,7 @@ pub const SourceMapOption = enum { .{ "none", .none }, .{ "inline", .@"inline" }, .{ "external", .external }, + .{ "linked", .linked }, }); }; @@ -1742,7 +1753,7 @@ pub const BundleOptions = struct { opts.external = ExternalModules.init(allocator, &fs.fs, fs.top_level_dir, transform.external, log, opts.target); opts.out_extensions = opts.target.outExtensions(allocator); - opts.source_map = SourceMapOption.fromApi(transform.source_map orelse Api.SourceMapMode._none); + opts.source_map = SourceMapOption.fromApi(transform.source_map orelse .none); opts.tree_shaking = opts.target.isBun() or opts.production; opts.inlining = opts.tree_shaking; diff --git a/src/sourcemap/sourcemap.zig b/src/sourcemap/sourcemap.zig index 6f4855fac9..2beba9bfda 100644 --- a/src/sourcemap/sourcemap.zig +++ b/src/sourcemap/sourcemap.zig @@ -1904,7 +1904,7 @@ pub const DebugIDFormatter = struct { // We fill the end of the id with "bun!bun!" hex encoded var buf: [32]u8 = undefined; const formatter = bun.fmt.hexIntUpper(self.id); - _ = std.fmt.bufPrint(&buf, "{}64756e2164756e21", .{formatter}) catch unreachable; + _ = std.fmt.bufPrint(&buf, "{}64756E2164756E21", .{formatter}) catch unreachable; try writer.writeAll(&buf); } }; diff --git a/test/bundler/esbuild/default.test.ts b/test/bundler/esbuild/default.test.ts index e4b24d8ca5..59cf373a95 100644 --- a/test/bundler/esbuild/default.test.ts +++ b/test/bundler/esbuild/default.test.ts @@ -1149,7 +1149,6 @@ describe("bundler", () => { }, }); itBundled("default/SourceMap", { - todo: true, files: { "/Users/user/project/src/entry.js": /* js */ ` import {bar} from './bar' @@ -1162,6 +1161,30 @@ describe("bundler", () => { sourceMap: "external", onAfterBundle(api) { const json = JSON.parse(api.readFile("/Users/user/project/out/entry.js.map")); + api.expectFile("/Users/user/project/out/entry.js").not.toContain(`//# sourceMappingURL`); + api.expectFile("/Users/user/project/out/entry.js").toContain(`//# debugId=${json.debugId}`); + // see src/sourcemap/sourcemap.zig DebugIDFormatter for more info + expect(json.debugId).toMatch(/^[A-F0-9]{32}$/); + expect(json.debugId.endsWith("64756e2164756e21")); + }, + run: { + stdout: "hi", + }, + }); + itBundled("default/SourceMapLinked", { + files: { + "/Users/user/project/src/entry.js": /* js */ ` + import {bar} from './bar' + function foo() { bar() } + foo() + `, + "/Users/user/project/src/bar.js": `export function bar() { console.log('hi') }`, + }, + outdir: "/Users/user/project/out", + sourceMap: "linked", + onAfterBundle(api) { + const json = JSON.parse(api.readFile("/Users/user/project/out/entry.js.map")); + api.expectFile("/Users/user/project/out/entry.js").toContain(`//# sourceMappingURL=entry.js.map`); api.expectFile("/Users/user/project/out/entry.js").toContain(`//# debugId=${json.debugId}`); // see src/sourcemap/sourcemap.zig DebugIDFormatter for more info expect(json.debugId).toMatch(/^[A-F0-9]{32}$/); diff --git a/test/bundler/expectBundled.ts b/test/bundler/expectBundled.ts index 47c4a7c32b..86398d6fc0 100644 --- a/test/bundler/expectBundled.ts +++ b/test/bundler/expectBundled.ts @@ -196,7 +196,7 @@ export interface BundlerTestInput { unsupportedJSFeatures?: string[]; /** if set to true or false, create or edit tsconfig.json to set compilerOptions.useDefineForClassFields */ useDefineForClassFields?: boolean; - sourceMap?: "inline" | "external" | "none"; + sourceMap?: "inline" | "external" | "linked" | "none"; plugins?: BunPlugin[] | ((builder: PluginBuilder) => void | Promise); install?: string[];