Skip to content

Commit 0de61f8

Browse files
lobsterkatielobsterkatie
authored
fix(nextjs): Fix types in config code (#4057)
This cleans up the types we use in `withSentryConfig` and friends. Three specific changes: - When a user passes their nextjs config to nextjs, they're free to specify as few or as many of the options as they like. Nextjs config objects provided directly by nextjs, on the other hand, will always have certain properties, because they will have been filled in with defaults if not specified by the user. This differentiates between these two cases, through the use of the `Partial< ... >` utility type when describing config provided by the user. - In order to play more nicely with `@sentry/webpack-plugin` and its types, this uses the official `WebpackPluginInstance` type from webpack itself. This requires including webpack as a dependency, in this case a peer dependency, since we're only using it for its types and therefore don't actually care if it's installed or not. (Given nextjs's reliance on it, you might be asking yourself how a nextjs user would ever _not_ have it already installed, and the answer is that nextjs ships [with a precompiled version](https://github.com/vercel/next.js/tree/canary/packages/next/compiled/webpack) and therefore [doesn't include it](https://github.com/vercel/next.js/blob/canary/packages/next/package.json) as anything other than a dev dependency.) - Given that we still support webpack 4, this ramps our types dev dependency down to the lowest version which includes the compatibility type `WebpackPluginInstance`.
1 parent 2dbe5d7 commit 0de61f8

File tree

6 files changed

+127
-269
lines changed

6 files changed

+127
-269
lines changed

‎packages/nextjs/package.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,15 @@
2828
},
2929
"devDependencies": {
3030
"@sentry/types": "6.13.3",
31-
"@types/webpack": "^5.28.0",
31+
"@types/webpack": "^4.41.31",
3232
"eslint": "7.20.0",
3333
"next": "10.1.3",
3434
"rimraf": "3.0.2"
3535
},
3636
"peerDependencies": {
3737
"next": "^10.0.8 || ^11.0",
38-
"react": "15.x || 16.x || 17.x"
38+
"react": "15.x || 16.x || 17.x",
39+
"webpack": ">= 4.0.0"
3940
},
4041
"scripts": {
4142
"build": "run-p build:esm build:es5",

‎packages/nextjs/src/config/index.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ import { constructWebpackConfigFunction } from './webpack';
1111
export function withSentryConfig(
1212
userNextConfig: ExportedNextConfig = {},
1313
userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions> = {},
14-
): NextConfigFunction | NextConfigObject {
14+
): NextConfigFunction | Partial<NextConfigObject> {
1515
// If the user has passed us a function, we need to return a function, so that we have access to `phase` and
1616
// `defaults` in order to pass them along to the user's function
1717
if (typeof userNextConfig === 'function') {
18-
return function(phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject {
18+
return function(phase: string, defaults: { defaultConfig: NextConfigObject }): Partial<NextConfigObject> {
1919
const materializedUserNextConfig = userNextConfig(phase, defaults);
2020
return {
2121
...materializedUserNextConfig,

‎packages/nextjs/src/config/types.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
import { SentryCliPluginOptions } from '@sentry/webpack-plugin';
2+
import { WebpackPluginInstance } from 'webpack';
23

34
export type SentryWebpackPluginOptions = SentryCliPluginOptions;
4-
export type SentryWebpackPlugin = { options: SentryWebpackPluginOptions };
5+
export type SentryWebpackPlugin = WebpackPluginInstance & { options: SentryWebpackPluginOptions };
56

67
/**
78
* Overall Nextjs config
89
*/
910

10-
export type ExportedNextConfig = NextConfigObject | NextConfigFunction;
11+
export type ExportedNextConfig = Partial<NextConfigObject> | NextConfigFunction;
1112

1213
export type NextConfigObject = {
1314
// custom webpack options
14-
webpack?: WebpackConfigFunction;
15+
webpack: WebpackConfigFunction;
1516
// whether to build serverless functions for all pages, not just API routes
16-
target?: 'server' | 'experimental-serverless-trace';
17+
target: 'server' | 'experimental-serverless-trace';
1718
sentry?: {
1819
disableServerWebpackPlugin?: boolean;
1920
disableClientWebpackPlugin?: boolean;
@@ -25,8 +26,8 @@ export type NextConfigObject = {
2526

2627
export type NextConfigFunction = (
2728
phase: string,
28-
defaults: { defaultConfig: { [key: string]: unknown } },
29-
) => NextConfigObject;
29+
defaults: { defaultConfig: NextConfigObject },
30+
) => Partial<NextConfigObject>;
3031

3132
/**
3233
* Webpack config
@@ -37,7 +38,7 @@ export type WebpackConfigFunction = (config: WebpackConfigObject, options: Build
3738

3839
export type WebpackConfigObject = {
3940
devtool?: string;
40-
plugins?: Array<{ [key: string]: unknown }>;
41+
plugins?: Array<WebpackPluginInstance | SentryWebpackPlugin>;
4142
entry: WebpackEntryProperty;
4243
output: { filename: string; path: string };
4344
target: string;
@@ -56,7 +57,7 @@ export type BuildContext = {
5657
isServer: boolean;
5758
buildId: string;
5859
dir: string;
59-
config: Partial<NextConfigObject>;
60+
config: NextConfigObject;
6061
webpack: { version: string };
6162
};
6263

‎packages/nextjs/src/config/webpack.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { getSentryRelease } from '@sentry/node';
22
import { dropUndefinedKeys, logger } from '@sentry/utils';
3-
import * as SentryWebpackPlugin from '@sentry/webpack-plugin';
3+
import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin';
44
import * as fs from 'fs';
55
import * as path from 'path';
66

@@ -34,7 +34,7 @@ export { SentryWebpackPlugin };
3434
* @returns The function to set as the nextjs config's `webpack` value
3535
*/
3636
export function constructWebpackConfigFunction(
37-
userNextConfig: NextConfigObject = {},
37+
userNextConfig: Partial<NextConfigObject> = {},
3838
userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions> = {},
3939
): WebpackConfigFunction {
4040
// Will be called by nextjs and passed its default webpack configuration and context data about the build (whether
@@ -96,8 +96,6 @@ export function constructWebpackConfigFunction(
9696

9797
newConfig.plugins = newConfig.plugins || [];
9898
newConfig.plugins.push(
99-
// @ts-ignore Our types for the plugin are messed up somehow - TS wants this to be `SentryWebpackPlugin.default`,
100-
// but that's not actually a thing
10199
new SentryWebpackPlugin(getWebpackPluginOptions(buildContext, userSentryWebpackPluginOptions)),
102100
);
103101
}

‎packages/nextjs/test/config.test.ts‎

Lines changed: 55 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ import * as fs from 'fs';
22
import * as os from 'os';
33
import * as path from 'path';
44
import * as rimraf from 'rimraf';
5+
import { WebpackPluginInstance } from 'webpack';
56

67
import { withSentryConfig } from '../src/config';
78
import {
89
BuildContext,
910
EntryPropertyFunction,
1011
ExportedNextConfig,
1112
NextConfigObject,
12-
SentryWebpackPlugin as SentryWebpackPluginType,
1313
SentryWebpackPluginOptions,
1414
WebpackConfigObject,
1515
} from '../src/config/types';
@@ -58,7 +58,7 @@ process.env.SENTRY_RELEASE = 'doGsaREgReaT';
5858
const runtimePhase = 'ball-fetching';
5959
// `defaultConfig` is the defaults for all nextjs options (we don't use these at all in the tests, so for our purposes
6060
// here the values don't matter)
61-
const defaultsObject = { defaultConfig: {} };
61+
const defaultsObject = { defaultConfig: {} as NextConfigObject };
6262

6363
/** mocks of the arguments passed to `nextConfig.webpack` */
6464
const serverWebpackConfig = {
@@ -103,7 +103,7 @@ function getBuildContext(
103103
dev: false,
104104
buildId: 'sItStAyLiEdOwN',
105105
dir: '/Users/Maisey/projects/squirrelChasingSimulator',
106-
config: { target: 'server', ...userNextConfig },
106+
config: { target: 'server', ...userNextConfig } as NextConfigObject,
107107
webpack: { version: webpackVersion },
108108
isServer: buildTarget === 'server',
109109
};
@@ -177,6 +177,14 @@ async function materializeFinalWebpackConfig(options: {
177177
return finalWebpackConfigValue;
178178
}
179179

180+
// helper function to make sure we're checking the correct plugin's data
181+
export function findWebpackPlugin(
182+
webpackConfig: WebpackConfigObject,
183+
pluginName: string,
184+
): WebpackPluginInstance | SentryWebpackPlugin | undefined {
185+
return webpackConfig.plugins?.find(plugin => plugin.constructor.name === pluginName);
186+
}
187+
180188
describe('withSentryConfig', () => {
181189
it('includes expected properties', () => {
182190
const finalConfig = materializeFinalNextConfig(userNextConfig);
@@ -335,8 +343,9 @@ describe('Sentry webpack plugin config', () => {
335343
incomingWebpackConfig: serverWebpackConfig,
336344
incomingWebpackBuildContext: serverBuildContext,
337345
});
346+
const sentryWebpackPluginInstance = findWebpackPlugin(finalWebpackConfig, 'SentryCliPlugin') as SentryWebpackPlugin;
338347

339-
expect(finalWebpackConfig.plugins?.[0].options).toEqual(
348+
expect(sentryWebpackPluginInstance.options).toEqual(
340349
expect.objectContaining({
341350
include: expect.any(Array), // default, tested separately elsewhere
342351
ignore: [], // default
@@ -359,8 +368,9 @@ describe('Sentry webpack plugin config', () => {
359368
incomingWebpackConfig: serverWebpackConfig,
360369
incomingWebpackBuildContext: serverBuildContext,
361370
});
371+
const sentryWebpackPluginInstance = findWebpackPlugin(finalWebpackConfig, 'SentryCliPlugin') as SentryWebpackPlugin;
362372

363-
expect((finalWebpackConfig.plugins?.[0].options as SentryWebpackPluginOptions).debug).toEqual(true);
373+
expect(sentryWebpackPluginInstance.options.debug).toEqual(true);
364374
});
365375

366376
it('warns when overriding certain default values', () => {
@@ -379,9 +389,12 @@ describe('Sentry webpack plugin config', () => {
379389
incomingWebpackBuildContext: clientBuildContext,
380390
});
381391

382-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
392+
const sentryWebpackPluginInstance = findWebpackPlugin(
393+
finalWebpackConfig,
394+
'SentryCliPlugin',
395+
) as SentryWebpackPlugin;
383396

384-
expect(sentryWebpackPlugin.options?.include).toEqual([
397+
expect(sentryWebpackPluginInstance.options.include).toEqual([
385398
{ paths: ['.next/static/chunks/pages'], urlPrefix: '~/_next/static/chunks/pages' },
386399
]);
387400
});
@@ -396,9 +409,12 @@ describe('Sentry webpack plugin config', () => {
396409
incomingWebpackBuildContext: getBuildContext('server', userNextConfigServerless),
397410
});
398411

399-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
412+
const sentryWebpackPluginInstance = findWebpackPlugin(
413+
finalWebpackConfig,
414+
'SentryCliPlugin',
415+
) as SentryWebpackPlugin;
400416

401-
expect(sentryWebpackPlugin.options?.include).toEqual([
417+
expect(sentryWebpackPluginInstance.options.include).toEqual([
402418
{ paths: ['.next/serverless/'], urlPrefix: '~/_next/serverless' },
403419
]);
404420
});
@@ -413,9 +429,12 @@ describe('Sentry webpack plugin config', () => {
413429
incomingWebpackBuildContext: serverBuildContextWebpack4,
414430
});
415431

416-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
432+
const sentryWebpackPluginInstance = findWebpackPlugin(
433+
finalWebpackConfig,
434+
'SentryCliPlugin',
435+
) as SentryWebpackPlugin;
417436

418-
expect(sentryWebpackPlugin.options?.include).toEqual([
437+
expect(sentryWebpackPluginInstance.options.include).toEqual([
419438
{ paths: ['.next/server/pages/'], urlPrefix: '~/_next/server/pages' },
420439
]);
421440
});
@@ -427,9 +446,12 @@ describe('Sentry webpack plugin config', () => {
427446
incomingWebpackBuildContext: serverBuildContext,
428447
});
429448

430-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
449+
const sentryWebpackPluginInstance = findWebpackPlugin(
450+
finalWebpackConfig,
451+
'SentryCliPlugin',
452+
) as SentryWebpackPlugin;
431453

432-
expect(sentryWebpackPlugin.options?.include).toEqual([
454+
expect(sentryWebpackPluginInstance.options.include).toEqual([
433455
{ paths: ['.next/server/pages/'], urlPrefix: '~/_next/server/pages' },
434456
{ paths: ['.next/server/chunks/'], urlPrefix: '~/_next/server/chunks' },
435457
]);
@@ -449,9 +471,12 @@ describe('Sentry webpack plugin config', () => {
449471
incomingWebpackBuildContext: getBuildContext('client', userNextConfigWithBasePath),
450472
});
451473

452-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
474+
const sentryWebpackPluginInstance = findWebpackPlugin(
475+
finalWebpackConfig,
476+
'SentryCliPlugin',
477+
) as SentryWebpackPlugin;
453478

454-
expect(sentryWebpackPlugin.options?.include).toEqual([
479+
expect(sentryWebpackPluginInstance.options.include).toEqual([
455480
{ paths: ['.next/static/chunks/pages'], urlPrefix: '~/city-park/_next/static/chunks/pages' },
456481
]);
457482
});
@@ -466,9 +491,12 @@ describe('Sentry webpack plugin config', () => {
466491
incomingWebpackBuildContext: getBuildContext('server', userNextConfigServerless),
467492
});
468493

469-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
494+
const sentryWebpackPluginInstance = findWebpackPlugin(
495+
finalWebpackConfig,
496+
'SentryCliPlugin',
497+
) as SentryWebpackPlugin;
470498

471-
expect(sentryWebpackPlugin.options?.include).toEqual([
499+
expect(sentryWebpackPluginInstance.options.include).toEqual([
472500
{ paths: ['.next/serverless/'], urlPrefix: '~/city-park/_next/serverless' },
473501
]);
474502
});
@@ -483,9 +511,12 @@ describe('Sentry webpack plugin config', () => {
483511
incomingWebpackBuildContext: serverBuildContextWebpack4,
484512
});
485513

486-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
514+
const sentryWebpackPluginInstance = findWebpackPlugin(
515+
finalWebpackConfig,
516+
'SentryCliPlugin',
517+
) as SentryWebpackPlugin;
487518

488-
expect(sentryWebpackPlugin.options?.include).toEqual([
519+
expect(sentryWebpackPluginInstance.options.include).toEqual([
489520
{ paths: ['.next/server/pages/'], urlPrefix: '~/city-park/_next/server/pages' },
490521
]);
491522
});
@@ -497,9 +528,12 @@ describe('Sentry webpack plugin config', () => {
497528
incomingWebpackBuildContext: getBuildContext('server', userNextConfigWithBasePath),
498529
});
499530

500-
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
531+
const sentryWebpackPluginInstance = findWebpackPlugin(
532+
finalWebpackConfig,
533+
'SentryCliPlugin',
534+
) as SentryWebpackPlugin;
501535

502-
expect(sentryWebpackPlugin.options?.include).toEqual([
536+
expect(sentryWebpackPluginInstance.options.include).toEqual([
503537
{ paths: ['.next/server/pages/'], urlPrefix: '~/city-park/_next/server/pages' },
504538
{ paths: ['.next/server/chunks/'], urlPrefix: '~/city-park/_next/server/chunks' },
505539
]);

0 commit comments

Comments
 (0)