fix(nextjs): Move userNextConfig.sentry to closure (#5473)

As of vercel/next.js#38498, nextjs now warns (voluminously) if the config exported from `next.config.js` contains properties it doesn't recognize, including the `sentry` property we've been having people use to do things like [disable the webpack plugin](https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#disable-sentrywebpackplugin). In order to prevent these warnings, the final config returned from `withSentryConfig` must not have such a property, so this deletes the property from the `userNextConfig` object before returning it. 

If we were to stop there, the warning would be gone, but so would the data `userNextConfig.sentry` contains. (This is true even if we run `constructWebpackConfigFunction` before doing the deletion, because the place we need the `userNextConfig.sentry` data is not in `constructWebpackConfigFunction` itself but in the function it returns. By the time that function is called by nextjs, the deletion will already have gone through and the data will be gone unless we keep a reference to it elsewhere.) Therefore, in order to allow the returned function to retain access the `userNextConfig.sentry` data, this captures it before it's deleted and passes it to `constructWebpackConfigFunction`, where it lives in a closure around the returned function.

Author: lobsterkatie

packages/nextjs/src/config/index.ts

@@ -17,16 +17,33 @@ export function withSentryConfig(
   if (typeof userNextConfig === 'function') {
     return function (phase: string, defaults: { defaultConfig: NextConfigObject }): Partial<NextConfigObject> {
       const materializedUserNextConfig = userNextConfig(phase, defaults);
+
+      // Next 12.2.3+ warns about non-canonical properties on `userNextConfig`, so grab and then remove the `sentry`
+      // property there. Where we actually need it is in the webpack config function we're going to create, so pass it
+      // to `constructWebpackConfigFunction` so that it will be in the created function's closure.
+      const { sentry: userSentryOptions } = materializedUserNextConfig;
+      delete materializedUserNextConfig.sentry;
+
       return {
         ...materializedUserNextConfig,
-        webpack: constructWebpackConfigFunction(materializedUserNextConfig, userSentryWebpackPluginOptions),
+        webpack: constructWebpackConfigFunction(
+          materializedUserNextConfig,
+          userSentryWebpackPluginOptions,
+          userSentryOptions,
+        ),
       };
     };
   }
 
   // Otherwise, we can just merge their config with ours and return an object.
+
+  // Prevent nextjs from getting mad about having a non-standard config property in `userNextConfig`. (See note above
+  // for a more thorough explanation of what we're doing here.)
+  const { sentry: userSentryOptions } = userNextConfig;
+  delete userNextConfig.sentry;
+
   return {
     ...userNextConfig,
-    webpack: constructWebpackConfigFunction(userNextConfig, userSentryWebpackPluginOptions),
+    webpack: constructWebpackConfigFunction(userNextConfig, userSentryWebpackPluginOptions, userSentryOptions),
   };
 }

packages/nextjs/src/config/types.ts

@@ -17,26 +17,29 @@ export type NextConfigObject = {
   target: 'server' | 'experimental-serverless-trace';
   // the output directory for the built app (defaults to ".next")
   distDir: string;
-  sentry?: {
-    disableServerWebpackPlugin?: boolean;
-    disableClientWebpackPlugin?: boolean;
-    hideSourceMaps?: boolean;
-
-    // Force webpack to apply the same transpilation rules to the SDK code as apply to user code. Helpful when targeting
-    // older browsers which don't support ES6 (or ES6+ features like object spread).
-    transpileClientSDK?: boolean;
-    // Upload files from `<distDir>/static/chunks` rather than `<distDir>/static/chunks/pages`. Usually files outside of
-    // `pages/` only contain third-party code, but in cases where they contain user code, restricting the webpack
-    // plugin's upload breaks sourcemaps for those user-code-containing files, because it keeps them from being
-    // uploaded. At the same time, we don't want to widen the scope if we don't have to, because we're guaranteed to end
-    // up uploading too many files, which is why this defaults to `false`.
-    widenClientFileUpload?: boolean;
-  };
+  sentry?: UserSentryOptions;
 } & {
   // other `next.config.js` options
   [key: string]: unknown;
 };
 
+export type UserSentryOptions = {
+  disableServerWebpackPlugin?: boolean;
+  disableClientWebpackPlugin?: boolean;
+  hideSourceMaps?: boolean;
+
+  // Force webpack to apply the same transpilation rules to the SDK code as apply to user code. Helpful when targeting
+  // older browsers which don't support ES6 (or ES6+ features like object spread).
+  transpileClientSDK?: boolean;
+
+  // Upload files from `<distDir>/static/chunks` rather than `<distDir>/static/chunks/pages`. Usually files outside of
+  // `pages/` only contain third-party code, but in cases where they contain user code, restricting the webpack
+  // plugin's upload breaks sourcemaps for those user-code-containing files, because it keeps them from being
+  // uploaded. At the same time, we don't want to widen the scope if we don't have to, because we're guaranteed to end
+  // up uploading too many files, which is why this defaults to `false`.
+  widenClientFileUpload?: boolean;
+};
+
 export type NextConfigFunction = (
   phase: string,
   defaults: { defaultConfig: NextConfigObject },

packages/nextjs/src/config/webpack.ts

@@ -10,6 +10,7 @@ import {
   EntryPropertyObject,
   NextConfigObject,
   SentryWebpackPluginOptions,
+  UserSentryOptions,
   WebpackConfigFunction,
   WebpackConfigObject,
   WebpackEntryProperty,
@@ -37,6 +38,7 @@ export { SentryWebpackPlugin };
 export function constructWebpackConfigFunction(
   userNextConfig: Partial<NextConfigObject> = {},
   userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions> = {},
+  userSentryOptions: UserSentryOptions = {},
 ): WebpackConfigFunction {
   // Will be called by nextjs and passed its default webpack configuration and context data about the build (whether
   // we're building server or client, whether we're in dev, what version of webpack we're using, etc). Note that
@@ -122,9 +124,7 @@ export function constructWebpackConfigFunction(
       // with the `--ignore-scripts` option, this will be blocked and the missing binary will cause an error when users
       // try to build their apps.)
       ensureCLIBinaryExists() &&
-      (isServer
-        ? !userNextConfig.sentry?.disableServerWebpackPlugin
-        : !userNextConfig.sentry?.disableClientWebpackPlugin);
+      (isServer ? !userSentryOptions.disableServerWebpackPlugin : !userSentryOptions.disableClientWebpackPlugin);
 
     if (enableWebpackPlugin) {
       // TODO Handle possibility that user is using `SourceMapDevToolPlugin` (see
@@ -138,12 +138,14 @@ export function constructWebpackConfigFunction(
         // the browser won't look for them and throw errors into the console when it can't find them. Because this is a
         // front-end-only problem, and because `sentry-cli` handles sourcemaps more reliably with the comment than
         // without, the option to use `hidden-source-map` only applies to the client-side build.
-        newConfig.devtool = userNextConfig.sentry?.hideSourceMaps && !isServer ? 'hidden-source-map' : 'source-map';
+        newConfig.devtool = userSentryOptions.hideSourceMaps && !isServer ? 'hidden-source-map' : 'source-map';
       }
 
       newConfig.plugins = newConfig.plugins || [];
       newConfig.plugins.push(
-        new SentryWebpackPlugin(getWebpackPluginOptions(buildContext, userSentryWebpackPluginOptions)),
+        new SentryWebpackPlugin(
+          getWebpackPluginOptions(buildContext, userSentryWebpackPluginOptions, userSentryOptions),
+        ),
       );
     }
 
@@ -381,6 +383,7 @@ function shouldAddSentryToEntryPoint(entryPointName: string, isServer: boolean):
 export function getWebpackPluginOptions(
   buildContext: BuildContext,
   userPluginOptions: Partial<SentryWebpackPluginOptions>,
+  userSentryOptions: UserSentryOptions,
 ): SentryWebpackPluginOptions {
   const { buildId, isServer, webpack, config: userNextConfig, dev: isDev, dir: projectDir } = buildContext;
   const distDir = userNextConfig.distDir ?? '.next'; // `.next` is the default directory
@@ -396,14 +399,14 @@ export function getWebpackPluginOptions(
         isWebpack5 ? [{ paths: [`${distDir}/server/chunks/`], urlPrefix: `${urlPrefix}/server/chunks` }] : [],
       );
 
-  const clientInclude = userNextConfig.sentry?.widenClientFileUpload
+  const clientInclude = userSentryOptions.widenClientFileUpload
     ? [{ paths: [`${distDir}/static/chunks`], urlPrefix: `${urlPrefix}/static/chunks` }]
     : [{ paths: [`${distDir}/static/chunks/pages`], urlPrefix: `${urlPrefix}/static/chunks/pages` }];
 
   const defaultPluginOptions = dropUndefinedKeys({
     include: isServer ? serverInclude : clientInclude,
     ignore:
-      isServer || !userNextConfig.sentry?.widenClientFileUpload
+      isServer || !userSentryOptions.widenClientFileUpload
         ? []
         : // Widening the upload scope is necessarily going to lead to us uploading files we don't need to (ones which
           // don't include any user code). In order to lessen that where we can, exclude the internal nextjs files we know

packages/nextjs/test/config.test.ts

@@ -200,6 +200,7 @@ async function materializeFinalWebpackConfig(options: {
   const webpackConfigFunction = constructWebpackConfigFunction(
     materializedUserNextConfig,
     userSentryWebpackPluginConfig,
+    materializedUserNextConfig.sentry,
   );
 
   // call it to get concrete values for comparison
@@ -870,9 +871,11 @@ describe('Sentry webpack plugin config', () => {
       [getBuildContext('server', {}, '4'), '.next'],
       [getBuildContext('server', {}, '5'), '.next'],
     ])('`distDir` is not defined', (buildContext: BuildContext, expectedDistDir) => {
-      const includePaths = getWebpackPluginOptions(buildContext, {
-        /** userPluginOptions */
-      }).include as { paths: [] }[];
+      const includePaths = getWebpackPluginOptions(
+        buildContext,
+        {}, // userPluginOptions
+        {}, // userSentryOptions
+      ).include as { paths: [] }[];
 
       for (const pathDescriptor of includePaths) {
         for (const path of pathDescriptor.paths) {
@@ -887,9 +890,11 @@ describe('Sentry webpack plugin config', () => {
       [getBuildContext('server', { distDir: 'tmpDir' }, '4'), 'tmpDir'],
       [getBuildContext('server', { distDir: 'tmpDir' }, '5'), 'tmpDir'],
     ])('`distDir` is defined', (buildContext: BuildContext, expectedDistDir) => {
-      const includePaths = getWebpackPluginOptions(buildContext, {
-        /** userPluginOptions */
-      }).include as { paths: [] }[];
+      const includePaths = getWebpackPluginOptions(
+        buildContext,
+        {}, // userPluginOptions
+        {}, // userSentryOptions
+      ).include as { paths: [] }[];
 
       for (const pathDescriptor of includePaths) {
         for (const path of pathDescriptor.paths) {