feat(nextjs): Support distDir Next.js option (#3990)

iker-barriocanal · web-flow · commit 27f8609771b4 · 2021-09-22T11:09:18.000+02:00
Add support for [setting custom build directories](https://nextjs.org/docs/api-reference/next.config.js/setting-a-custom-build-directory). Although this allows to custom that directory, source maps won't work; supporting them will come in a follow-up PR.
diff --git a/packages/nextjs/src/config/index.ts b/packages/nextjs/src/config/index.ts
@@ -1,3 +1,4 @@
+import includeAllNextjsProps from './nextConfigToWebpackPluginConfig';
 import { ExportedNextConfig, NextConfigFunction, NextConfigObject, SentryWebpackPluginOptions } from './types';
 import { constructWebpackConfigFunction } from './webpack';
 
@@ -17,13 +18,21 @@ export function withSentryConfig(
   if (typeof userNextConfig === 'function') {
     return function(phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject {
       const materializedUserNextConfig = userNextConfig(phase, defaults);
+      const sentryWebpackPluginOptionsWithSources = includeAllNextjsProps(
+        materializedUserNextConfig,
+        userSentryWebpackPluginOptions,
+      );
       return {
         ...materializedUserNextConfig,
-        webpack: constructWebpackConfigFunction(materializedUserNextConfig, userSentryWebpackPluginOptions),
+        webpack: constructWebpackConfigFunction(materializedUserNextConfig, sentryWebpackPluginOptionsWithSources),
       };
     };
   }
 
+  const webpackPluginOptionsWithSources = includeAllNextjsProps(userNextConfig, userSentryWebpackPluginOptions);
   // Otherwise, we can just merge their config with ours and return an object.
-  return { ...userNextConfig, webpack: constructWebpackConfigFunction(userNextConfig, userSentryWebpackPluginOptions) };
+  return {
+    ...userNextConfig,
+    webpack: constructWebpackConfigFunction(userNextConfig, webpackPluginOptionsWithSources),
+  };
 }
diff --git a/packages/nextjs/src/config/nextConfigToWebpackPluginConfig.ts b/packages/nextjs/src/config/nextConfigToWebpackPluginConfig.ts
@@ -0,0 +1,125 @@
+import { NextConfigObject, SentryWebpackPluginOptions } from './types';
+
+/**
+ * About types:
+ * It's not possible to set strong types because they end up forcing you to explicitly
+ * set `undefined` for properties you don't want to include, which is quite
+ * inconvenient. The workaround to this is to relax type requirements at some point,
+ * which means not enforcing types (why have strong typing then?) and still having code
+ * that is hard to read.
+ */
+
+/**
+ * Next.js properties that should modify the webpack plugin properties.
+ * They should have an includer function in the map.
+ */
+export const SUPPORTED_NEXTJS_PROPERTIES = ['distDir'];
+
+export type PropIncluderFn = (
+  nextConfig: NextConfigObject,
+  sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>,
+) => Partial<SentryWebpackPluginOptions>;
+
+export const PROPS_INCLUDER_MAP: Record<string, PropIncluderFn> = {
+  distDir: includeDistDir,
+};
+
+/**
+ * Creates a new Sentry Webpack Plugin config from the given one, including all available
+ * properties in the Nextjs Config.
+ *
+ * @param nextConfig User's Next.js config.
+ * @param sentryWebpackPluginOptions User's Sentry Webpack Plugin config.
+ * @returns New Sentry Webpack Plugin Config.
+ */
+export default function includeAllNextjsProps(
+  nextConfig: NextConfigObject,
+  sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>,
+): Partial<SentryWebpackPluginOptions> {
+  return includeNextjsProps(nextConfig, sentryWebpackPluginOptions, PROPS_INCLUDER_MAP, SUPPORTED_NEXTJS_PROPERTIES);
+}
+
+/**
+ * Creates a new Sentry Webpack Plugin config from the given one, and applying the corresponding
+ * modifications to the given next properties. If more than one option generates the same
+ * properties, the values generated last will override previous ones.
+ *
+ * @param nextConfig User's Next.js config.
+ * @param sentryWebpackPluginOptions User's Sentry Webapck Plugin config.
+ * @param nextProps Next.js config's properties that should modify webpack plugin properties.
+ * @returns New Sentry Webpack Plugin config.
+ */
+export function includeNextjsProps(
+  nextConfig: NextConfigObject,
+  sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>,
+  propsIncluderMap: Record<string, PropIncluderFn>,
+  nextProps: string[],
+): Partial<SentryWebpackPluginOptions> {
+  const propsToInclude = Array.from(new Set(nextProps));
+  return (
+    propsToInclude
+      // Types are not strict enought to ensure there's a function in the map
+      .filter(prop => propsIncluderMap[prop])
+      .map(prop => propsIncluderMap[prop](nextConfig, sentryWebpackPluginOptions))
+      .reduce((prev, current) => ({ ...prev, ...current }), {})
+  );
+}
+
+/**
+ * Creates a new Sentry Webpack Plugin config with the `distDir` option from Next.js config
+ * in the `include` property, if `distDir` is provided.
+ *
+ * If no `distDir` is provided, the Webpack Plugin config doesn't change and the same object is returned.
+ * If no `include` has been defined defined, the `distDir` value is assigned.
+ * The `distDir` directory is merged to the directories in `include`, if defined.
+ * Duplicated paths are removed while merging.
+ *
+ * @param nextConfig User's Next.js config
+ * @param sentryWebpackPluginOptions User's Sentry Webpack Plugin config
+ * @returns Sentry Webpack Plugin config
+ */
+export function includeDistDir(
+  nextConfig: NextConfigObject,
+  sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>,
+): Partial<SentryWebpackPluginOptions> {
+  if (!nextConfig.distDir) {
+    return sentryWebpackPluginOptions;
+  }
+  // It's assumed `distDir` is a string as that's what Next.js is expecting. If it's not, Next.js itself will complain
+  const usersInclude = sentryWebpackPluginOptions.include;
+
+  let sourcesToInclude;
+  if (typeof usersInclude === 'undefined') {
+    sourcesToInclude = nextConfig.distDir;
+  } else if (typeof usersInclude === 'string') {
+    sourcesToInclude = usersInclude === nextConfig.distDir ? usersInclude : [usersInclude, nextConfig.distDir];
+  } else if (Array.isArray(usersInclude)) {
+    sourcesToInclude = Array.from(new Set(usersInclude.concat(nextConfig.distDir as string)));
+  } else {
+    // Object
+    if (Array.isArray(usersInclude.paths)) {
+      const uniquePaths = Array.from(new Set(usersInclude.paths.concat(nextConfig.distDir as string)));
+      sourcesToInclude = { ...usersInclude, paths: uniquePaths };
+    } else if (typeof usersInclude.paths === 'undefined') {
+      // eslint-disable-next-line no-console
+      console.warn(
+        'Sentry Logger [Warn]:',
+        `An object was set in \`include\` but no \`paths\` was provided, so added the \`distDir\`: "${nextConfig.distDir}"\n` +
+          'See https://github.com/getsentry/sentry-webpack-plugin#optionsinclude',
+      );
+      sourcesToInclude = { ...usersInclude, paths: [nextConfig.distDir] };
+    } else {
+      // eslint-disable-next-line no-console
+      console.error(
+        'Sentry Logger [Error]:',
+        'Found unexpected object in `include.paths`\n' +
+          'See https://github.com/getsentry/sentry-webpack-plugin#optionsinclude',
+      );
+      // Keep the same object even if it's incorrect, so that the user can get a more precise error from sentry-cli
+      // Casting to `any` for TS not complaining about it being `unknown`
+      sourcesToInclude = usersInclude as any;
+    }
+  }
+
+  return { ...sentryWebpackPluginOptions, include: sourcesToInclude };
+}
diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts
@@ -256,24 +256,25 @@ function shouldAddSentryToEntryPoint(entryPointName: string): boolean {
  * @param userPluginOptions User-provided SentryWebpackPlugin options
  * @returns Final set of combined options
  */
-function getWebpackPluginOptions(
+export function getWebpackPluginOptions(
   buildContext: BuildContext,
   userPluginOptions: Partial<SentryWebpackPluginOptions>,
 ): SentryWebpackPluginOptions {
   const { isServer, dir: projectDir, buildId, dev: isDev, config: nextConfig, webpack } = buildContext;
+  const distDir = nextConfig.distDir ?? '.next'; // `.next` is the default directory
 
   const isWebpack5 = webpack.version.startsWith('5');
   const isServerless = nextConfig.target === 'experimental-serverless-trace';
   const hasSentryProperties = fs.existsSync(path.resolve(projectDir, 'sentry.properties'));
   const urlPrefix = nextConfig.basePath ? `~${nextConfig.basePath}/_next` : '~/_next';
 
   const serverInclude = isServerless
-    ? [{ paths: ['.next/serverless/'], urlPrefix: `${urlPrefix}/serverless` }]
-    : [{ paths: ['.next/server/pages/'], urlPrefix: `${urlPrefix}/server/pages` }].concat(
-        isWebpack5 ? [{ paths: ['.next/server/chunks/'], urlPrefix: `${urlPrefix}/server/chunks` }] : [],
+    ? [{ paths: [`${distDir}/serverless/`], urlPrefix: `${urlPrefix}/serverless` }]
+    : [{ paths: [`${distDir}/server/pages/`], urlPrefix: `${urlPrefix}/server/pages` }].concat(
+        isWebpack5 ? [{ paths: [`${distDir}/server/chunks/`], urlPrefix: `${urlPrefix}/server/chunks` }] : [],
       );
 
-  const clientInclude = [{ paths: ['.next/static/chunks/pages'], urlPrefix: `${urlPrefix}/static/chunks/pages` }];
+  const clientInclude = [{ paths: [`${distDir}/static/chunks/pages`], urlPrefix: `${urlPrefix}/static/chunks/pages` }];
 
   const defaultPluginOptions = dropUndefinedKeys({
     include: isServer ? serverInclude : clientInclude,
diff --git a/packages/nextjs/test/config.test.ts b/packages/nextjs/test/config.test.ts
@@ -13,7 +13,12 @@ import {
   SentryWebpackPluginOptions,
   WebpackConfigObject,
 } from '../src/config/types';
-import { constructWebpackConfigFunction, getUserConfigFile, SentryWebpackPlugin } from '../src/config/webpack';
+import {
+  constructWebpackConfigFunction,
+  getUserConfigFile,
+  getWebpackPluginOptions,
+  SentryWebpackPlugin,
+} from '../src/config/webpack';
 
 const SERVER_SDK_CONFIG_FILE = 'sentry.server.config.js';
 const CLIENT_SDK_CONFIG_FILE = 'sentry.client.config.js';
@@ -89,16 +94,21 @@ const clientWebpackConfig = {
 // In real life, next will copy the `userNextConfig` into the `buildContext`. Since we're providing mocks for both of
 // those, we need to mimic that behavior, and since `userNextConfig` can vary per test, we need to have the option do it
 // dynamically.
-function getBuildContext(buildTarget: 'server' | 'client', userNextConfig: Partial<NextConfigObject>): BuildContext {
+function getBuildContext(
+  buildTarget: 'server' | 'client',
+  userNextConfig: Partial<NextConfigObject>,
+  webpackVersion: string = '5.4.15',
+): BuildContext {
   return {
     dev: false,
     buildId: 'sItStAyLiEdOwN',
     dir: '/Users/Maisey/projects/squirrelChasingSimulator',
     config: { target: 'server', ...userNextConfig },
-    webpack: { version: '5.4.15' },
+    webpack: { version: webpackVersion },
     isServer: buildTarget === 'server',
   };
 }
+
 const serverBuildContext = getBuildContext('server', userNextConfig);
 const clientBuildContext = getBuildContext('client', userNextConfig);
 
@@ -580,4 +590,40 @@ describe('Sentry webpack plugin config', () => {
       );
     });
   });
+
+  describe('correct paths from `distDir` in WebpackPluginOptions', () => {
+    it.each([
+      [getBuildContext('client', {}), '.next'],
+      [getBuildContext('server', { target: 'experimental-serverless-trace' }), '.next'], // serverless
+      [getBuildContext('server', {}, '4'), '.next'],
+      [getBuildContext('server', {}, '5'), '.next'],
+    ])('`distDir` is not defined', (buildContext: BuildContext, expectedDistDir) => {
+      const includePaths = getWebpackPluginOptions(buildContext, {
+        /** userPluginOptions */
+      }).include as { paths: [] }[];
+
+      for (const pathDescriptor of includePaths) {
+        for (const path of pathDescriptor.paths) {
+          expect(path).toMatch(new RegExp(`^${expectedDistDir}.*`));
+        }
+      }
+    });
+
+    it.each([
+      [getBuildContext('client', { distDir: 'tmpDir' }), 'tmpDir'],
+      [getBuildContext('server', { distDir: 'tmpDir', target: 'experimental-serverless-trace' }), 'tmpDir'], // serverless
+      [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: [] }[];
+
+      for (const pathDescriptor of includePaths) {
+        for (const path of pathDescriptor.paths) {
+          expect(path).toMatch(new RegExp(`^${expectedDistDir}.*`));
+        }
+      }
+    });
+  });
 });
diff --git a/packages/nextjs/test/config/nextConfigToWebpackPluginConfig.test.ts b/packages/nextjs/test/config/nextConfigToWebpackPluginConfig.test.ts
@@ -0,0 +1,117 @@
+import includeAllNextjsProps, {
+  includeDistDir,
+  includeNextjsProps,
+  PropIncluderFn,
+} from '../../src/config/nextConfigToWebpackPluginConfig';
+import { SentryWebpackPluginOptions } from '../../src/config/types';
+
+test('includeAllNextjsProps', () => {
+  expect(includeAllNextjsProps({ distDir: 'test' }, {})).toMatchObject({ include: 'test' });
+});
+
+describe('includeNextjsProps', () => {
+  const includerMap: Record<string, PropIncluderFn> = {
+    test: includeEverythingFn,
+  };
+  const includeEverything = {
+    include: 'everything',
+  };
+  function includeEverythingFn(): Partial<SentryWebpackPluginOptions> {
+    return includeEverything;
+  }
+
+  test('a prop and an includer', () => {
+    expect(includeNextjsProps({ test: true }, {}, includerMap, ['test'])).toMatchObject(includeEverything);
+  });
+
+  test('a prop without includer', () => {
+    expect(includeNextjsProps({ noExist: false }, {}, includerMap, ['noExist'])).toMatchObject({});
+  });
+
+  test('an includer without a prop', () => {
+    expect(includeNextjsProps({ noExist: false }, {}, includerMap, ['test'])).toMatchObject({});
+  });
+
+  test('neither prop nor includer', () => {
+    expect(includeNextjsProps({}, {}, {}, [])).toMatchObject({});
+  });
+
+  test('duplicated props', () => {
+    let counter: number = 0;
+    const mock = jest.fn().mockImplementation(() => {
+      const current = counter;
+      counter += 1;
+      return { call: current };
+    });
+    const map: Record<string, PropIncluderFn> = {
+      dup: mock,
+    };
+
+    expect(includeNextjsProps({}, {}, map, ['dup', 'dup'])).toMatchObject({ call: 0 });
+    expect(mock).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe('next config to webpack plugin config', () => {
+  describe('includeDistDir', () => {
+    const consoleWarnMock = jest.fn();
+    const consoleErrorMock = jest.fn();
+
+    beforeAll(() => {
+      global.console.warn = consoleWarnMock;
+      global.console.error = consoleErrorMock;
+    });
+
+    afterAll(() => {
+      jest.restoreAllMocks();
+    });
+
+    test.each([
+      [{}, {}, {}],
+      [{}, { include: 'path' }, { include: 'path' }],
+      [{}, { include: [] }, { include: [] }],
+      [{}, { include: ['path'] }, { include: ['path'] }],
+      [{}, { include: { paths: ['path'] } }, { include: { paths: ['path'] } }],
+    ])('without `distDir`', (nextConfig, webpackPluginConfig, expectedConfig) => {
+      expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig);
+    });
+
+    test.each([
+      [{ distDir: 'test' }, {}, { include: 'test' }],
+      [{ distDir: 'test' }, { include: 'path' }, { include: ['path', 'test'] }],
+      [{ distDir: 'test' }, { include: [] }, { include: ['test'] }],
+      [{ distDir: 'test' }, { include: ['path'] }, { include: ['path', 'test'] }],
+      [{ distDir: 'test' }, { include: { paths: ['path'] } }, { include: { paths: ['path', 'test'] } }],
+    ])('with `distDir`, different paths', (nextConfig, webpackPluginConfig, expectedConfig) => {
+      expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig);
+    });
+
+    test.each([
+      [{ distDir: 'path' }, { include: 'path' }, { include: 'path' }],
+      [{ distDir: 'path' }, { include: ['path'] }, { include: ['path'] }],
+      [{ distDir: 'path' }, { include: { paths: ['path'] } }, { include: { paths: ['path'] } }],
+    ])('with `distDir`, same path', (nextConfig, webpackPluginConfig, expectedConfig) => {
+      expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig);
+    });
+
+    test.each([
+      [{ distDir: 'path' }, { include: {} }, { include: { paths: ['path'] } }],
+      [{ distDir: 'path' }, { include: { prop: 'val' } }, { include: { prop: 'val', paths: ['path'] } }],
+    ])('webpack plugin config as object with other prop', (nextConfig, webpackPluginConfig, expectedConfig) => {
+      // @ts-ignore Other props don't match types
+      expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig);
+      expect(consoleWarnMock).toHaveBeenCalledTimes(1);
+      consoleWarnMock.mockClear();
+    });
+
+    test.each([
+      [{ distDir: 'path' }, { include: { paths: {} } }, { include: { paths: {} } }],
+      [{ distDir: 'path' }, { include: { paths: { badObject: true } } }, { include: { paths: { badObject: true } } }],
+    ])('webpack plugin config as object with bad structure', (nextConfig, webpackPluginConfig, expectedConfig) => {
+      // @ts-ignore Bad structures don't match types
+      expect(includeDistDir(nextConfig, webpackPluginConfig)).toMatchObject(expectedConfig);
+      expect(consoleErrorMock).toHaveBeenCalledTimes(1);
+      consoleErrorMock.mockClear();
+    });
+  });
+});
