diff --git a/src/StringJoiner.zig b/src/StringJoiner.zig index a1385b998f..3691090616 100644 --- a/src/StringJoiner.zig +++ b/src/StringJoiner.zig @@ -56,6 +56,7 @@ pub fn pushStatic(this: *StringJoiner, data: []const u8) void { /// `data` is cloned pub fn pushCloned(this: *StringJoiner, data: []const u8) void { + if (data.len == 0) return; this.push( this.allocator.dupe(u8, data) catch bun.outOfMemory(), this.allocator, @@ -63,6 +64,7 @@ pub fn pushCloned(this: *StringJoiner, data: []const u8) void { } pub fn push(this: *StringJoiner, data: []const u8, allocator: ?Allocator) void { + if (data.len == 0) return; this.len += data.len; const new_tail = Node.init(this.allocator, data, allocator); @@ -142,7 +144,8 @@ pub fn doneWithEnd(this: *StringJoiner, allocator: Allocator, end: []const u8) ! pub fn lastByte(this: *const StringJoiner) u8 { const slice = (this.tail orelse return 0).slice; - return if (slice.len > 0) slice[slice.len - 1] else 0; + assert(slice.len > 0); + return slice[slice.len - 1]; } pub fn ensureNewlineAtEnd(this: *StringJoiner) void { diff --git a/src/bun.js/WebKit b/src/bun.js/WebKit index ddc47cfa03..2c4f31e109 160000 --- a/src/bun.js/WebKit +++ b/src/bun.js/WebKit @@ -1 +1 @@ -Subproject commit ddc47cfa03b87d48217079b7aaa64d23ebd3c3a2 +Subproject commit 2c4f31e10974404bc8316a70d491ec0f400c880d diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index fdd8711cc2..754a177c14 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -190,8 +190,10 @@ pub const SavedSourceMap = struct { pub const MissingSourceMapNoteInfo = struct { pub var storage: bun.PathBuffer = undefined; pub var path: ?[]const u8 = null; + pub var seen_invalid = false; pub fn print() void { + if (seen_invalid) return; if (path) |note| { Output.note( "missing sourcemaps for {s}", diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig index 0dd0a7f769..f32b0ae74d 100644 --- a/src/bundler/bundle_v2.zig +++ b/src/bundler/bundle_v2.zig @@ -6903,18 +6903,18 @@ const LinkerContext = struct { line_offset.advance(compile_result.code()); j.push(compile_result.code(), bun.default_allocator); } else { - const generated_offset = line_offset; j.push(compile_result.code(), bun.default_allocator); if (compile_result.source_map_chunk()) |source_map_chunk| { - line_offset.reset(); if (c.options.source_maps != .none) { try compile_results_for_source_map.append(worker.allocator, CompileResultForSourceMap{ .source_map_chunk = source_map_chunk, - .generated_offset = generated_offset.value, + .generated_offset = line_offset.value, .source_index = compile_result.sourceIndex(), }); } + + line_offset.reset(); } else { line_offset.advance(compile_result.code()); } diff --git a/src/js_printer.zig b/src/js_printer.zig index 42fffe91d8..cfe0cd2530 100644 --- a/src/js_printer.zig +++ b/src/js_printer.zig @@ -1381,7 +1381,6 @@ fn NewPrinter( if (comptime !generate_source_map) { return; } - printer.source_map_builder.addSourceMapping(location, printer.writer.slice()); } @@ -1389,8 +1388,6 @@ fn NewPrinter( if (comptime !generate_source_map) { return; } - - // TODO: printer.addSourceMapping(location); } diff --git a/src/sourcemap/sourcemap.zig b/src/sourcemap/sourcemap.zig index e46c136eae..1e59b552ad 100644 --- a/src/sourcemap/sourcemap.zig +++ b/src/sourcemap/sourcemap.zig @@ -484,11 +484,6 @@ pub const Mapping = struct { } remain = remain[source_index_delta.start..]; - // // "AAAA" is extremely common - // if (strings.hasPrefixComptime(remain, "AAAA;")) { - - // } - // Read the original line const original_line_delta = decodeVLQ(remain, 0); if (original_line_delta.start == 0) { @@ -713,7 +708,15 @@ pub const SourceProviderMap = opaque { arena.allocator(), found_url.slice(), result, - ) catch return null, + ) catch |err| { + bun.Output.warn("Could not decode sourcemap in '{s}': {s}", .{ + source_filename, + @errorName(err), + }); + // Disable the "try using --sourcemap=external" hint + bun.JSC.SavedSourceMap.MissingSourceMapNoteInfo.seen_invalid = true; + return null; + }, }; } @@ -725,11 +728,8 @@ pub const SourceProviderMap = opaque { @memcpy(load_path_buf[0..source_filename.len], source_filename); @memcpy(load_path_buf[source_filename.len..][0..4], ".map"); - const data = switch (bun.sys.File.readFrom( - std.fs.cwd(), - load_path_buf[0 .. source_filename.len + 4], - arena.allocator(), - )) { + const load_path = load_path_buf[0 .. source_filename.len + 4]; + const data = switch (bun.sys.File.readFrom(std.fs.cwd(), load_path, arena.allocator())) { .err => break :try_external, .result => |data| data, }; @@ -741,7 +741,15 @@ pub const SourceProviderMap = opaque { arena.allocator(), data, result, - ) catch return null, + ) catch |err| { + bun.Output.warn("Could not decode sourcemap '{s}': {s}", .{ + source_filename, + @errorName(err), + }); + // Disable the "try using --sourcemap=external" hint + bun.JSC.SavedSourceMap.MissingSourceMapNoteInfo.seen_invalid = true; + return null; + }, }; } @@ -766,14 +774,14 @@ pub const LineColumnOffset = struct { pub fn advance(this: *Optional, input: []const u8) void { switch (this.*) { .null => {}, - .value => this.value.advance(input), + .value => |*v| v.advance(input), } } pub fn reset(this: *Optional) void { switch (this.*) { .null => {}, - .value => this.value = .{}, + .value => this.* = .{ .value = .{} }, } } }; @@ -787,9 +795,13 @@ pub const LineColumnOffset = struct { } } - pub fn advance(this: *LineColumnOffset, input: []const u8) void { - var columns = this.columns; - defer this.columns = columns; + pub fn advance(this_ptr: *LineColumnOffset, input: []const u8) void { + // Instead of mutating `this_ptr` directly, copy the state to the stack and do + // all the work here, then move it back to the input pointer. When sourcemaps + // are enabled, this function is extremely hot. + var this = this_ptr.*; + defer this_ptr.* = this; + var offset: u32 = 0; while (strings.indexOfNewlineOrNonASCII(input, offset)) |i| { assert(i >= offset); @@ -803,7 +815,7 @@ pub const LineColumnOffset = struct { // This can lead to integer overflow, crashes, or hangs. // https://github.com/oven-sh/bun/issues/10624 if (cursor.width == 0) { - columns += 1; + this.columns += 1; offset = i + 1; continue; } @@ -814,22 +826,32 @@ pub const LineColumnOffset = struct { '\r', '\n', 0x2028, 0x2029 => { // Handle Windows-specific "\r\n" newlines if (cursor.c == '\r' and input.len > i + 1 and input[i + 1] == '\n') { - columns += 1; + this.columns += 1; continue; } this.lines += 1; - columns = 0; + this.columns = 0; }, else => |c| { // Mozilla's "source-map" library counts columns using UTF-16 code units - columns += switch (c) { + this.columns += switch (c) { 0...0xFFFF => 1, else => 2, }; }, } } + + const remain = input[offset..]; + + if (bun.Environment.allow_assert) { + assert(bun.strings.isAllASCII(remain)); + assert(!bun.strings.containsChar(remain, '\n')); + assert(!bun.strings.containsChar(remain, '\r')); + } + + this.columns += @intCast(remain.len); } pub fn comesBefore(a: LineColumnOffset, b: LineColumnOffset) bool { @@ -977,43 +999,44 @@ pub fn appendSourceMapChunk(j: *StringJoiner, allocator: std.mem.Allocator, prev var prev_end_state = prev_end_state_; var start_state = start_state_; // Handle line breaks in between this mapping and the previous one - if (start_state.generated_line > 0) { + if (start_state.generated_line != 0) { j.push(try strings.repeatingAlloc(allocator, @intCast(start_state.generated_line), ';'), allocator); prev_end_state.generated_column = 0; } + // Skip past any leading semicolons, which indicate line breaks var source_map = source_map_; if (strings.indexOfNotChar(source_map, ';')) |semicolons| { - j.pushStatic(source_map[0..semicolons]); - source_map = source_map[semicolons..]; - prev_end_state.generated_column = 0; - start_state.generated_column = 0; + if (semicolons > 0) { + j.pushStatic(source_map[0..semicolons]); + source_map = source_map[semicolons..]; + prev_end_state.generated_column = 0; + start_state.generated_column = 0; + } } // Strip off the first mapping from the buffer. The first mapping should be // for the start of the original file (the printer always generates one for // the start of the file). - // - // Bun has a 24-byte header for source map meta-data var i: usize = 0; - const generated_column_ = decodeVLQAssumeValid(source_map, i); - i = generated_column_.start; - const source_index_ = decodeVLQAssumeValid(source_map, i); - i = source_index_.start; - const original_line_ = decodeVLQAssumeValid(source_map, i); - i = original_line_.start; - const original_column_ = decodeVLQAssumeValid(source_map, i); - i = original_column_.start; + const generated_column = decodeVLQAssumeValid(source_map, i); + i = generated_column.start; + const source_index = decodeVLQAssumeValid(source_map, i); + i = source_index.start; + const original_line = decodeVLQAssumeValid(source_map, i); + i = original_line.start; + const original_column = decodeVLQAssumeValid(source_map, i); + i = original_column.start; source_map = source_map[i..]; // Rewrite the first mapping to be relative to the end state of the previous // chunk. We now know what the end state is because we're in the second pass // where all chunks have already been generated. - start_state.source_index += source_index_.value; - start_state.generated_column += generated_column_.value; - start_state.original_line += original_line_.value; - start_state.original_column += original_column_.value; + start_state.source_index += source_index.value; + start_state.generated_column += generated_column.value; + start_state.original_line += original_line.value; + start_state.original_column += original_column.value; j.push( appendMappingToBuffer( diff --git a/test/bundler/bundler_edgecase.test.ts b/test/bundler/bundler_edgecase.test.ts index 2676f55703..56ef384136 100644 --- a/test/bundler/bundler_edgecase.test.ts +++ b/test/bundler/bundler_edgecase.test.ts @@ -574,7 +574,7 @@ describe("bundler", () => { break; } console.log(a); - + var x = 123, y = 45; switch (console) { case 456: @@ -582,14 +582,14 @@ describe("bundler", () => { } var y = 67; console.log(x, y); - + var z = 123; switch (console) { default: var z = typeof z; } console.log(z); - + var A = 1, B = 2; switch (A) { case A: @@ -1079,6 +1079,31 @@ describe("bundler", () => { minifyWhitespace: true, splitting: true, }); + // chunk-concat weaved mappings together incorrectly causing the `console` + // token to be -2, thus breaking the rest of the mappings in the file + itBundled("edgecase/EmitInvalidSourceMap2", { + files: { + "/entry.js": ` + import * as react from "react"; + console.log(react); + `, + "/node_modules/react/index.js": ` + var _ = module; + sideEffect(() => {}); + `, + }, + outdir: "/out", + sourceMap: "external", + minifySyntax: true, + minifyIdentifiers: true, + minifyWhitespace: true, + snapshotSourceMap: { + "entry.js.map": { + files: ["../node_modules/react/index.js", "../entry.js"], + mappingsExactMatch: "uYACA,WAAW,IAAQ,EAAE,ICDrB,eACA,QAAQ,IAAI,CAAK", + }, + }, + }); // TODO(@paperdave): test every case of this. I had already tested it manually, but it may break later const requireTranspilationListESM = [ diff --git a/test/bundler/bundler_npm.test.ts b/test/bundler/bundler_npm.test.ts index b3661ce0e2..de2032597b 100644 --- a/test/bundler/bundler_npm.test.ts +++ b/test/bundler/bundler_npm.test.ts @@ -4,7 +4,7 @@ var { describe, test, expect } = testForFile(import.meta.path); describe("bundler", () => { itBundled("npm/ReactSSR", { todo: process.platform === "win32", // TODO(@paperdave) - install: ["react@next", "react-dom@next"], + install: ["react@18.3.1", "react-dom@18.3.1"], files: { "/entry.tsx": /* tsx */ ` import React from "react"; @@ -37,8 +37,36 @@ describe("bundler", () => { console.log(await res.text()); `, }, + // this test serves two purposes + // - does react work when bundled + // - do sourcemaps on a real-world library work + sourceMap: "external", + outdir: "out/", + minifySyntax: true, + minifyWhitespace: true, + minifyIdentifiers: true, + snapshotSourceMap: { + "entry.js.map": { + files: [ + "../node_modules/react/cjs/react.development.js", + "../node_modules/react/cjs/react-jsx-dev-runtime.development.js", + "../node_modules/react-dom/cjs/react-dom-server-legacy.browser.development.js", + "../node_modules/react-dom/cjs/react-dom-server.browser.development.js", + "../node_modules/react-dom/server.browser.js", + "../entry.tsx", + ], + mappings: [ + ["react.development.js:524:'getContextName'", "1:5404:r1"], + ["react.development.js:2495:'actScopeDepth'", "1:26072:GJ++"], + ["react.development.js:696:''Component'", '1:7470:\'Component "%s"'], + ["entry.tsx:6:'\"Content-Type\"'", '1:221674:"Content-Type"'], + ["entry.tsx:11:''", "1:221930:void"], + ["entry.tsx:23:'await'", "1:222035:await"], + ], + }, + }, run: { - stdout: "
This is an example.
", + stdout: "This is an example.
", }, }); itBundled("npm/LodashES", { diff --git a/test/bundler/expectBundled.ts b/test/bundler/expectBundled.ts index a1b26b388c..e320eb4542 100644 --- a/test/bundler/expectBundled.ts +++ b/test/bundler/expectBundled.ts @@ -1,9 +1,9 @@ /** * See `./expectBundled.md` for how this works. */ -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync, readdirSync } from "fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync, readdirSync, realpathSync } from "fs"; import path from "path"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, joinP } from "harness"; import { tmpdir } from "os"; import { callerSourceOrigin } from "bun:jsc"; import { BuildConfig, BunPlugin, fileURLToPath } from "bun"; @@ -105,7 +105,7 @@ const HIDE_SKIP = process.env.BUN_BUNDLER_TEST_HIDE_SKIP; const BUN_EXE = (process.env.BUN_EXE && Bun.which(process.env.BUN_EXE)) ?? bunExe(); export const RUN_UNCHECKED_TESTS = false; -const tempDirectoryTemplate = path.join(tmpdir(), "bun-build-tests", `${ESBUILD ? "esbuild" : "bun"}-`); +const tempDirectoryTemplate = path.join(realpathSync(tmpdir()), "bun-build-tests", `${ESBUILD ? "esbuild" : "bun"}-`); if (!existsSync(path.dirname(tempDirectoryTemplate))) mkdirSync(path.dirname(tempDirectoryTemplate), { recursive: true }); const tempDirectory = mkdtempSync(tempDirectoryTemplate); @@ -263,8 +263,35 @@ export interface BundlerTestInput { /* TODO: remove this from the tests after this is implemented */ skipIfWeDidNotImplementWildcardSideEffects?: boolean; + + snapshotSourceMap?: Record