Don't include //# sourcemap comments in .html or .css files (#16159)

oven-sh · Jan 4, 2025 · 33233b1 · 33233b1
1 parent ed0b4e1
commit 33233b1
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 4 deletions.
diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig
@@ -12961,7 +12961,7 @@ pub const LinkerContext = struct {
                     chunk,
                     chunks,
                     &display_size,
-                    c.options.source_maps != .none,
+                    chunk.content.sourcemap(c.options.source_maps) != .none,
                 );
                 var code_result = _code_result catch @panic("Failed to allocate memory for output file");
 
@@ -12974,7 +12974,7 @@ pub const LinkerContext = struct {
                         chunk.final_rel_path,
                 );
 
-                switch (c.options.source_maps) {
+                switch (chunk.content.sourcemap(c.options.source_maps)) {
                     .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;
@@ -13280,7 +13280,7 @@ pub const LinkerContext = struct {
                 chunk,
                 chunks,
                 &display_size,
-                c.options.source_maps != .none,
+                chunk.content.sourcemap(c.options.source_maps) != .none,
             ) catch |err| bun.Output.panic("Failed to create output chunk: {s}", .{@errorName(err)});
 
             var source_map_output_file: ?options.OutputFile = null;
@@ -13293,7 +13293,7 @@ pub const LinkerContext = struct {
                     chunk.final_rel_path,
             );
 
-            switch (c.options.source_maps) {
+            switch (chunk.content.sourcemap(c.options.source_maps)) {
                 .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, &.{
@@ -15463,6 +15463,18 @@ pub const Chunk = struct {
         javascript: JavaScriptChunk,
         css: CssChunk,
         html: HtmlChunk,
+
+        pub fn sourcemap(this: *const Content, default: options.SourceMapOption) options.SourceMapOption {
+            return switch (this.*) {
+                .javascript => default,
+                // TODO:
+                .css => options.SourceMapOption.none,
+
+                // probably never
+                .html => options.SourceMapOption.none,
+            };
+        }
+
         pub fn loader(this: *const Content) Loader {
             return switch (this.*) {
                 .javascript => .js,

diff --git a/test/bundler/bundler_html.test.ts b/test/bundler/bundler_html.test.ts
@@ -809,4 +809,107 @@ body {
       expect(jsBundle).toContain("App loaded");
     },
   });
+
+  // Test that sourcemap comments are not included in HTML and CSS files
+  itBundled("html/no-sourcemap-comments", {
+    outdir: "out/",
+    sourceMap: "linked",
+    files: {
+      "/index.html": `
+<!DOCTYPE html>
+<html>
+  <head>
+    <link rel="stylesheet" href="./styles.css">
+    <script src="./script.js"></script>
+  </head>
+  <body>
+    <h1>No Sourcemap Comments</h1>
+  </body>
+</html>`,
+      "/styles.css": `
+body {
+  background-color: red;
+}
+/* This is a comment */`,
+      "/script.js": "console.log('Hello World')",
+    },
+    experimentalHtml: true,
+    experimentalCss: true,
+    sourceMap: "linked",
+    entryPoints: ["/index.html"],
+    onAfterBundle(api) {
+      // Check HTML file doesn't contain sourcemap comments
+      const htmlContent = api.readFile("out/index.html");
+      api.expectFile("out/index.html").not.toContain("sourceMappingURL");
+      api.expectFile("out/index.html").not.toContain("debugId");
+
+      // Get the CSS filename from the HTML
+      const cssMatch = htmlContent.match(/href="(.*\.css)"/);
+      expect(cssMatch).not.toBeNull();
+      const cssFile = cssMatch![1];
+
+      // Check CSS file doesn't contain sourcemap comments
+      api.expectFile("out/" + cssFile).not.toContain("sourceMappingURL");
+      api.expectFile("out/" + cssFile).not.toContain("debugId");
+
+      // Get the JS filename from the HTML
+      const jsMatch = htmlContent.match(/src="(.*\.js)"/);
+      expect(jsMatch).not.toBeNull();
+      const jsFile = jsMatch![1];
+
+      // JS file SHOULD contain sourcemap comment since it's supported
+      api.expectFile("out/" + jsFile).toContain("sourceMappingURL");
+    },
+  });
+
+  // Also test with inline sourcemaps
+  itBundled("html/no-sourcemap-comments-inline", {
+    outdir: "out/",
+    files: {
+      "/index.html": `
+<!DOCTYPE html>
+<html>
+  <head>
+    <link rel="stylesheet" href="./styles.css">
+    <script src="./script.js"></script>
+  </head>
+  <body>
+    <h1>No Sourcemap Comments</h1>
+  </body>
+</html>`,
+      "/styles.css": `
+body {
+  background-color: red;
+}
+/* This is a comment */`,
+      "/script.js": "console.log('Hello World')",
+    },
+    experimentalHtml: true,
+    experimentalCss: true,
+    sourceMap: "inline",
+    entryPoints: ["/index.html"],
+    onAfterBundle(api) {
+      // Check HTML file doesn't contain sourcemap comments
+      const htmlContent = api.readFile("out/index.html");
+      api.expectFile("out/index.html").not.toContain("sourceMappingURL");
+      api.expectFile("out/index.html").not.toContain("debugId");
+
+      // Get the CSS filename from the HTML
+      const cssMatch = htmlContent.match(/href="(.*\.css)"/);
+      expect(cssMatch).not.toBeNull();
+      const cssFile = cssMatch![1];
+
+      // Check CSS file doesn't contain sourcemap comments
+      api.expectFile("out/" + cssFile).not.toContain("sourceMappingURL");
+      api.expectFile("out/" + cssFile).not.toContain("debugId");
+
+      // Get the JS filename from the HTML
+      const jsMatch = htmlContent.match(/src="(.*\.js)"/);
+      expect(jsMatch).not.toBeNull();
+      const jsFile = jsMatch![1];
+
+      // JS file SHOULD contain sourcemap comment since it's supported
+      api.expectFile("out/" + jsFile).toContain("sourceMappingURL");
+    },
+  });
 });