Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 18 commits into from
Sep 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
iker-barriocanal marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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 }), {})
);
iker-barriocanal marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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();
});
});
});