Skip to content

fix(nextjs): Correctly handle functional next config in withSentryConfig #3698

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.

Sign up for GitHub

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 1 commit into from
Jun 17, 2021
Merged

fix(nextjs): Correctly handle functional next config in withSentryConfig #3698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 15 additions & 7 deletions packages/nextjs/src/config/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ExportedNextConfig, NextConfigObject, SentryWebpackPluginOptions } from './types';
import { ExportedNextConfig, NextConfigFunction, NextConfigObject, SentryWebpackPluginOptions } from './types';
import { constructWebpackConfigFunction } from './webpack';

/**
Expand All @@ -11,14 +11,22 @@ import { constructWebpackConfigFunction } from './webpack';
export function withSentryConfig(
userNextConfig: ExportedNextConfig = {},
userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions> = {},
): NextConfigObject {
): NextConfigFunction {
const newWebpackExport = constructWebpackConfigFunction(userNextConfig, userSentryWebpackPluginOptions);

const finalNextConfig = {
...userNextConfig,
// TODO When we add a way to disable the webpack plugin, doing so should turn this off, too
productionBrowserSourceMaps: true,
webpack: newWebpackExport,
const finalNextConfig = (
phase: string,
defaults: { defaultConfig: { [key: string]: unknown } },
): NextConfigObject => {
const materializedUserNextConfig =
typeof userNextConfig === 'function' ? userNextConfig(phase, defaults) : userNextConfig;

return {
...materializedUserNextConfig,
// TODO When we add a way to disable the webpack plugin, doing so should turn this off, too
productionBrowserSourceMaps: true,
webpack: newWebpackExport,
};
};

return finalNextConfig;
Expand Down
7 changes: 6 additions & 1 deletion packages/nextjs/src/config/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export { SentryCliPluginOptions as SentryWebpackPluginOptions } from '@sentry/we
* Overall Nextjs config
*/

export type ExportedNextConfig = NextConfigObject;
export type ExportedNextConfig = NextConfigObject | NextConfigFunction;

export type NextConfigObject = {
// whether or not next should create source maps for browser code
Expand All @@ -17,6 +17,11 @@ export type NextConfigObject = {
[key: string]: unknown;
};

export type NextConfigFunction = (
phase: string,
Copy link
Member

Choose a reason for hiding this comment

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

this could be an interesting thing to experiment with in the future, like for ex. we construct different webpack configs based on dev/prod/export (if I'm understanding this correctly). https://github.com/vercel/next.js/blob/canary/packages/next/next-server/lib/constants.ts#L1-L4

Copy link
Member Author

Choose a reason for hiding this comment

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

You are. I think we might wait if/until it proves a problem, though, as I don't believe those things influence the parts of the config we muck with.

defaults: { defaultConfig: { [key: string]: unknown } },
) => NextConfigObject;

/**
* Webpack config
*/
Expand Down
19 changes: 18 additions & 1 deletion packages/nextjs/test/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ function materializeFinalNextConfig(
userNextConfig: ExportedNextConfig,
userSentryWebpackPluginConfig: SentryWebpackPluginOptions,
): NextConfigObject {
const finalConfigValues = withSentryConfig(userNextConfig, userSentryWebpackPluginConfig);
const configFunction = withSentryConfig(userNextConfig, userSentryWebpackPluginConfig);
const finalConfigValues = configFunction('phase-production-build', {
defaultConfig: {},
});

return finalConfigValues;
}
Expand Down Expand Up @@ -128,6 +131,20 @@ describe('withSentryConfig', () => {
}),
);
});

it("works when user's overall config is a function", () => {
const userNextConfigFunction = () => userNextConfig;

const finalConfig = materializeFinalNextConfig(userNextConfigFunction, userSentryWebpackPluginConfig);

expect(finalConfig).toEqual(
expect.objectContaining({
...userNextConfigFunction(),
productionBrowserSourceMaps: true,
webpack: expect.any(Function), // `webpack` is tested specifically elsewhere
}),
);
});
});

describe('webpack config', () => {
Expand Down