Skip to content

feat(nextjs): Add options to disable webpack plugin #3771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
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: 4 additions & 0 deletions packages/nextjs/src/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ export type ExportedNextConfig = NextConfigObject | NextConfigFunction;
export type NextConfigObject = {
// custom webpack options
webpack?: WebpackConfigFunction;
sentry?: {
disableServerWebpackPlugin?: boolean;
disableClientWebpackPlugin?: boolean;
};
} & {
// other `next.config.js` options
[key: string]: unknown;
Expand Down
53 changes: 30 additions & 23 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,6 @@ export function constructWebpackConfigFunction(
newConfig = userNextConfig.webpack(newConfig, options);
}

// Ensure quality source maps in production. (Source maps aren't uploaded in dev, and besides, Next doesn't let you
// change this is dev even if you want to - see
// https://github.com/vercel/next.js/blob/master/errors/improper-devtool.md.)
if (!options.dev) {
// TODO Handle possibility that user is using `SourceMapDevToolPlugin` (see
// https://webpack.js.org/plugins/source-map-dev-tool-plugin/)
// TODO Give user option to use `hidden-source-map` ?
newConfig.devtool = 'source-map';
}

// Tell webpack to inject user config files (containing the two `Sentry.init()` calls) into the appropriate output
// bundles. Store a separate reference to the original `entry` value to avoid an infinite loop. (If we don't do
// this, we'll have a statement of the form `x.y = () => f(x.y)`, where one of the things `f` does is call `x.y`.
Expand All @@ -90,19 +80,36 @@ export function constructWebpackConfigFunction(
const origEntryProperty = newConfig.entry;
newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, options.isServer);

// Add the Sentry plugin, which uploads source maps to Sentry when not in dev
checkWebpackPluginOverrides(userSentryWebpackPluginOptions);
newConfig.plugins = newConfig.plugins || [];
newConfig.plugins.push(
// @ts-ignore Our types for the plugin are messed up somehow - TS wants this to be `SentryWebpackPlugin.default`,
// but that's not actually a thing
new SentryWebpackPlugin({
dryRun: options.dev,
release: getSentryRelease(options.buildId),
...defaultSentryWebpackPluginOptions,
...userSentryWebpackPluginOptions,
}),
);
// Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default
const enableWebpackPlugin = options.isServer
? !userNextConfig.sentry?.disableServerWebpackPlugin
: !userNextConfig.sentry?.disableClientWebpackPlugin;

if (enableWebpackPlugin) {
// TODO Handle possibility that user is using `SourceMapDevToolPlugin` (see
// https://webpack.js.org/plugins/source-map-dev-tool-plugin/)
// TODO Give user option to use `hidden-source-map` ?

// Next doesn't let you change this is dev even if you want to - see
// https://github.com/vercel/next.js/blob/master/errors/improper-devtool.md
if (!options.dev) {
newConfig.devtool = 'source-map';
}

checkWebpackPluginOverrides(userSentryWebpackPluginOptions);

newConfig.plugins = newConfig.plugins || [];
newConfig.plugins.push(
// @ts-ignore Our types for the plugin are messed up somehow - TS wants this to be `SentryWebpackPlugin.default`,
// but that's not actually a thing
new SentryWebpackPlugin({
dryRun: options.dev,
release: getSentryRelease(options.buildId),
...defaultSentryWebpackPluginOptions,
...userSentryWebpackPluginOptions,
}),
);
}

return newConfig;
};
Expand Down
39 changes: 39 additions & 0 deletions packages/nextjs/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,4 +275,43 @@ describe('Sentry webpack plugin config', () => {
it("merges default include and ignore/ignoreFile options with user's values", () => {
// do we even want to do this?
});

it('allows SentryWebpackPlugin to be turned off for client code (independent of server code)', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Taking some time to think about this, the tests for constructWebpackConfigFunction should just be a table test, where we pass in different permutations of the config and assert on the return value. We can do this in another PR though.

const clientFinalNextConfig = materializeFinalNextConfig({
...userNextConfig,
sentry: { disableClientWebpackPlugin: true },
});
const clientFinalWebpackConfig = clientFinalNextConfig.webpack?.(clientWebpackConfig, clientBuildContext);

const serverFinalNextConfig = materializeFinalNextConfig(userNextConfig, userSentryWebpackPluginConfig);
const serverFinalWebpackConfig = serverFinalNextConfig.webpack?.(serverWebpackConfig, serverBuildContext);

expect(clientFinalWebpackConfig?.plugins).not.toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)]));
expect(serverFinalWebpackConfig?.plugins).toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)]));
});

it('allows SentryWebpackPlugin to be turned off for server code (independent of client code)', () => {
const serverFinalNextConfig = materializeFinalNextConfig({
...userNextConfig,
sentry: { disableServerWebpackPlugin: true },
});
const serverFinalWebpackConfig = serverFinalNextConfig.webpack?.(serverWebpackConfig, serverBuildContext);

const clientFinalNextConfig = materializeFinalNextConfig(userNextConfig, userSentryWebpackPluginConfig);
const clientFinalWebpackConfig = clientFinalNextConfig.webpack?.(clientWebpackConfig, clientBuildContext);

expect(serverFinalWebpackConfig?.plugins).not.toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)]));
expect(clientFinalWebpackConfig?.plugins).toEqual(expect.arrayContaining([expect.any(SentryWebpackPlugin)]));
});

it("doesn't set devtool if webpack plugin is disabled", () => {
const finalNextConfig = materializeFinalNextConfig({
...userNextConfig,
webpack: () => ({ devtool: 'something-besides-source-map' } as any),
sentry: { disableServerWebpackPlugin: true },
});
const finalWebpackConfig = finalNextConfig.webpack?.(serverWebpackConfig, serverBuildContext);

expect(finalWebpackConfig?.devtool).not.toEqual('source-map');
});
});