Skip to content

fix(nextjs): Fix types in config code #4057

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 12, 2021
Merged

fix(nextjs): Fix types in config code #4057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
72f16e2
differentiate between partial and full next config objects
lobsterkatie Oct 11, 2021
87cf229
add webpack as a peer dependency and roll back to webpack 4 types
lobsterkatie Oct 11, 2021
a0fccaa
correctly type sentry webpack plugin
lobsterkatie Oct 11, 2021
c9eb97b
stop importing sentry webpack plugin type separately
lobsterkatie Oct 11, 2021
1b46c28
use helper function for finding correct plugin to test
lobsterkatie Oct 11, 2021
58dfdd9
Merge branch 'master' into kmclb-nextjs-fix-config-types-oct-2021
lobsterkatie Oct 12, 2021
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
5 changes: 3 additions & 2 deletions packages/nextjs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@
},
"devDependencies": {
"@sentry/types": "6.13.3",
"@types/webpack": "^5.28.0",
"@types/webpack": "^4.41.31",
"eslint": "7.20.0",
"next": "10.1.3",
"rimraf": "3.0.2"
},
"peerDependencies": {
"next": "^10.0.8 || ^11.0",
"react": "15.x || 16.x || 17.x"
"react": "15.x || 16.x || 17.x",
"webpack": ">= 4.0.0"
},
"scripts": {
"build": "run-p build:esm build:es5",
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/config/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import { constructWebpackConfigFunction } from './webpack';
export function withSentryConfig(
userNextConfig: ExportedNextConfig = {},
userSentryWebpackPluginOptions: Partial<SentryWebpackPluginOptions> = {},
): NextConfigFunction | NextConfigObject {
): NextConfigFunction | Partial<NextConfigObject> {
// If the user has passed us a function, we need to return a function, so that we have access to `phase` and
// `defaults` in order to pass them along to the user's function
if (typeof userNextConfig === 'function') {
return function(phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject {
return function(phase: string, defaults: { defaultConfig: NextConfigObject }): Partial<NextConfigObject> {
const materializedUserNextConfig = userNextConfig(phase, defaults);
return {
...materializedUserNextConfig,
Expand Down
17 changes: 9 additions & 8 deletions packages/nextjs/src/config/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import { SentryCliPluginOptions } from '@sentry/webpack-plugin';
import { WebpackPluginInstance } from 'webpack';

export type SentryWebpackPluginOptions = SentryCliPluginOptions;
export type SentryWebpackPlugin = { options: SentryWebpackPluginOptions };
export type SentryWebpackPlugin = WebpackPluginInstance & { options: SentryWebpackPluginOptions };

/**
* Overall Nextjs config
*/

export type ExportedNextConfig = NextConfigObject | NextConfigFunction;
export type ExportedNextConfig = Partial<NextConfigObject> | NextConfigFunction;

export type NextConfigObject = {
// custom webpack options
webpack?: WebpackConfigFunction;
webpack: WebpackConfigFunction;
// whether to build serverless functions for all pages, not just API routes
target?: 'server' | 'experimental-serverless-trace';
target: 'server' | 'experimental-serverless-trace';
sentry?: {
disableServerWebpackPlugin?: boolean;
disableClientWebpackPlugin?: boolean;
Expand All @@ -25,8 +26,8 @@ export type NextConfigObject = {

export type NextConfigFunction = (
phase: string,
defaults: { defaultConfig: { [key: string]: unknown } },
) => NextConfigObject;
defaults: { defaultConfig: NextConfigObject },
) => Partial<NextConfigObject>;

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

export type WebpackConfigObject = {
devtool?: string;
plugins?: Array<{ [key: string]: unknown }>;
plugins?: Array<WebpackPluginInstance | SentryWebpackPlugin>;
entry: WebpackEntryProperty;
output: { filename: string; path: string };
target: string;
Expand All @@ -56,7 +57,7 @@ export type BuildContext = {
isServer: boolean;
buildId: string;
dir: string;
config: Partial<NextConfigObject>;
config: NextConfigObject;
webpack: { version: string };
};

Expand Down
6 changes: 2 additions & 4 deletions packages/nextjs/src/config/webpack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { getSentryRelease } from '@sentry/node';
import { dropUndefinedKeys, logger } from '@sentry/utils';
import * as SentryWebpackPlugin from '@sentry/webpack-plugin';
import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin';
import * as fs from 'fs';
import * as path from 'path';

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

newConfig.plugins = newConfig.plugins || [];
newConfig.plugins.push(
// @ts-ignore Our types for the plugin are messed up somehow - TS wants this to be `SentryWebpackPlugin.default`,
// but that's not actually a thing
new SentryWebpackPlugin(getWebpackPluginOptions(buildContext, userSentryWebpackPluginOptions)),
);
}
Expand Down
76 changes: 55 additions & 21 deletions packages/nextjs/test/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import * as rimraf from 'rimraf';
import { WebpackPluginInstance } from 'webpack';

import { withSentryConfig } from '../src/config';
import {
BuildContext,
EntryPropertyFunction,
ExportedNextConfig,
NextConfigObject,
SentryWebpackPlugin as SentryWebpackPluginType,
SentryWebpackPluginOptions,
WebpackConfigObject,
} from '../src/config/types';
Expand Down Expand Up @@ -58,7 +58,7 @@ process.env.SENTRY_RELEASE = 'doGsaREgReaT';
const runtimePhase = 'ball-fetching';
// `defaultConfig` is the defaults for all nextjs options (we don't use these at all in the tests, so for our purposes
// here the values don't matter)
const defaultsObject = { defaultConfig: {} };
const defaultsObject = { defaultConfig: {} as NextConfigObject };

/** mocks of the arguments passed to `nextConfig.webpack` */
const serverWebpackConfig = {
Expand Down Expand Up @@ -103,7 +103,7 @@ function getBuildContext(
dev: false,
buildId: 'sItStAyLiEdOwN',
dir: '/Users/Maisey/projects/squirrelChasingSimulator',
config: { target: 'server', ...userNextConfig },
config: { target: 'server', ...userNextConfig } as NextConfigObject,
webpack: { version: webpackVersion },
isServer: buildTarget === 'server',
};
Expand Down Expand Up @@ -177,6 +177,14 @@ async function materializeFinalWebpackConfig(options: {
return finalWebpackConfigValue;
}

// helper function to make sure we're checking the correct plugin's data
export function findWebpackPlugin(
webpackConfig: WebpackConfigObject,
pluginName: string,
): WebpackPluginInstance | SentryWebpackPlugin | undefined {
return webpackConfig.plugins?.find(plugin => plugin.constructor.name === pluginName);
}

describe('withSentryConfig', () => {
it('includes expected properties', () => {
const finalConfig = materializeFinalNextConfig(userNextConfig);
Expand Down Expand Up @@ -335,8 +343,9 @@ describe('Sentry webpack plugin config', () => {
incomingWebpackConfig: serverWebpackConfig,
incomingWebpackBuildContext: serverBuildContext,
});
const sentryWebpackPluginInstance = findWebpackPlugin(finalWebpackConfig, 'SentryCliPlugin') as SentryWebpackPlugin;

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

expect((finalWebpackConfig.plugins?.[0].options as SentryWebpackPluginOptions).debug).toEqual(true);
expect(sentryWebpackPluginInstance.options.debug).toEqual(true);
});

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

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

const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
const sentryWebpackPluginInstance = findWebpackPlugin(
finalWebpackConfig,
'SentryCliPlugin',
) as SentryWebpackPlugin;

expect(sentryWebpackPlugin.options?.include).toEqual([
expect(sentryWebpackPluginInstance.options.include).toEqual([
{ paths: ['.next/server/pages/'], urlPrefix: '~/city-park/_next/server/pages' },
{ paths: ['.next/server/chunks/'], urlPrefix: '~/city-park/_next/server/chunks' },
]);
Expand Down
Loading