Skip to content
Open
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
16 changes: 10 additions & 6 deletions packages/gatsby/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { sentryWebpackPlugin } = require('@sentry/bundler-plugins/webpack');

const SENTRY_USER_CONFIG = ['./sentry.config.js', './sentry.config.ts'];

exports.onCreateWebpackConfig = ({ getConfig, actions }, options) => {
exports.onCreateWebpackConfig = ({ getConfig, actions, stage }, options) => {
const enableClientWebpackPlugin = options.enableClientWebpackPlugin !== false;
if (process.env.NODE_ENV === 'production' && enableClientWebpackPlugin) {
const prevSourceMapSetting = getConfig() && 'devtool' in getConfig() ? getConfig().devtool : undefined;
Expand All @@ -18,10 +18,13 @@ exports.onCreateWebpackConfig = ({ getConfig, actions }, options) => {
);
}

// Delete source maps per default or when this is explicitly set to `true` (`deleteSourceMapsAfterUpload: true` can override the default behavior)
// Gatsby reads the `build-html` maps in `.cache` to map HTML render errors.
const deleteSourcemapsAfterUpload =
options.deleteSourcemapsAfterUpload ||
(options.deleteSourcemapsAfterUpload !== false && shouldAutomaticallyEnableSourceMaps);
stage === 'build-javascript' &&
(options.deleteSourcemapsAfterUpload ||
(options.deleteSourcemapsAfterUpload !== false && shouldAutomaticallyEnableSourceMaps))
? true
: undefined;

actions.setWebpackConfig({
devtool: shouldAutomaticallyEnableSourceMaps ? 'hidden-source-map' : prevSourceMapSetting,
Expand All @@ -30,8 +33,9 @@ exports.onCreateWebpackConfig = ({ getConfig, actions }, options) => {
sourcemaps: {
// Only include files from the build output directory
assets: ['./public/**'],
// Delete source files after uploading
filesToDeleteAfterUpload: deleteSourcemapsAfterUpload ? ['./public/**/*.map'] : undefined,
deleteSourcemapsAfterUpload,
// Also removes CSS maps and maps of chunks that are not uploaded.
sdkEnabledSourceMaps: deleteSourcemapsAfterUpload,
// Ignore files that aren't users' source code related
ignore: [
'polyfill-*', // related to polyfills
Expand Down
160 changes: 75 additions & 85 deletions packages/gatsby/test/gatsby-node.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest';
import { onCreateWebpackConfig } from '../gatsby-node';

vi.hoisted(
Expand Down Expand Up @@ -106,121 +106,111 @@ describe('onCreateWebpackConfig', () => {
});
});

describe('delete source maps after upload', () => {
beforeEach(() => {
vi.clearAllMocks();
});

const actions = {
setWebpackConfig: vi.fn(),
};

const getConfig = vi.fn();

it('sets sourceMapFilesToDeleteAfterUpload when provided in options', () => {
const actions = {
setWebpackConfig: vi.fn(),
};

const getConfig = vi.fn().mockReturnValue({ devtool: 'source-map' });

onCreateWebpackConfig({ actions, getConfig }, { deleteSourcemapsAfterUpload: true });

expect(actions.setWebpackConfig).toHaveBeenCalledTimes(1);

expect(sentryWebpackPlugin).toHaveBeenCalledWith(
expect.objectContaining({
sourcemaps: expect.objectContaining({
filesToDeleteAfterUpload: ['./public/**/*.map'],
}),
}),
);
});

describe('source maps', () => {
it.each([
{
name: 'without provided options: sets hidden source maps and deletes source maps',
name: 'build-javascript, devtool unset: enables hidden source maps and passes delete options',
stage: 'build-javascript',
initialConfig: undefined,
options: {},
expected: {
devtool: 'hidden-source-map',
deleteSourceMaps: true,
},
expected: { devtool: 'hidden-source-map', deleteOptions: true },
},
{
name: "preserves enabled source-map and doesn't delete",
initialConfig: { devtool: 'source-map' },
name: 'build-javascript, devtool false: enables hidden source maps and passes delete options',
stage: 'build-javascript',
initialConfig: { devtool: false },
options: {},
expected: {
devtool: 'source-map',
deleteSourceMaps: false,
},
expected: { devtool: 'hidden-source-map', deleteOptions: true },
},
{
name: "preserves enabled hidden-source-map and doesn't delete",
initialConfig: { devtool: 'hidden-source-map' },
name: 'build-javascript, devtool eval: enables hidden source maps and passes delete options',
stage: 'build-javascript',
initialConfig: { devtool: 'eval' },
options: {},
expected: {
devtool: 'hidden-source-map',
deleteSourceMaps: false,
},
expected: { devtool: 'hidden-source-map', deleteOptions: true },
},
{
name: 'deletes source maps, when user explicitly sets it',
initialConfig: { devtool: 'eval' },
name: 'build-javascript, source-map: keeps it and passes no delete options',
stage: 'build-javascript',
initialConfig: { devtool: 'source-map' },
options: {},
expected: {
devtool: 'hidden-source-map',
deleteSourceMaps: true,
},
expected: { devtool: 'source-map', deleteOptions: undefined },
},
{
name: 'replaces disabled source maps with hidden ones and deletes them',
initialConfig: { devtool: false },
name: 'build-javascript, hidden-source-map: keeps it and passes no delete options',
stage: 'build-javascript',
initialConfig: { devtool: 'hidden-source-map' },
options: {},
expected: {
devtool: 'hidden-source-map',
deleteSourceMaps: true,
},
expected: { devtool: 'hidden-source-map', deleteOptions: undefined },
},
{
name: 'explicit deleteSourcemapsAfterUpload true',
name: 'build-javascript, devtool false, user false: enables hidden source maps and passes no delete options',
stage: 'build-javascript',
initialConfig: { devtool: false },
options: { deleteSourcemapsAfterUpload: false },
expected: { devtool: 'hidden-source-map', deleteOptions: undefined },
},
{
name: 'build-javascript, source-map, user true: passes delete options',
stage: 'build-javascript',
initialConfig: { devtool: 'source-map' },
options: { deleteSourcemapsAfterUpload: true },
expected: {
devtool: 'source-map',
deleteSourceMaps: true,
},
expected: { devtool: 'source-map', deleteOptions: true },
},
{
name: 'explicit deleteSourcemapsAfterUpload false',
name: 'build-javascript, hidden-source-map, user true: passes delete options',
stage: 'build-javascript',
initialConfig: { devtool: 'hidden-source-map' },
options: { deleteSourcemapsAfterUpload: true },
expected: { devtool: 'hidden-source-map', deleteOptions: true },
},
{
name: 'build-javascript, source-map, user truthy value: passes delete options as true',
stage: 'build-javascript',
initialConfig: { devtool: 'source-map' },
options: { deleteSourcemapsAfterUpload: 'true' },
expected: { devtool: 'source-map', deleteOptions: true },
},
{
name: 'build-javascript, hidden-source-map, user false: passes no delete options',
stage: 'build-javascript',
initialConfig: { devtool: 'hidden-source-map' },
options: { deleteSourcemapsAfterUpload: false },
expected: {
devtool: 'hidden-source-map',
deleteSourceMaps: false,
},
expected: { devtool: 'hidden-source-map', deleteOptions: undefined },
},
{
name: 'build-html, devtool false: enables hidden source maps and passes no delete options',
stage: 'build-html',
initialConfig: { devtool: false },
options: {},
expected: { devtool: 'hidden-source-map', deleteOptions: undefined },
},
{
name: 'build-html, source-map, user true: passes no delete options',
stage: 'build-html',
initialConfig: { devtool: 'source-map' },
options: { deleteSourcemapsAfterUpload: true },
expected: { devtool: 'source-map', deleteOptions: undefined },
},
])('$name', ({ initialConfig, options, expected }) => {
getConfig.mockReturnValue(initialConfig);
])('$name', ({ stage, initialConfig, options, expected }) => {
const actions = { setWebpackConfig: vi.fn() };
const getConfig = vi.fn().mockReturnValue(initialConfig);

onCreateWebpackConfig({ actions: actions, getConfig: getConfig }, options);
onCreateWebpackConfig({ actions, getConfig, stage }, options);

expect(actions.setWebpackConfig).toHaveBeenCalledTimes(1);

expect(actions.setWebpackConfig).toHaveBeenCalledWith(
expect.objectContaining({
devtool: expected.devtool,
plugins: expect.arrayContaining([expect.any(Object)]),
}),
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Feat lacks integration or E2E test

Medium Severity

This was flagged because the review guidelines require feat PRs to include an integration or E2E test. Coverage here is only unit tests that assert plugin options, so they never confirm that a real Gatsby public/ build has neither leftover .map files nor sourceMappingURL comments after deletion.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit c91ce96. Configure here.

expect(actions.setWebpackConfig).toHaveBeenCalledWith({
devtool: expected.devtool,
plugins: [expect.any(Object)],
});
expect(sentryWebpackPlugin).toHaveBeenCalledWith(
expect.objectContaining({
sourcemaps: expect.objectContaining({
sourcemaps: {
assets: ['./public/**'],
filesToDeleteAfterUpload: expected.deleteSourceMaps ? ['./public/**/*.map'] : undefined,
}),
deleteSourcemapsAfterUpload: expected.deleteOptions,
sdkEnabledSourceMaps: expected.deleteOptions,
ignore: ['polyfill-*', 'framework-*', 'webpack-runtime-*'],
},
}),
);
});
Expand Down
Loading