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

Author: Luca Forstner

packages/nextjs/src/config/withSentryConfig.ts

@@ -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,
@@ -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 {
@@ -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 {

packages/nextjs/src/utils/isBuild.ts

@@ -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.
  */
@@ -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;

packages/nextjs/src/utils/phases.ts

@@ -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';

packages/nextjs/test/config/fixtures.ts

@@ -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 };

packages/nextjs/test/config/testUtils.ts

@@ -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
@@ -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;

packages/nextjs/test/config/withSentryConfig.test.ts

@@ -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);
@@ -50,7 +47,7 @@ describe('withSentryConfig', () => {
 
     materializeFinalNextConfig(exportedNextConfigFunction);
 
-    expect(exportedNextConfigFunction).toHaveBeenCalledWith(runtimePhase, defaultsObject);
+    expect(exportedNextConfigFunction).toHaveBeenCalledWith(defaultRuntimePhase, defaultsObject);
   });
 
   it('removes `sentry` property', () => {
@@ -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();
       });