Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .github/dependency-review-config.yml

This file was deleted.

12 changes: 11 additions & 1 deletion .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,17 @@ jobs:
strategy:
fail-fast: false
matrix:
node-version: ["18", "20", "22"]
node-version: [
# nx uses a `yargs-parser` versision which isn't compatible with node 10
# "10.24.1",
# vite uses optional chaining which isn't compatible with node 12
# "12.22.12",
"14",
"16",
"18",
"20",
"22",
]
os: [ubuntu-latest, windows-latest]
runs-on: ${{ matrix.os }}
steps:
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

# Sentry Bundler Plugins

Sentry plugins for various JavaScript bundlers. Currently supporting Rollup, Vite, esbuild, Webpack 5.
Sentry plugins for various JavaScript bundlers. Currently supporting Rollup, Vite, esbuild, Webpack 4 and Webpack 5.

Check out the individual packages for more information and examples:

Expand Down
4 changes: 3 additions & 1 deletion packages/e2e-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"@types/axios": "^0.14.0",
"@types/glob": "8.0.0",
"@types/jest": "^28.1.3",
"@types/webpack4": "npm:@types/webpack@^4",
"esbuild": "0.14.49",
"eslint": "^8.18.0",
"glob": "8.0.3",
Expand All @@ -36,7 +37,8 @@
"rollup": "3.2.0",
"ts-node": "^10.9.1",
"vite": "3.0.0",
"webpack": "5.74.0"
"webpack4": "npm:webpack@^4",
"webpack5": "npm:webpack@5.74.0"
},
"volta": {
"extends": "../../package.json"
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/utils/bundlers.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export const BUNDLERS = ["rollup", "vite", "esbuild", "webpack"];
export const BUNDLERS = ["rollup", "vite", "esbuild", "webpack4", "webpack5"];
41 changes: 36 additions & 5 deletions packages/e2e-tests/utils/create-cjs-bundles.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as vite from "vite";
import * as path from "path";
import * as rollup from "rollup";
import { webpack } from "webpack";
import { default as webpack4 } from "webpack4";
import { webpack as webpack5 } from "webpack5";
import * as esbuild from "esbuild";

import type { Options } from "@sentry/bundler-plugin-core";
Expand Down Expand Up @@ -90,13 +91,43 @@ export function createCjsBundles(
format: "cjs",
});

