Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 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),
};
}
128 changes: 128 additions & 0 deletions packages/nextjs/src/config/nextConfigToWebpackPluginConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
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'];

type PropIncluderFn = (
nextConfig: NextConfigObject,
sentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions>,
) => Partial<SentryWebpackPluginOptions>;

export type PropsIncluderMapType = Record<string, PropIncluderFn>;
export const PROPS_INCLUDER_MAP: PropsIncluderMapType = {
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> {
// @ts-ignore '__spreadArray' import from tslib, ts(2343)
const propsToInclude = [...new Set(nextProps)];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to do this, we know that nextProps will always be unique. I'd rather refer to the global rather than the function arg as well to make the intent as clear as possible.

If you want to enforce that they will be unique, write a test that just checks if SUPPORTED_NEXTJS_PROPERTIES is all unique.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we could make SUPPORTED_NEXTJS_PROPERTIES a Set and then just iterate with https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/values

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't know whether they are unique, even if we make SUPPORTED_NEXTJS_PROPERTIES a Set. includeNextjsProps can be called from anywhere (even if it's only used in includeAllNextjsProps for now, but that's the point of generalizing after all) and with any array.

I've thought about two alternatives here: using a set as the argument type (which I think it's less convenient than using an array), and enforcing it with types (added a comment at the top of the file why this can't be accomplished). So decided this approach is the most suitable and added a test for duplicated keys.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can possibly enforce it with the types, let chat about it.

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 no `distDir` is provided, the Webpack Plugin config doesn't change.
* 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 New 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)) {
// @ts-ignore '__spreadArray' import from tslib, ts(2343)
sourcesToInclude = [...new Set(usersInclude.concat(nextConfig.distDir))];
} else {
// Object
if (Array.isArray(usersInclude.paths)) {
const uniquePaths = [...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}.*`));
}
}
});
});
});
102 changes: 102 additions & 0 deletions packages/nextjs/test/config/nextConfigToWebpackPluginConfig.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import includeAllNextjsProps, {
includeDistDir,
includeNextjsProps,
PropsIncluderMapType,
} from '../../src/config/nextConfigToWebpackPluginConfig';
import { SentryWebpackPluginOptions } from '../../src/config/types';

test('includeAllNextjsProps', () => {
expect(includeAllNextjsProps({ distDir: 'test' }, {})).toMatchObject({ include: 'test' });
});

describe('includeNextjsProps', () => {
const includerMap: PropsIncluderMapType = {
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({});
});
});

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