diff --git a/packages/nextjs/src/config/index.ts b/packages/nextjs/src/config/index.ts index 5cc4d5964893..5622f0055511 100644 --- 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 new file mode 100644 index 000000000000..89b8e00bf94f --- /dev/null +++ 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, +) => Partial; + +export const PROPS_INCLUDER_MAP: Record = { + 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, +): Partial { + 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, + propsIncluderMap: Record, + nextProps: string[], +): Partial { + 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, +): Partial { + 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 index 76dd5a8382da..748a98e75d67 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -256,11 +256,12 @@ 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 { 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'; @@ -268,12 +269,12 @@ function getWebpackPluginOptions( 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 index d395f43dcba5..8343a9053cc3 100644 --- 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): BuildContext { +function getBuildContext( + buildTarget: 'server' | 'client', + userNextConfig: Partial, + 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 new file mode 100644 index 000000000000..6d359934b464 --- /dev/null +++ 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 = { + test: includeEverythingFn, + }; + const includeEverything = { + include: 'everything', + }; + function includeEverythingFn(): Partial { + 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 = { + 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(); + }); + }); +});