webpack(
webpack4(
{
devtool: "source-map",
mode: "production",
entry: entrypoints,
cache: false,
output: {
path: path.join(outFolder, "webpack4"),
libraryTarget: "commonjs",
},
target: "node", // needed for webpack 4 so we can access node api
plugins: [
sentryWebpackPlugin({
...sentryPluginOptions,
release: {
name: `${sentryPluginOptions.release.name!}-webpack4`,
uploadLegacySourcemaps: `${
sentryPluginOptions.release.uploadLegacySourcemaps as string
}/webpack4`,
},
}),
],
},
(err) => {
if (err) {
throw err;
}
}
);
Comment on lines +112 to +122
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The e2e test utility create-cjs-bundles.ts unconditionally runs webpack4, which will fail on Node.js 18+ when the currently disabled e2e tests are re-enabled.
Severity: MEDIUM

Suggested Fix

Add a Node.js version check in packages/e2e-tests/utils/create-cjs-bundles.ts to skip the webpack4 build process when the major version is 18 or greater. This will align its behavior with similar scripts like create-cjs-bundles-for-react.ts.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/e2e-tests/utils/create-cjs-bundles.ts#L91-L122

Potential issue: The script at `packages/e2e-tests/utils/create-cjs-bundles.ts`
unconditionally executes a webpack4 build. This is problematic because webpack4 is
incompatible with Node.js versions 18 and higher, causing build failures. While the e2e
tests are currently disabled in the CI pipeline, this latent bug will manifest as soon
as they are re-enabled in an environment running a modern Node.js version. Other similar
utility scripts in the integration tests already contain guards to prevent this exact
issue by checking the Node.js version before attempting to run webpack4.

Did we get this right? 👍 / 👎 to inform future reviews.


webpack5(
{
devtool: "source-map",
cache: false,
entry: entrypoints,
output: {
path: path.join(outFolder, "webpack"),
path: path.join(outFolder, "webpack5"),
library: {
type: "commonjs",
},
Expand All @@ -106,10 +137,10 @@ export function createCjsBundles(
sentryWebpackPlugin({
...sentryPluginOptions,
release: {
name: `${sentryPluginOptions.release.name!}-webpack`,
name: `${sentryPluginOptions.release.name!}-webpack5`,
uploadLegacySourcemaps: `${
sentryPluginOptions.release.uploadLegacySourcemaps as string
}/webpack`,
}/webpack5`,
},
}),
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@
/* eslint-disable jest/expect-expect */
import path from "path";
import fs from "fs";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

describe("Deletes files with `filesToDeleteAfterUpload` set to a promise", () => {
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
expect(fs.existsSync(path.join(__dirname, "out", "webpack4", "bundle.js.map"))).toBe(false);
});

test("webpack 5 bundle", () => {
expect(fs.existsSync(path.join(__dirname, "out", "webpack", "bundle.js.map"))).toBe(false);
expect(fs.existsSync(path.join(__dirname, "out", "webpack5", "bundle.js.map"))).toBe(false);
});

test("esbuild bundle", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { createCjsBundles } from "../../utils/create-cjs-bundles";

const outputDir = path.resolve(__dirname, "out");

["webpack", "esbuild", "rollup", "vite"].forEach((bundler) => {
["webpack4", "webpack5", "esbuild", "rollup", "vite"].forEach((bundler) => {
const fileDeletionGlobPromise = new Promise<string[]>((resolve) => {
setTimeout(() => {
resolve([path.join(__dirname, "out", bundler, "bundle.js.map")]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@
/* eslint-disable jest/expect-expect */
import path from "path";
import fs from "fs";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

describe("Deletes with `filesToDeleteAfterUpload` even without uploading anything", () => {
test("webpack bundle", () => {
expect(fs.existsSync(path.join(__dirname, "out", "webpack", "bundle.js.map"))).toBe(false);
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
expect(fs.existsSync(path.join(__dirname, "out", "webpack4", "bundle.js.map"))).toBe(false);
});

test("webpack 5 bundle", () => {
expect(fs.existsSync(path.join(__dirname, "out", "webpack5", "bundle.js.map"))).toBe(false);
});

test("esbuild bundle", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { createCjsBundles } from "../../utils/create-cjs-bundles";

const outputDir = path.resolve(__dirname, "out");

["webpack", "esbuild", "rollup", "vite"].forEach((bundler) => {
["webpack4", "webpack5", "esbuild", "rollup", "vite"].forEach((bundler) => {
createCjsBundles(
{
bundle: path.resolve(__dirname, "input", "bundle.js"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* eslint-disable jest/expect-expect */
import { execSync } from "child_process";
import path from "path";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

function checkBundle(bundlePath: string): void {
const output = execSync(`node ${bundlePath}`, { encoding: "utf-8" });
Expand All @@ -15,8 +16,12 @@ function checkBundle(bundlePath: string): void {
}

describe("appKey injection", () => {
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
checkBundle(path.join(__dirname, "out", "webpack4", "bundle.js"));
});

test("webpack 5 bundle", () => {
checkBundle(path.join(__dirname, "out", "webpack", "bundle.js"));
checkBundle(path.join(__dirname, "out", "webpack5", "bundle.js"));
});

test("esbuild bundle", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ createCjsBundles(
{
applicationKey: "my-app",
},
["webpack", "esbuild", "rollup", "vite"]
["webpack4", "webpack5", "esbuild", "rollup", "vite"]
);
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* eslint-disable jest/expect-expect */
import { execSync } from "child_process";
import path from "path";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

interface BundleOutput {
debugIds: Record<string, string> | undefined;
Expand Down Expand Up @@ -34,8 +35,12 @@ function checkBundle(bundlePath: string): void {
}

describe("applicationKey with debug ID injection", () => {
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
checkBundle(path.join(__dirname, "out", "webpack4", "bundle.js"));
});

test("webpack 5 bundle", () => {
checkBundle(path.join(__dirname, "out", "webpack", "bundle.js"));
checkBundle(path.join(__dirname, "out", "webpack5", "bundle.js"));
});

test("esbuild bundle", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ createCjsBundles(
telemetry: false,
release: { name: "test-release", create: false },
},
["webpack", "esbuild", "rollup", "vite"]
["webpack4", "webpack5", "esbuild", "rollup", "vite"]
);
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import childProcess from "child_process";
import path from "path";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

/**
* Runs a node file in a seprate process.
Expand Down Expand Up @@ -27,7 +28,12 @@ test("vite bundle", () => {
checkBundle(path.join(__dirname, "./out/vite/index.js"));
});

testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "./out/webpack4/index.js"));
});

test("webpack 5 bundle", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "./out/webpack/index.js"));
checkBundle(path.join(__dirname, "./out/webpack5/index.js"));
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import childProcess from "child_process";
import path from "path";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

/**
* Runs a node file in a seprate process.
Expand All @@ -14,8 +15,14 @@ function checkBundle(bundlePath: string): void {

expect(JSON.parse(processOutput)).toEqual(
expect.objectContaining({
deps: expect.arrayContaining<string>(["esbuild", "rollup", "vite", "webpack"]) as string[],
depsVersions: { rollup: 3, vite: 3, react: 18, webpack: 5 },
deps: expect.arrayContaining<string>([
"esbuild",
"rollup",
"vite",
"webpack4",
"webpack5",
]) as string[],
depsVersions: { rollup: 3, vite: 3, react: 18 },
// This will differ based on what env this is run on
nodeVersion: expectedNodeVersion,
})
Expand All @@ -37,7 +44,12 @@ test("vite bundle", () => {
checkBundle(path.join(__dirname, "out", "vite", "index.js"));
});

test("webpack bundle", () => {
testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "out", "webpack", "index.js"));
checkBundle(path.join(__dirname, "out", "webpack4", "index.js"));
});

test("webpack 5 bundle", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "out", "webpack5", "index.js"));
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* eslint-disable jest/expect-expect */
import path from "path";
import fs from "fs";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

const expectedOutputs: Record<string, Record<string, string>> = {
esbuild: {
Expand All @@ -23,7 +24,11 @@ const expectedOutputs: Record<string, Record<string, string>> = {
"bundle1.js": `console.log(1);`,
"bundle2.js": `console.log({debug:"b",trace:"b",replayCanvas:"a",replayIframe:"a",replayShadowDom:"a",replayWorker:"a"})`,
},
webpack: {
webpack4: {
"bundle1.js": `console.log(1)`,
"bundle2.js": `console.log({debug:"b",trace:"b",replayCanvas:"a",replayIframe:"a",replayShadowDom:"a",replayWorker:"a"})`,
},
webpack5: {
"bundle1.js": `console.log(1)`,
"bundle2.js": `console.log({debug:"b",trace:"b",replayCanvas:"a",replayIframe:"a",replayShadowDom:"a",replayWorker:"a"})`,
},
Expand Down Expand Up @@ -54,7 +59,12 @@ test("vite bundle", () => {
checkBundle("vite", "bundle2.js");
});

testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
checkBundle("webpack4", "bundle1.js");
checkBundle("webpack4", "bundle2.js");
});

test("webpack 5 bundle", () => {
checkBundle("webpack", "bundle1.js");
checkBundle("webpack", "bundle2.js");
checkBundle("webpack5", "bundle1.js");
checkBundle("webpack5", "bundle2.js");
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import childProcess from "child_process";
import path from "path";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

const SNAPSHOT = `"<div><span data-sentry-component=\\"ComponentA\\">Component A</span></div>"`;
const ESBUILD_SNAPSHOT = `"<div><span>Component A</span></div>"`;
Expand All @@ -24,7 +25,12 @@ test("vite bundle", () => {
checkBundle(path.join(__dirname, "./out/vite/index.js"));
});

testIfNodeMajorVersionIsLessThan18("webpack 4 bundle if node is < 18", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "./out/webpack4/index.js"));
});

test("webpack 5 bundle", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "./out/webpack/index.js"));
checkBundle(path.join(__dirname, "./out/webpack5/index.js"));
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import childProcess from "child_process";
import path from "path";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

const SNAPSHOT = `"<div><span data-sentry-component=\\"ComponentA\\" data-sentry-source-file=\\"component-a.jsx\\">Component A</span></div>"`;
const ESBUILD_SNAPSHOT = `"<div><span>Component A</span></div>"`;
Expand All @@ -24,7 +25,12 @@ test("vite bundle", () => {
checkBundle(path.join(__dirname, "./out/vite/index.js"));
});

testIfNodeMajorVersionIsLessThan18("webpack 4 bundle if node is < 18", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "./out/webpack4/index.js"));
});

test("webpack 5 bundle", () => {
expect.assertions(1);
checkBundle(path.join(__dirname, "./out/webpack/index.js"));
checkBundle(path.join(__dirname, "./out/webpack5/index.js"));
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* eslint-disable jest/expect-expect */
import childProcess from "child_process";
import path from "path";
import { testIfNodeMajorVersionIsLessThan18 } from "../../utils/testIf";

function checkBundle(bundlePath1: string, bundlePath2: string): string[] {
const process1Output = childProcess.execSync(`node ${bundlePath1}`, { encoding: "utf-8" });
Expand Down Expand Up @@ -50,9 +51,16 @@ test("vite bundle", () => {
);
});

testIfNodeMajorVersionIsLessThan18("webpack 4 bundle", () => {
checkBundle(
path.join(__dirname, "out", "webpack4", "bundle1.js"),
path.join(__dirname, "out", "webpack4", "bundle2.js")
);
});

test("webpack 5 bundle", () => {
checkBundle(
path.join(__dirname, "out", "webpack", "bundle1.js"),
path.join(__dirname, "out", "webpack", "bundle2.js")
path.join(__dirname, "out", "webpack5", "bundle1.js"),
path.join(__dirname, "out", "webpack5", "bundle2.js")
);
});
Loading
Loading