Skip to content

fix(nextjs): Apply Webpack configuration in dev mode #6291

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 2 commits into from
Nov 24, 2022
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
25 changes: 11 additions & 14 deletions packages/nextjs/src/config/withSentryConfig.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isBuild } from '../utils/isBuild';
import { NEXT_PHASE_DEVELOPMENT_SERVER, NEXT_PHASE_PRODUCTION_BUILD } from '../utils/phases';
import type {
ExportedNextConfig,
NextConfigFunction,
Expand All @@ -18,23 +18,20 @@ export function withSentryConfig(
exportedUserNextConfig: ExportedNextConfig = {},
userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions> = {},
): NextConfigFunction | NextConfigObject {
// If the user has passed us a function, we need to return a function, so that we have access to `phase` and
// `defaults` in order to pass them along to the user's function
if (typeof exportedUserNextConfig === 'function') {
return function (phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject {
return function (phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject {
if (typeof exportedUserNextConfig === 'function') {
const userNextConfigObject = exportedUserNextConfig(phase, defaults);

return getFinalConfigObject(userNextConfigObject, userSentryWebpackPluginOptions);
};
}

// Otherwise, we can just merge their config with ours and return an object.
return getFinalConfigObject(exportedUserNextConfig, userSentryWebpackPluginOptions);
return getFinalConfigObject(phase, userNextConfigObject, userSentryWebpackPluginOptions);
} else {
return getFinalConfigObject(phase, exportedUserNextConfig, userSentryWebpackPluginOptions);
}
};
}

// Modify the materialized object form of the user's next config by deleting the `sentry` property and wrapping the
// `webpack` property
function getFinalConfigObject(
phase: string,
incomingUserNextConfigObject: NextConfigObjectWithSentry,
userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>,
): NextConfigObject {
Expand All @@ -48,8 +45,8 @@ function getFinalConfigObject(

// In order to prevent all of our build-time code from being bundled in people's route-handling serverless functions,
// we exclude `webpack.ts` and all of its dependencies from nextjs's `@vercel/nft` filetracing. We therefore need to
// make sure that we only require it at build time.
if (isBuild()) {
// make sure that we only require it at build time or in development mode.
if (phase === NEXT_PHASE_PRODUCTION_BUILD || phase === NEXT_PHASE_DEVELOPMENT_SERVER) {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { constructWebpackConfigFunction } = require('./webpack');
return {
Expand Down
4 changes: 3 additions & 1 deletion packages/nextjs/src/utils/isBuild.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { NEXT_PHASE_PRODUCTION_BUILD } from './phases';

/**
* Decide if the currently running process is part of the build phase or happening at runtime.
*/
Expand All @@ -18,7 +20,7 @@ export function isBuild(): boolean {
process.env.SENTRY_BUILD_PHASE ||
// This is set by next, but not until partway through the build process, which is why we need the above checks. That
// said, in case this function isn't called until we're in a child process, it can serve as a good backup.
process.env.NEXT_PHASE === 'phase-production-build'
process.env.NEXT_PHASE === NEXT_PHASE_PRODUCTION_BUILD
) {
process.env.SENTRY_BUILD_PHASE = 'true';
return true;
Expand Down
3 changes: 3 additions & 0 deletions packages/nextjs/src/utils/phases.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const NEXT_PHASE_PRODUCTION_BUILD = 'phase-production-build';
export const NEXT_PHASE_PRODUCTION_SERVER = 'phase-production-server';
export const NEXT_PHASE_DEVELOPMENT_SERVER = 'phase-development-server';
2 changes: 1 addition & 1 deletion packages/nextjs/test/config/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ process.env.SENTRY_AUTH_TOKEN = 'dogsarebadatkeepingsecrets';
process.env.SENTRY_RELEASE = 'doGsaREgReaT';

/** Mocks of the arguments passed to the result of `withSentryConfig` (when it's a function). */
export const runtimePhase = 'ball-fetching';
export const defaultRuntimePhase = 'ball-fetching';
// `defaultConfig` is the defaults for all nextjs options (we don't use these at all in the tests, so for our purposes
// here the values don't matter)
export const defaultsObject = { defaultConfig: {} as NextConfigObject };
Expand Down
5 changes: 3 additions & 2 deletions packages/nextjs/test/config/testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '../../src/config/types';
import { constructWebpackConfigFunction, SentryWebpackPlugin } from '../../src/config/webpack';
import { withSentryConfig } from '../../src/config/withSentryConfig';
import { defaultsObject, runtimePhase } from './fixtures';
import { defaultRuntimePhase, defaultsObject } from './fixtures';

/**
* Derive the final values of all next config options, by first applying `withSentryConfig` and then, if it returns a
Expand All @@ -25,14 +25,15 @@ import { defaultsObject, runtimePhase } from './fixtures';
export function materializeFinalNextConfig(
exportedNextConfig: ExportedNextConfig,
userSentryWebpackPluginConfig?: Partial<SentryWebpackPluginOptions>,
runtimePhase?: string,
): NextConfigObject {
const sentrifiedConfig = withSentryConfig(exportedNextConfig, userSentryWebpackPluginConfig);
let finalConfigValues = sentrifiedConfig;

if (typeof sentrifiedConfig === 'function') {
// for some reason TS won't recognize that `finalConfigValues` is now a NextConfigObject, which is why the cast
// below is necessary
finalConfigValues = sentrifiedConfig(runtimePhase, defaultsObject);
finalConfigValues = sentrifiedConfig(runtimePhase ?? defaultRuntimePhase, defaultsObject);
}

return finalConfigValues as NextConfigObject;
Expand Down
26 changes: 16 additions & 10 deletions packages/nextjs/test/config/withSentryConfig.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import * as isBuildModule from '../../src/utils/isBuild';
import { defaultsObject, exportedNextConfig, runtimePhase, userNextConfig } from './fixtures';
import { defaultRuntimePhase, defaultsObject, exportedNextConfig, userNextConfig } from './fixtures';
import { materializeFinalNextConfig } from './testUtils';

const isBuildSpy = jest.spyOn(isBuildModule, 'isBuild').mockReturnValue(true);

describe('withSentryConfig', () => {
it('includes expected properties', () => {
const finalConfig = materializeFinalNextConfig(exportedNextConfig);
Expand Down Expand Up @@ -50,7 +47,7 @@ describe('withSentryConfig', () => {

materializeFinalNextConfig(exportedNextConfigFunction);

expect(exportedNextConfigFunction).toHaveBeenCalledWith(runtimePhase, defaultsObject);
expect(exportedNextConfigFunction).toHaveBeenCalledWith(defaultRuntimePhase, defaultsObject);
});

it('removes `sentry` property', () => {
Expand All @@ -71,25 +68,34 @@ describe('withSentryConfig', () => {
// time, but the spy belongs to the first instance of the module and therefore never registers a call. Thus we have
// to test whether or not the file is required instead.

it('imports from `webpack.ts` if `isBuild` returns true', () => {
it('imports from `webpack.ts` if build phase is "phase-production-build"', () => {
jest.isolateModules(() => {
// In case this is still set from elsewhere, reset it
delete (global as any)._sentryWebpackModuleLoaded;

materializeFinalNextConfig(exportedNextConfig);
materializeFinalNextConfig(exportedNextConfig, undefined, 'phase-production-build');

expect((global as any)._sentryWebpackModuleLoaded).toBe(true);
});
});

it("doesn't import from `webpack.ts` if `isBuild` returns false", () => {
it('imports from `webpack.ts` if build phase is "phase-development-server"', () => {
jest.isolateModules(() => {
isBuildSpy.mockReturnValueOnce(false);
// In case this is still set from elsewhere, reset it
delete (global as any)._sentryWebpackModuleLoaded;

materializeFinalNextConfig(exportedNextConfig, undefined, 'phase-production-build');

expect((global as any)._sentryWebpackModuleLoaded).toBe(true);
});
});

it('Doesn\'t import from `webpack.ts` if build phase is "phase-production-server"', () => {
jest.isolateModules(() => {
// In case this is still set from elsewhere, reset it
delete (global as any)._sentryWebpackModuleLoaded;

materializeFinalNextConfig(exportedNextConfig);
materializeFinalNextConfig(exportedNextConfig, undefined, 'phase-production-server');

expect((global as any)._sentryWebpackModuleLoaded).toBeUndefined();
});
Expand Down