Skip to content

Commit

Permalink
feat(nextjs): Support distDir Next.js option (#3990)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
iker-barriocanal authored Sep 22, 2021
1 parent 873f167 commit 27f8609
Show file tree
Hide file tree
Showing 5 changed files with 308 additions and 10 deletions.
13 changes: 11 additions & 2 deletions packages/nextjs/src/config/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import includeAllNextjsProps from './nextConfigToWebpackPluginConfig';
import { ExportedNextConfig, NextConfigFunction, NextConfigObject, SentryWebpackPluginOptions } from './types';
import { constructWebpackConfigFunction } from './webpack';

Expand All @@ -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),
};
}
125 changes: 125 additions & 0 deletions packages/nextjs/src/config/nextConfigToWebpackPluginConfig.ts
Original file line number Diff line number Diff line change
@@ -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 };
}
11 changes: 6 additions & 5 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
52 changes: 49 additions & 3 deletions packages/nextjs/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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}.*`));
}
}
});
});
});
117 changes: 117 additions & 0 deletions packages/nextjs/test/config/nextConfigToWebpackPluginConfig.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
});

0 comments on commit 27f8609

Please sign in to comment.