From 1e7920e564f75e23d2ae7d67348fdbb0fdf6a15b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 30 Aug 2021 23:02:18 -0700 Subject: [PATCH 1/5] use correct type when userNextConfig is a function --- packages/nextjs/src/config/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/config/index.ts b/packages/nextjs/src/config/index.ts index 499c96805d9d..5cc4d5964893 100644 --- a/packages/nextjs/src/config/index.ts +++ b/packages/nextjs/src/config/index.ts @@ -15,7 +15,7 @@ export function withSentryConfig( // 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: { [key: string]: unknown } }): NextConfigObject { + return function(phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject { const materializedUserNextConfig = userNextConfig(phase, defaults); return { ...materializedUserNextConfig, From 4317b02bdf6ab1c8873beb0c63729ec14c4f9734 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 30 Aug 2021 23:03:12 -0700 Subject: [PATCH 2/5] add type to userNextConfig mock value --- packages/nextjs/test/config.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/test/config.test.ts b/packages/nextjs/test/config.test.ts index 29e575452ed6..9181c1b59c2d 100644 --- a/packages/nextjs/test/config.test.ts +++ b/packages/nextjs/test/config.test.ts @@ -33,7 +33,7 @@ const mockExistsSync = (path: fs.PathLike) => { const exitsSync = jest.spyOn(fs, 'existsSync').mockImplementation(mockExistsSync); /** Mocks of the arguments passed to `withSentryConfig` */ -const userNextConfig = { +const userNextConfig: Partial = { publicRuntimeConfig: { location: 'dogpark', activities: ['fetch', 'chasing', 'digging'] }, webpack: (config: WebpackConfigObject, _options: BuildContext) => ({ ...config, @@ -243,7 +243,7 @@ describe('webpack config', () => { // Run the user's webpack config function, so we can check the results against ours. Delete `entry` because we'll // test it separately, and besides, it's one that we *should* be overwriting. - const materializedUserWebpackConfig = userNextConfig.webpack(serverWebpackConfig, serverBuildContext); + const materializedUserWebpackConfig = userNextConfig.webpack!(serverWebpackConfig, serverBuildContext); // @ts-ignore `entry` may be required in real life, but we don't need it for our tests delete materializedUserWebpackConfig.entry; From 0daa465f94702867b0729071c918e1bf124bfed9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 30 Aug 2021 23:04:28 -0700 Subject: [PATCH 3/5] make mock for functional userNextConfig's second argument include full object --- packages/nextjs/test/config.test.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/nextjs/test/config.test.ts b/packages/nextjs/test/config.test.ts index 9181c1b59c2d..f0026c59811e 100644 --- a/packages/nextjs/test/config.test.ts +++ b/packages/nextjs/test/config.test.ts @@ -51,7 +51,9 @@ process.env.SENTRY_RELEASE = 'doGsaREgReaT'; /** Mocks of the arguments passed to the result of `withSentryConfig` (when it's a function). */ const runtimePhase = 'ball-fetching'; -const defaultNextConfig = { nappingHoursPerDay: 20, oversizeFeet: true, shouldChaseTail: true }; +// `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: {} }; /** mocks of the arguments passed to `nextConfig.webpack` */ const serverWebpackConfig = { @@ -114,9 +116,7 @@ function materializeFinalNextConfig( if (typeof sentrifiedConfig === 'function') { // for some reason TS won't recognize that `finalConfigValues` is now a NextConfigObject, which is why the cast // below is necessary - finalConfigValues = sentrifiedConfig(runtimePhase, { - defaultConfig: defaultNextConfig, - }); + finalConfigValues = sentrifiedConfig(runtimePhase, defaultsObject); } return finalConfigValues as NextConfigObject; @@ -145,11 +145,7 @@ async function materializeFinalWebpackConfig(options: { // if the user's next config is a function, run it so we have access to the values const materializedUserNextConfig = - typeof userNextConfig === 'function' - ? userNextConfig('phase-production-build', { - defaultConfig: {}, - }) - : userNextConfig; + typeof userNextConfig === 'function' ? userNextConfig('phase-production-build', defaultsObject) : userNextConfig; // get the webpack config function we'd normally pass back to next const webpackConfigFunction = constructWebpackConfigFunction( @@ -211,9 +207,7 @@ describe('withSentryConfig', () => { materializeFinalNextConfig(userNextConfigFunction); - expect(userNextConfigFunction).toHaveBeenCalledWith(runtimePhase, { - defaultConfig: defaultNextConfig, - }); + expect(userNextConfigFunction).toHaveBeenCalledWith(runtimePhase, defaultsObject); }); }); From 2b7e1cfa2266c6652badad759cd24d32bb8912b9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 30 Aug 2021 23:05:44 -0700 Subject: [PATCH 4/5] let mock build context be dynamic --- packages/nextjs/test/config.test.ts | 37 +++++++++++++++++++---------- 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/nextjs/test/config.test.ts b/packages/nextjs/test/config.test.ts index f0026c59811e..e3af19bc68df 100644 --- a/packages/nextjs/test/config.test.ts +++ b/packages/nextjs/test/config.test.ts @@ -86,15 +86,22 @@ const clientWebpackConfig = { context: '/Users/Maisey/projects/squirrelChasingSimulator', }; -const baseBuildContext = { - dev: false, - buildId: 'sItStAyLiEdOwN', - dir: '/Users/Maisey/projects/squirrelChasingSimulator', - config: { target: 'server' as const }, - webpack: { version: '5.4.15' }, -}; -const serverBuildContext = { isServer: true, ...baseBuildContext }; -const clientBuildContext = { isServer: false, ...baseBuildContext }; +// In real life, next will copy the userNextConfig into the buildContext. Since we're providing mocks for both of +// those, though, we need to do that manually. +function getBuildContext(buildTarget: 'server' | 'client', userNextConfig: Partial): BuildContext { + const baseBuildContext = { + dev: false, + buildId: 'sItStAyLiEdOwN', + dir: '/Users/Maisey/projects/squirrelChasingSimulator', + config: { target: 'server', ...userNextConfig }, + webpack: { version: '5.4.15' }, + }; + const isServer = buildTarget === 'server'; + + return { isServer, ...baseBuildContext } as BuildContext; +} +const serverBuildContext = getBuildContext('server', userNextConfig); +const clientBuildContext = getBuildContext('client', userNextConfig); /** * Derive the final values of all next config options, by first applying `withSentryConfig` and then, if it returns a @@ -371,10 +378,13 @@ describe('Sentry webpack plugin config', () => { }); it('has the correct value when building serverless server bundles', async () => { + const userNextConfigServerless = { ...userNextConfig }; + userNextConfigServerless.target = 'experimental-serverless-trace'; + const finalWebpackConfig = await materializeFinalWebpackConfig({ - userNextConfig, + userNextConfig: userNextConfigServerless, incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: { ...serverBuildContext, config: { target: 'experimental-serverless-trace' } }, + incomingWebpackBuildContext: getBuildContext('server', userNextConfigServerless), }); const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType; @@ -385,10 +395,13 @@ describe('Sentry webpack plugin config', () => { }); it('has the correct value when building serverful server bundles using webpack 4', async () => { + const serverBuildContextWebpack4 = getBuildContext('server', userNextConfig); + serverBuildContextWebpack4.webpack.version = '4.15.13'; + const finalWebpackConfig = await materializeFinalWebpackConfig({ userNextConfig, incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: { ...serverBuildContext, webpack: { version: '4.15.13' } }, + incomingWebpackBuildContext: serverBuildContextWebpack4, }); const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType; From 544d299204e4e2566f313481a7e5cc9103c6fd87 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 31 Aug 2021 06:35:56 -0700 Subject: [PATCH 5/5] apply review suggestion and clarify comment --- packages/nextjs/test/config.test.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/nextjs/test/config.test.ts b/packages/nextjs/test/config.test.ts index e3af19bc68df..eda16631d1a0 100644 --- a/packages/nextjs/test/config.test.ts +++ b/packages/nextjs/test/config.test.ts @@ -86,19 +86,18 @@ const clientWebpackConfig = { context: '/Users/Maisey/projects/squirrelChasingSimulator', }; -// In real life, next will copy the userNextConfig into the buildContext. Since we're providing mocks for both of -// those, though, we need to do that manually. +// 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): BuildContext { - const baseBuildContext = { + return { dev: false, buildId: 'sItStAyLiEdOwN', dir: '/Users/Maisey/projects/squirrelChasingSimulator', config: { target: 'server', ...userNextConfig }, webpack: { version: '5.4.15' }, + isServer: buildTarget === 'server', }; - const isServer = buildTarget === 'server'; - - return { isServer, ...baseBuildContext } as BuildContext; } const serverBuildContext = getBuildContext('server', userNextConfig); const clientBuildContext = getBuildContext('client', userNextConfig);