From c819c7809eddd22054e27600024fe4efda8d24de Mon Sep 17 00:00:00 2001 From: Tom Shea Date: Wed, 28 Aug 2024 16:19:02 +0000 Subject: [PATCH 1/7] fix(webpack-plugin): Allow injection plugins to apply to files with query parameters in their name --- packages/webpack-plugin/src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/webpack-plugin/src/index.ts b/packages/webpack-plugin/src/index.ts index 39b62332..391fa02a 100644 --- a/packages/webpack-plugin/src/index.ts +++ b/packages/webpack-plugin/src/index.ts @@ -39,7 +39,7 @@ function webpackReleaseInjectionPlugin(injectionCode: string): UnpluginOptions { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?$/, banner: injectionCode, }) ); @@ -105,7 +105,7 @@ function webpackDebugIdInjectionPlugin(): UnpluginOptions { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?$/, banner: (arg?: BannerPluginCallbackArg) => { const debugId = arg?.chunk?.hash ? stringToUUID(arg.chunk.hash) : uuidv4(); return getDebugIdSnippet(debugId); @@ -156,7 +156,7 @@ function webpackModuleMetadataInjectionPlugin(injectionCode: string): UnpluginOp // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?$/, banner: injectionCode, }) ); From 721944e2697f92800ad2bffd9427761bab22293a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 29 Aug 2024 10:18:50 +0200 Subject: [PATCH 2/7] Add other bundlers, fragment and tests --- .../src/debug-id-upload.ts | 5 +- packages/bundler-plugin-core/src/index.ts | 19 +++- .../injection-with-query-param.test.ts | 71 ++++++++++++ .../input/bundle1.js | 8 ++ .../input/bundle2.js | 8 ++ .../injection-with-query-param/setup.ts | 16 +++ .../utils/create-cjs-bundles-with-query.ts | 105 ++++++++++++++++++ packages/webpack-plugin/src/index.ts | 6 +- 8 files changed, 228 insertions(+), 10 deletions(-) create mode 100644 packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts create mode 100644 packages/integration-tests/fixtures/injection-with-query-param/input/bundle1.js create mode 100644 packages/integration-tests/fixtures/injection-with-query-param/input/bundle2.js create mode 100644 packages/integration-tests/fixtures/injection-with-query-param/setup.ts create mode 100644 packages/integration-tests/utils/create-cjs-bundles-with-query.ts diff --git a/packages/bundler-plugin-core/src/debug-id-upload.ts b/packages/bundler-plugin-core/src/debug-id-upload.ts index 22f4b666..22b798b7 100644 --- a/packages/bundler-plugin-core/src/debug-id-upload.ts +++ b/packages/bundler-plugin-core/src/debug-id-upload.ts @@ -90,10 +90,7 @@ export function createDebugIdUploadFunction({ globSpan.finish(); const debugIdChunkFilePaths = globResult.filter( - (debugIdChunkFilePath) => - debugIdChunkFilePath.endsWith(".js") || - debugIdChunkFilePath.endsWith(".mjs") || - debugIdChunkFilePath.endsWith(".cjs") + (debugIdChunkFilePath) => !!debugIdChunkFilePath.match(/\.(js|mjs|cjs)(\?.*)?(#.*)?$/) ); // The order of the files output by glob() is not deterministic diff --git a/packages/bundler-plugin-core/src/index.ts b/packages/bundler-plugin-core/src/index.ts index 8c7ca78a..01716376 100644 --- a/packages/bundler-plugin-core/src/index.ts +++ b/packages/bundler-plugin-core/src/index.ts @@ -528,7 +528,10 @@ export function createRollupDebugIdInjectionHooks() { return { renderChunk(code: string, chunk: { fileName: string }) { if ( - [".js", ".mjs", ".cjs"].some((ending) => chunk.fileName.endsWith(ending)) // chunks could be any file (html, md, ...) + // chunks could be any file (html, md, ...) + [".js", ".mjs", ".cjs"].some((ending) => + stripQueryAndHashFromPath(chunk.fileName).endsWith(ending) + ) ) { const debugId = stringToUUID(code); // generate a deterministic debug ID const codeToInject = getDebugIdSnippet(debugId); @@ -562,7 +565,10 @@ export function createRollupModuleMetadataInjectionHooks(injectionCode: string) return { renderChunk(code: string, chunk: { fileName: string }) { if ( - [".js", ".mjs", ".cjs"].some((ending) => chunk.fileName.endsWith(ending)) // chunks could be any file (html, md, ...) + // chunks could be any file (html, md, ...) + [".js", ".mjs", ".cjs"].some((ending) => + stripQueryAndHashFromPath(chunk.fileName).endsWith(ending) + ) ) { const ms = new MagicString(code, { filename: chunk.fileName }); @@ -600,7 +606,14 @@ export function createRollupDebugIdUploadHooks( if (outputOptions.dir) { const outputDir = outputOptions.dir; const buildArtifacts = await glob( - ["/**/*.js", "/**/*.mjs", "/**/*.cjs", "/**/*.js.map", "/**/*.mjs.map", "/**/*.cjs.map"], + [ + "/**/*.js", + "/**/*.mjs", + "/**/*.cjs", + "/**/*.js.map", + "/**/*.mjs.map", + "/**/*.cjs.map", + ].map((q) => `${q}?(\\?*)?(#*)`), // We want to allow query and hashes strings at the end of files { root: outputDir, absolute: true, diff --git a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts new file mode 100644 index 00000000..91d785c9 --- /dev/null +++ b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts @@ -0,0 +1,71 @@ +/* eslint-disable jest/no-standalone-expect */ +/* eslint-disable jest/expect-expect */ +import childProcess from "child_process"; +import path from "path"; +import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf"; + +function checkBundleForDebugIds(bundlePath1: string, bundlePath2: string): string[] { + const process1Output = childProcess.execSync(`node ${bundlePath1}`, { encoding: "utf-8" }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const debugIdMap1 = JSON.parse(process1Output).debugIds as Record; + const debugIds1 = Object.values(debugIdMap1); + expect(debugIds1.length).toBeGreaterThan(0); + expect(debugIds1).toContainEqual( + expect.stringMatching(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/) + ); + + expect(Object.keys(debugIdMap1)[0]).toContain("Error"); + + const process2Output = childProcess.execSync(`node ${bundlePath2}`, { encoding: "utf-8" }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + const debugIdMap2 = JSON.parse(process2Output).debugIds as Record; + const debugIds2 = Object.values(debugIdMap2); + expect(debugIds2.length).toBeGreaterThan(0); + expect(debugIds2).toContainEqual( + expect.stringMatching(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/) + ); + + expect(Object.keys(debugIdMap2)[0]).toContain("Error"); + + expect(debugIds1).not.toEqual(debugIds2); + + return [...debugIds1, ...debugIds2]; +} + +function checkBundleForRelease(bundlePath: string): void { + const processOutput = childProcess.execSync(`node ${bundlePath}`, { encoding: "utf-8" }); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + expect(JSON.parse(processOutput).release).toBe("I AM A RELEASE!"); +} + +test("vite bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"), + path.join(__dirname, "out", "vite", "bundle2.js?foo=bar#baz") + ); + checkBundleForRelease(path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz")); +}); + +test("rollup bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz"), + path.join(__dirname, "out", "rollup", "bundle2.js?foo=bar#baz") + ); + checkBundleForRelease(path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz")); +}); + +testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "webpack4", "bundle1.js"), + path.join(__dirname, "out", "webpack4", "bundle2.js") + ); + checkBundleForRelease(path.join(__dirname, "out", "webpack4", "bundle1.js")); +}); + +test("webpack 5 bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "webpack5", "bundle1.js"), + path.join(__dirname, "out", "webpack5", "bundle2.js") + ); + checkBundleForRelease(path.join(__dirname, "out", "webpack5", "bundle1.js")); +}); diff --git a/packages/integration-tests/fixtures/injection-with-query-param/input/bundle1.js b/packages/integration-tests/fixtures/injection-with-query-param/input/bundle1.js new file mode 100644 index 00000000..beedb096 --- /dev/null +++ b/packages/integration-tests/fixtures/injection-with-query-param/input/bundle1.js @@ -0,0 +1,8 @@ +// eslint-disable-next-line no-console +console.log( + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access + JSON.stringify({ debugIds: global._sentryDebugIds, release: global.SENTRY_RELEASE.id }) +); + +// Just so the two bundles generate different hashes: +global.iAmBundle1 = 1; diff --git a/packages/integration-tests/fixtures/injection-with-query-param/input/bundle2.js b/packages/integration-tests/fixtures/injection-with-query-param/input/bundle2.js new file mode 100644 index 00000000..13ee683c --- /dev/null +++ b/packages/integration-tests/fixtures/injection-with-query-param/input/bundle2.js @@ -0,0 +1,8 @@ +// eslint-disable-next-line no-console +console.log( + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access + JSON.stringify({ debugIds: global._sentryDebugIds, release: global.SENTRY_RELEASE.id }) +); + +// Just so the two bundles generate different hashes: +global.iAmBundle2 = 2; diff --git a/packages/integration-tests/fixtures/injection-with-query-param/setup.ts b/packages/integration-tests/fixtures/injection-with-query-param/setup.ts new file mode 100644 index 00000000..5eeabafa --- /dev/null +++ b/packages/integration-tests/fixtures/injection-with-query-param/setup.ts @@ -0,0 +1,16 @@ +import * as path from "path"; +import { createCjsBundlesWithQueryParam } from "../../utils/create-cjs-bundles-with-query"; + +const outputDir = path.resolve(__dirname, "out"); + +createCjsBundlesWithQueryParam( + { + bundle1: path.resolve(__dirname, "input", "bundle1.js"), + bundle2: path.resolve(__dirname, "input", "bundle2.js"), + }, + outputDir, + { + telemetry: false, + release: { name: "I AM A RELEASE!", create: false }, + } +); diff --git a/packages/integration-tests/utils/create-cjs-bundles-with-query.ts b/packages/integration-tests/utils/create-cjs-bundles-with-query.ts new file mode 100644 index 00000000..b3c065ed --- /dev/null +++ b/packages/integration-tests/utils/create-cjs-bundles-with-query.ts @@ -0,0 +1,105 @@ +import * as vite from "vite"; +import * as path from "path"; +import * as rollup from "rollup"; +import { default as webpack4 } from "webpack4"; +import { webpack as webpack5 } from "webpack5"; +import { Options } from "@sentry/bundler-plugin-core"; +import { sentryVitePlugin } from "@sentry/vite-plugin"; +import { sentryWebpackPlugin } from "@sentry/webpack-plugin"; +import { sentryRollupPlugin } from "@sentry/rollup-plugin"; + +// eslint-disable-next-line @typescript-eslint/no-non-null-assertion +const nodejsMajorVersion = process.version.split(".")[0]!.slice(1); + +export function createCjsBundlesWithQueryParam( + entrypoints: { [name: string]: string }, + outFolder: string, + sentryUnpluginOptions: Options, + plugins: string[] = [] +): void { + if (plugins.length === 0 || plugins.includes("vite")) { + void vite.build({ + clearScreen: false, + build: { + sourcemap: true, + outDir: path.join(outFolder, "vite"), + rollupOptions: { + input: entrypoints, + output: { + format: "cjs", + entryFileNames: "[name].js?foo=bar#baz", + }, + }, + }, + plugins: [sentryVitePlugin(sentryUnpluginOptions)], + }); + } + if (plugins.length === 0 || plugins.includes("rollup")) { + void rollup + .rollup({ + input: entrypoints, + plugins: [sentryRollupPlugin(sentryUnpluginOptions)], + }) + .then((bundle) => + bundle.write({ + sourcemap: true, + dir: path.join(outFolder, "rollup"), + format: "cjs", + exports: "named", + entryFileNames: "[name].js?foo=bar#baz", + }) + ); + } + + if (plugins.length === 0 || plugins.includes("esbuild")) { + // esbuild doesn't have an option to add a query param + } + + // Webpack 4 doesn't work on Node.js versions >= 18 + if (parseInt(nodejsMajorVersion) < 18 && (plugins.length === 0 || plugins.includes("webpack4"))) { + webpack4( + { + devtool: "source-map", + mode: "production", + entry: entrypoints, + cache: false, + output: { + path: path.join(outFolder, "webpack4"), + filename: "[name].js?foo=bar#baz", // For some weird reason, the query param is not actually put to disk but the "virtual" behaviour we want to test still applies + libraryTarget: "commonjs", + }, + target: "node", // needed for webpack 4 so we can access node api + plugins: [sentryWebpackPlugin(sentryUnpluginOptions)], + }, + (err) => { + if (err) { + throw err; + } + } + ); + } + + if (plugins.length === 0 || plugins.includes("webpack5")) { + webpack5( + { + devtool: "source-map", + cache: false, + entry: entrypoints, + output: { + path: path.join(outFolder, "webpack5"), + filename: "[name].js?foo=bar#baz", // For some weird reason, the query param is not actually put to disk but the "virtual" behaviour we want to test still applies + library: { + type: "commonjs", + }, + }, + mode: "production", + plugins: [sentryWebpackPlugin(sentryUnpluginOptions)], + }, + (err) => { + if (err) { + throw err; + } + } + ); + } +} diff --git a/packages/webpack-plugin/src/index.ts b/packages/webpack-plugin/src/index.ts index 391fa02a..645e62ea 100644 --- a/packages/webpack-plugin/src/index.ts +++ b/packages/webpack-plugin/src/index.ts @@ -39,7 +39,7 @@ function webpackReleaseInjectionPlugin(injectionCode: string): UnpluginOptions { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?(#.*)?$/, banner: injectionCode, }) ); @@ -105,7 +105,7 @@ function webpackDebugIdInjectionPlugin(): UnpluginOptions { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?(#.*)?$/, banner: (arg?: BannerPluginCallbackArg) => { const debugId = arg?.chunk?.hash ? stringToUUID(arg.chunk.hash) : uuidv4(); return getDebugIdSnippet(debugId); @@ -156,7 +156,7 @@ function webpackModuleMetadataInjectionPlugin(injectionCode: string): UnpluginOp // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?(#.*)?$/, banner: injectionCode, }) ); From f1699bc649caaf39a166571c4de66daaa5c2c2ce Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 29 Aug 2024 10:29:19 +0200 Subject: [PATCH 3/7] fix for win --- .../injection-with-query-param.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts index 91d785c9..a3f8c3f4 100644 --- a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts +++ b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts @@ -2,7 +2,7 @@ /* eslint-disable jest/expect-expect */ import childProcess from "child_process"; import path from "path"; -import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf"; +import { testIf, testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf"; function checkBundleForDebugIds(bundlePath1: string, bundlePath2: string): string[] { const process1Output = childProcess.execSync(`node ${bundlePath1}`, { encoding: "utf-8" }); @@ -38,7 +38,10 @@ function checkBundleForRelease(bundlePath: string): void { expect(JSON.parse(processOutput).release).toBe("I AM A RELEASE!"); } -test("vite bundle", () => { +testIf( + // query params and fragments are weird on windows + process.platform !== "win32" +)("vite bundle", () => { checkBundleForDebugIds( path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"), path.join(__dirname, "out", "vite", "bundle2.js?foo=bar#baz") @@ -46,7 +49,10 @@ test("vite bundle", () => { checkBundleForRelease(path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz")); }); -test("rollup bundle", () => { +testIf( + // query params and fragments are weird on windows + process.platform !== "win32" +)("rollup bundle", () => { checkBundleForDebugIds( path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz"), path.join(__dirname, "out", "rollup", "bundle2.js?foo=bar#baz") From b6baee5d2d167788a5da2c153c1ab17c6c4e6418 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 29 Aug 2024 10:38:19 +0200 Subject: [PATCH 4/7] fix for win? --- .../injection-with-query-param.test.ts | 67 +++++++++---------- .../injection-with-query-param/setup.ts | 28 ++++---- 2 files changed, 47 insertions(+), 48 deletions(-) diff --git a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts index a3f8c3f4..e96499f1 100644 --- a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts +++ b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts @@ -2,7 +2,7 @@ /* eslint-disable jest/expect-expect */ import childProcess from "child_process"; import path from "path"; -import { testIf, testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf"; +import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf"; function checkBundleForDebugIds(bundlePath1: string, bundlePath2: string): string[] { const process1Output = childProcess.execSync(`node ${bundlePath1}`, { encoding: "utf-8" }); @@ -38,40 +38,37 @@ function checkBundleForRelease(bundlePath: string): void { expect(JSON.parse(processOutput).release).toBe("I AM A RELEASE!"); } -testIf( - // query params and fragments are weird on windows - process.platform !== "win32" -)("vite bundle", () => { - checkBundleForDebugIds( - path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"), - path.join(__dirname, "out", "vite", "bundle2.js?foo=bar#baz") - ); - checkBundleForRelease(path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz")); -}); +// Query params and hashes are weird on windows +if (process.platform !== "win32") { + test("vite bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"), + path.join(__dirname, "out", "vite", "bundle2.js?foo=bar#baz") + ); + checkBundleForRelease(path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz")); + }); -testIf( - // query params and fragments are weird on windows - process.platform !== "win32" -)("rollup bundle", () => { - checkBundleForDebugIds( - path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz"), - path.join(__dirname, "out", "rollup", "bundle2.js?foo=bar#baz") - ); - checkBundleForRelease(path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz")); -}); + test("rollup bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz"), + path.join(__dirname, "out", "rollup", "bundle2.js?foo=bar#baz") + ); + checkBundleForRelease(path.join(__dirname, "out", "rollup", "bundle1.js?foo=bar#baz")); + }); -testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { - checkBundleForDebugIds( - path.join(__dirname, "out", "webpack4", "bundle1.js"), - path.join(__dirname, "out", "webpack4", "bundle2.js") - ); - checkBundleForRelease(path.join(__dirname, "out", "webpack4", "bundle1.js")); -}); + testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "webpack4", "bundle1.js"), + path.join(__dirname, "out", "webpack4", "bundle2.js") + ); + checkBundleForRelease(path.join(__dirname, "out", "webpack4", "bundle1.js")); + }); -test("webpack 5 bundle", () => { - checkBundleForDebugIds( - path.join(__dirname, "out", "webpack5", "bundle1.js"), - path.join(__dirname, "out", "webpack5", "bundle2.js") - ); - checkBundleForRelease(path.join(__dirname, "out", "webpack5", "bundle1.js")); -}); + test("webpack 5 bundle", () => { + checkBundleForDebugIds( + path.join(__dirname, "out", "webpack5", "bundle1.js"), + path.join(__dirname, "out", "webpack5", "bundle2.js") + ); + checkBundleForRelease(path.join(__dirname, "out", "webpack5", "bundle1.js")); + }); +} diff --git a/packages/integration-tests/fixtures/injection-with-query-param/setup.ts b/packages/integration-tests/fixtures/injection-with-query-param/setup.ts index 5eeabafa..681d4b23 100644 --- a/packages/integration-tests/fixtures/injection-with-query-param/setup.ts +++ b/packages/integration-tests/fixtures/injection-with-query-param/setup.ts @@ -1,16 +1,18 @@ import * as path from "path"; import { createCjsBundlesWithQueryParam } from "../../utils/create-cjs-bundles-with-query"; -const outputDir = path.resolve(__dirname, "out"); - -createCjsBundlesWithQueryParam( - { - bundle1: path.resolve(__dirname, "input", "bundle1.js"), - bundle2: path.resolve(__dirname, "input", "bundle2.js"), - }, - outputDir, - { - telemetry: false, - release: { name: "I AM A RELEASE!", create: false }, - } -); +// Query params and hashes are weird on windows +if (process.platform !== "win32") { + const outputDir = path.resolve(__dirname, "out"); + createCjsBundlesWithQueryParam( + { + bundle1: path.resolve(__dirname, "input", "bundle1.js"), + bundle2: path.resolve(__dirname, "input", "bundle2.js"), + }, + outputDir, + { + telemetry: false, + release: { name: "I AM A RELEASE!", create: false }, + } + ); +} From 36c40672aa4bc34ef4bb11f1dee6b280891dcad1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 29 Aug 2024 11:44:46 +0200 Subject: [PATCH 5/7] god this is cumbersome --- .../injection-with-query-param.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts index e96499f1..e87603cd 100644 --- a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts +++ b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts @@ -39,7 +39,7 @@ function checkBundleForRelease(bundlePath: string): void { } // Query params and hashes are weird on windows -if (process.platform !== "win32") { +(process.platform === "win32" ? describe : describe.skip)("Injection with query params", () => { test("vite bundle", () => { checkBundleForDebugIds( path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"), @@ -71,4 +71,4 @@ if (process.platform !== "win32") { ); checkBundleForRelease(path.join(__dirname, "out", "webpack5", "bundle1.js")); }); -} +}); From e0734cd4ab6e350f7bab1a27c236e98539decb20 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 29 Aug 2024 11:45:35 +0200 Subject: [PATCH 6/7] ok that one is on me --- .../injection-with-query-param.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts index e87603cd..41d97bfe 100644 --- a/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts +++ b/packages/integration-tests/fixtures/injection-with-query-param/injection-with-query-param.test.ts @@ -39,7 +39,7 @@ function checkBundleForRelease(bundlePath: string): void { } // Query params and hashes are weird on windows -(process.platform === "win32" ? describe : describe.skip)("Injection with query params", () => { +(process.platform === "win32" ? describe.skip : describe)("Injection with query params", () => { test("vite bundle", () => { checkBundleForDebugIds( path.join(__dirname, "out", "vite", "bundle1.js?foo=bar#baz"), From 0eb39703196a5d7bf428fd45041ef1c5913cdc68 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 29 Aug 2024 14:33:21 +0200 Subject: [PATCH 7/7] Fix redos --- packages/bundler-plugin-core/src/debug-id-upload.ts | 7 ++++--- packages/webpack-plugin/src/index.ts | 6 +++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/bundler-plugin-core/src/debug-id-upload.ts b/packages/bundler-plugin-core/src/debug-id-upload.ts index 22b798b7..3dd846e5 100644 --- a/packages/bundler-plugin-core/src/debug-id-upload.ts +++ b/packages/bundler-plugin-core/src/debug-id-upload.ts @@ -9,6 +9,7 @@ import { Hub, NodeClient } from "@sentry/node"; import SentryCli from "@sentry/cli"; import { dynamicSamplingContextToSentryBaggageHeader } from "@sentry/utils"; import { safeFlushTelemetry } from "./sentry/telemetry"; +import { stripQueryAndHashFromPath } from "./utils"; interface RewriteSourcesHook { (source: string, map: any): string; @@ -89,9 +90,9 @@ export function createDebugIdUploadFunction({ }); globSpan.finish(); - const debugIdChunkFilePaths = globResult.filter( - (debugIdChunkFilePath) => !!debugIdChunkFilePath.match(/\.(js|mjs|cjs)(\?.*)?(#.*)?$/) - ); + const debugIdChunkFilePaths = globResult.filter((debugIdChunkFilePath) => { + return !!stripQueryAndHashFromPath(debugIdChunkFilePath).match(/\.(js|mjs|cjs)$/); + }); // The order of the files output by glob() is not deterministic // Ensure order within the files so that {debug-id}-{chunkIndex} coupling is consistent diff --git a/packages/webpack-plugin/src/index.ts b/packages/webpack-plugin/src/index.ts index 645e62ea..66790c34 100644 --- a/packages/webpack-plugin/src/index.ts +++ b/packages/webpack-plugin/src/index.ts @@ -39,7 +39,7 @@ function webpackReleaseInjectionPlugin(injectionCode: string): UnpluginOptions { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?(#.*)?$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?[^?]*)?(#[^#]*)?$/, banner: injectionCode, }) ); @@ -105,7 +105,7 @@ function webpackDebugIdInjectionPlugin(): UnpluginOptions { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?(#.*)?$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?[^?]*)?(#[^#]*)?$/, banner: (arg?: BannerPluginCallbackArg) => { const debugId = arg?.chunk?.hash ? stringToUUID(arg.chunk.hash) : uuidv4(); return getDebugIdSnippet(debugId); @@ -156,7 +156,7 @@ function webpackModuleMetadataInjectionPlugin(injectionCode: string): UnpluginOp // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call new BannerPlugin({ raw: true, - include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?.*)?(#.*)?$/, + include: /\.(js|ts|jsx|tsx|mjs|cjs)(\?[^?]*)?(#[^#]*)?$/, banner: injectionCode, }) );