Merge branch 'main' into timfish/test/ensure-debug-ids-match

timfish · web-flow · commit ad9f55e50418 · 2026-01-16T16:48:48.000+01:00
diff --git a/.github/workflows/changelog-preview.yml b/.github/workflows/changelog-preview.yml
@@ -1,6 +1,6 @@
 name: Changelog Preview
 on:
-  pull_request:
+  pull_request_target:
     types:
       - opened
       - synchronize
diff --git a/packages/bundler-plugin-core/src/build-plugin-manager.ts b/packages/bundler-plugin-core/src/build-plugin-manager.ts
@@ -23,6 +23,7 @@ import {
   arrayify,
   getProjects,
   getTurborepoEnvPassthroughWarning,
+  serializeIgnoreOptions,
   stripQueryAndHashFromPath,
 } from "./utils";
 import { glob } from "glob";
@@ -554,7 +555,12 @@ export function createSentryBuildPluginManager(
           try {
             const cliInstance = createCliInstance(options);
             await cliInstance.execute(
-              ["sourcemaps", "inject", ...buildArtifactPaths],
+              [
+                "sourcemaps",
+                "inject",
+                ...serializeIgnoreOptions(options.sourcemaps?.ignore),
+                ...buildArtifactPaths,
+              ],
               options.debug ? "rejectOnError" : false
             );
           } catch (e) {
diff --git a/packages/bundler-plugin-core/src/index.ts b/packages/bundler-plugin-core/src/index.ts
@@ -12,6 +12,7 @@ import { createDebugIdUploadFunction } from "./debug-id-upload";
 import { Logger } from "./logger";
 import { Options, SentrySDKBuildFlags } from "./types";
 import {
+  containsOnlyImports,
   generateGlobalInjectorCode,
   generateModuleMetadataInjectorCode,
   replaceBooleanFlagsInCode,
@@ -229,6 +230,9 @@ function isJsFile(fileName: string): boolean {
  * HTML entry points create "facade" chunks that should not contain injected code.
  * See: https://github.com/getsentry/sentry-javascript-bundler-plugins/issues/829
  *
+ * However, in SPA mode, the main bundle also has an HTML facade but contains
+ * substantial application code. We should NOT skip injection for these bundles.
+ *
  * @param code - The chunk's code content
  * @param facadeModuleId - The facade module ID (if any) - HTML files create facade chunks
  * @returns true if the chunk should be skipped
@@ -239,10 +243,9 @@ function shouldSkipCodeInjection(code: string, facadeModuleId: string | null | u
     return true;
   }
 
-  // Skip HTML facade chunks
-  // They only contain import statements and should not have Sentry code injected
+  // For HTML facade chunks, only skip if they contain only import statements
   if (facadeModuleId && stripQueryAndHashFromPath(facadeModuleId).endsWith(".html")) {
-    return true;
+    return containsOnlyImports(code);
   }
 
   return false;
diff --git a/packages/bundler-plugin-core/src/utils.ts b/packages/bundler-plugin-core/src/utils.ts
@@ -408,3 +408,54 @@ export function getProjects(project: string | string[] | undefined): string[] |
 
   return undefined;
 }
+
+/**
+ * Inlined functionality from @sentry/cli helper code to add `--ignore` options.
+ *
+ * Temporary workaround until we expose a function for injecting debug IDs. Currently, we directly call `execute` with CLI args to inject them.
+ */
+export function serializeIgnoreOptions(ignoreValue: string | string[] | undefined): string[] {
+  const DEFAULT_IGNORE = ["node_modules"];
+
+  const ignoreOptions: string[] = Array.isArray(ignoreValue)
+    ? ignoreValue
+    : typeof ignoreValue === "string"
+    ? [ignoreValue]
+    : DEFAULT_IGNORE;
+
+  return ignoreOptions.reduce(
+    (acc, value) => acc.concat(["--ignore", String(value)]),
+    [] as string[]
+  );
+}
+
+/**
+ * Checks if a chunk contains only import/export statements and no substantial code.
+ *
+ * In Vite MPA (multi-page application) mode, HTML entry points create "facade" chunks
+ * that only contain import statements to load shared modules. These should not have
+ * Sentry code injected. However, in SPA mode, the main bundle also has an HTML facade
+ * but contains substantial application code that SHOULD have debug IDs injected.
+ *
+ * @ref https://github.com/getsentry/sentry-javascript-bundler-plugins/issues/829
+ * @ref https://github.com/getsentry/sentry-javascript-bundler-plugins/issues/839
+ */
+export function containsOnlyImports(code: string): boolean {
+  const codeWithoutImports = code
+    // Remove side effect imports: import '/path'; or import "./path";
+    // Using explicit negated character classes to avoid polynomial backtracking
+    .replace(/^\s*import\s+(?:'[^'\n]*'|"[^"\n]*"|`[^`\n]*`)[\s;]*$/gm, "")
+    // Remove named/default imports: import x from '/path'; import { x } from '/path';
+    .replace(/^\s*import\b[^'"`\n]*\bfrom\s+(?:'[^'\n]*'|"[^"\n]*"|`[^`\n]*`)[\s;]*$/gm, "")
+    // Remove re-exports: export * from '/path'; export { x } from '/path';
+    .replace(/^\s*export\b[^'"`\n]*\bfrom\s+(?:'[^'\n]*'|"[^"\n]*"|`[^`\n]*`)[\s;]*$/gm, "")
+    // Remove block comments
+    .replace(/\/\*[\s\S]*?\*\//g, "")
+    // Remove line comments
+    .replace(/\/\/.*$/gm, "")
+    // Remove "use strict" directives
+    .replace(/["']use strict["']\s*;?/g, "")
+    .trim();
+
+  return codeWithoutImports.length === 0;
+}
diff --git a/packages/bundler-plugin-core/test/build-plugin-manager.test.ts b/packages/bundler-plugin-core/test/build-plugin-manager.test.ts
@@ -434,7 +434,7 @@ describe("createSentryBuildPluginManager", () => {
       await buildPluginManager.injectDebugIds(buildArtifactPaths);
 
       expect(mockCliExecute).toHaveBeenCalledWith(
-        ["sourcemaps", "inject", "/path/to/1", "/path/to/2"],
+        ["sourcemaps", "inject", "--ignore", "node_modules", "/path/to/1", "/path/to/2"],
         false
       );
     });
@@ -459,7 +459,7 @@ describe("createSentryBuildPluginManager", () => {
       await buildPluginManager.injectDebugIds(buildArtifactPaths);
 
       expect(mockCliExecute).toHaveBeenCalledWith(
-        ["sourcemaps", "inject", "/path/to/bundle"],
+        ["sourcemaps", "inject", "--ignore", "node_modules", "/path/to/bundle"],
         "rejectOnError"
       );
     });
diff --git a/packages/bundler-plugin-core/test/index.test.ts b/packages/bundler-plugin-core/test/index.test.ts
@@ -4,6 +4,7 @@ import {
   sentryUnpluginFactory,
   createRollupDebugIdInjectionHooks,
 } from "../src";
+import { containsOnlyImports } from "../src/utils";
 
 describe("getDebugIdSnippet", () => {
   it("returns the debugId injection snippet for a passed debugId", () => {
@@ -14,6 +15,137 @@ describe("getDebugIdSnippet", () => {
   });
 });
 
+describe("containsOnlyImports", () => {
+  describe("should return true (import-only code)", () => {
+    it.each([
+      ["empty string", ""],
+      ["whitespace only", "   \n\t  "],
+      ["side effect import with single quotes", "import './module.js';"],
+      ["side effect import with double quotes", 'import "./module.js";'],
+      ["side effect import with backticks", "import `./module.js`;"],
+      ["side effect import without semicolon", "import './module.js'"],
+      ["default import", "import foo from './module.js';"],
+      ["named import", "import { foo } from './module.js';"],
+      ["named import with alias", "import { foo as bar } from './module.js';"],
+      ["multiple named imports", "import { foo, bar, baz } from './module.js';"],
+      ["namespace import", "import * as utils from './utils.js';"],
+      ["default and named imports", "import React, { useState } from 'react';"],
+      ["re-export all", "export * from './module.js';"],
+      ["re-export named", "export { foo, bar } from './module.js';"],
+      ["re-export with alias", "export { foo as default } from './module.js';"],
+    ])("%s", (_, code) => {
+      expect(containsOnlyImports(code)).toBe(true);
+    });
+
+    it.each([
+      [
+        "multiple imports",
+        `
+import './polyfill.js';
+import { helper } from './utils.js';
+import config from './config.js';
+`,
+      ],
+      [
+        "imports with line comments",
+        `
+// This is a comment
+import './module.js';
+// Another comment
+`,
+      ],
+      [
+        "imports with block comments",
+        `
+/* Block comment */
+import './module.js';
+/* Multi
+   line
+   comment */
+`,
+      ],
+      ["'use strict' with imports", `"use strict";\nimport './module.js';`],
+      ["'use strict' with single quotes", `'use strict';\nimport './module.js';`],
+      [
+        "mixed imports, re-exports, and comments",
+        `
+"use strict";
+// Entry point facade
+import './polyfills.js';
+import { init } from './app.js';
+/* Re-export for external use */
+export * from './types.js';
+export { config } from './config.js';
+`,
+      ],
+    ])("%s", (_, code) => {
+      expect(containsOnlyImports(code)).toBe(true);
+    });
+  });
+
+  describe("should return false (contains substantial code)", () => {
+    it.each([
+      ["variable declaration", "const x = 1;"],
+      ["let declaration", "let y = 2;"],
+      ["var declaration", "var z = 3;"],
+      ["function declaration", "function foo() {}"],
+      ["arrow function", "const fn = () => {};"],
+      ["class declaration", "class MyClass {}"],
+      ["function call", "console.log('hello');"],
+      ["IIFE", "(function() {})();"],
+      ["expression statement", "1 + 1;"],
+      ["object literal", "({ foo: 'bar' });"],
+      ["export declaration (not re-export)", "export const foo = 1;"],
+      ["export default expression", "export default {};"],
+      ["export function", "export function foo() {}"],
+      ["minified bundle code", `import{a as e}from"./chunk.js";var t=function(){return e()};t();`],
+    ])("%s", (_, code) => {
+      expect(containsOnlyImports(code)).toBe(false);
+    });
+
+    // Multi-line code snippets
+    it.each([
+      [
+        "import followed by code",
+        `
+import { init } from './app.js';
+init();
+`,
+      ],
+      [
+        "import with variable declaration",
+        `
+import './module.js';
+const config = { debug: true };
+`,
+      ],
+      [
+        "import with function declaration",
+        `
+import { helper } from './utils.js';
+function main() {
+  helper();
+}
+`,
+      ],
+      [
+        "real-world SPA bundle snippet",
+        `
+import { createApp } from 'vue';
+import App from './App.vue';
+import router from './router';
+
+const app = createApp(App);
+app.use(router);
+app.mount('#app');
+`,
+      ],
+    ])("%s", (_, code) => {
+      expect(containsOnlyImports(code)).toBe(false);
+    });
+  });
+});
+
 describe("createRollupDebugIdInjectionHooks", () => {
   const hooks = createRollupDebugIdInjectionHooks();
 
@@ -99,6 +231,107 @@ describe("createRollupDebugIdInjectionHooks", () => {
         hooks.renderChunk(codeWithCommentBeyond500B, { fileName: "bundle.js" })
       ).not.toBeNull();
     });
+
+    describe("HTML facade chunks (MPA vs SPA)", () => {
+      // Issue #829: MPA facades should be skipped
+      // Regression fix: SPA main bundles with HTML facades should NOT be skipped
+
+      it.each([
+        ["empty", ""],
+        ["only side-effect imports", `import './shared-module.js';`],
+        ["only named imports", `import { foo, bar } from './shared-module.js';`],
+        ["only re-exports", `export * from './shared-module.js';`],
+        [
+          "multiple imports and comments",
+          `// This is a facade module
+import './moduleA.js';
+import { x } from './moduleB.js';
+/* block comment */
+export * from './moduleC.js';`,
+        ],
+        ["'use strict' and imports only", `"use strict";\nimport './shared-module.js';`],
+        ["query string in facadeModuleId", `import './shared.js';`, "?query=param"],
+        ["hash in facadeModuleId", `import './shared.js';`, "#hash"],
+      ])("should SKIP HTML facade chunks: %s", (_, code, suffix = "") => {
+        const result = hooks.renderChunk(code, {
+          fileName: "page1.js",
+          facadeModuleId: `/path/to/page1.html${suffix}`,
+        });
+        expect(result).toBeNull();
+      });
+
+      it("should inject into HTML facade with function declarations", () => {
+        const result = hooks.renderChunk(`function main() { console.log("hello"); }`, {
+          fileName: "index.js",
+          facadeModuleId: "/path/to/index.html",
+        });
+        expect(result).not.toBeNull();
+        expect(result?.code).toMatchInlineSnapshot(
+          `";{try{(function(){var e=\\"undefined\\"!=typeof window?window:\\"undefined\\"!=typeof global?global:\\"undefined\\"!=typeof globalThis?globalThis:\\"undefined\\"!=typeof self?self:{},n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]=\\"c4c89e04-3658-4874-b25b-07e638185091\\",e._sentryDebugIdIdentifier=\\"sentry-dbid-c4c89e04-3658-4874-b25b-07e638185091\\");})();}catch(e){}};function main() { console.log(\\"hello\\"); }"`
+        );
+      });
+
+      it("should inject into HTML facade with variable declarations", () => {
+        const result = hooks.renderChunk(`const x = 42;`, {
+          fileName: "index.js",
+          facadeModuleId: "/path/to/index.html",
+        });
+        expect(result).not.toBeNull();
+        expect(result?.code).toMatchInlineSnapshot(
+          `";{try{(function(){var e=\\"undefined\\"!=typeof window?window:\\"undefined\\"!=typeof global?global:\\"undefined\\"!=typeof globalThis?globalThis:\\"undefined\\"!=typeof self?self:{},n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]=\\"43e69766-1963-49f2-a291-ff8de60cc652\\",e._sentryDebugIdIdentifier=\\"sentry-dbid-43e69766-1963-49f2-a291-ff8de60cc652\\");})();}catch(e){}};const x = 42;"`
+        );
+      });
+
+      it("should inject into HTML facade with substantial code (SPA main bundle)", () => {
+        const code = `import { initApp } from './app.js';
+
+const config = { debug: true };
+
+function bootstrap() {
+  initApp(config);
+}
+
+bootstrap();`;
+        const result = hooks.renderChunk(code, {
+          fileName: "index.js",
+          facadeModuleId: "/path/to/index.html",
+        });
+        expect(result).not.toBeNull();
+        expect(result?.code).toMatchInlineSnapshot(`
+          ";{try{(function(){var e=\\"undefined\\"!=typeof window?window:\\"undefined\\"!=typeof global?global:\\"undefined\\"!=typeof globalThis?globalThis:\\"undefined\\"!=typeof self?self:{},n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]=\\"d0c4524b-496e-45a4-9852-7558d043ba3c\\",e._sentryDebugIdIdentifier=\\"sentry-dbid-d0c4524b-496e-45a4-9852-7558d043ba3c\\");})();}catch(e){}};import { initApp } from './app.js';
+
+          const config = { debug: true };
+
+          function bootstrap() {
+            initApp(config);
+          }
+
+          bootstrap();"
+        `);
+      });
+
+      it("should inject into HTML facade with mixed imports and code", () => {
+        const result = hooks.renderChunk(
+          `import './polyfills.js';\nimport { init } from './app.js';\n\ninit();`,
+          { fileName: "index.js", facadeModuleId: "/path/to/index.html" }
+        );
+        expect(result).not.toBeNull();
+        expect(result?.code).toMatchInlineSnapshot(`
+          ";{try{(function(){var e=\\"undefined\\"!=typeof window?window:\\"undefined\\"!=typeof global?global:\\"undefined\\"!=typeof globalThis?globalThis:\\"undefined\\"!=typeof self?self:{},n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]=\\"28f0bbaa-9aeb-40c4-98c9-4e44f1d4e175\\",e._sentryDebugIdIdentifier=\\"sentry-dbid-28f0bbaa-9aeb-40c4-98c9-4e44f1d4e175\\");})();}catch(e){}};import './polyfills.js';
+          import { init } from './app.js';
+
+          init();"
+        `);
+      });
+
+      it("should inject into regular JS chunks (no HTML facade)", () => {
+        const result = hooks.renderChunk(`console.log("Hello");`, { fileName: "bundle.js" });
+        expect(result).not.toBeNull();
+        expect(result?.code).toMatchInlineSnapshot(
+          `";{try{(function(){var e=\\"undefined\\"!=typeof window?window:\\"undefined\\"!=typeof global?global:\\"undefined\\"!=typeof globalThis?globalThis:\\"undefined\\"!=typeof self?self:{},n=(new e.Error).stack;n&&(e._sentryDebugIds=e._sentryDebugIds||{},e._sentryDebugIds[n]=\\"79f18a7f-ca16-4168-9797-906c82058367\\",e._sentryDebugIdIdentifier=\\"sentry-dbid-79f18a7f-ca16-4168-9797-906c82058367\\");})();}catch(e){}};console.log(\\"Hello\\");"`
+        );
+      });
+    });
   });
 });
 
diff --git a/packages/bundler-plugin-core/test/utils.test.ts b/packages/bundler-plugin-core/test/utils.test.ts
