8000 ref(nextjs): Small improvements to config tests (#3938) · GinMu/sentry-javascript@9703ce2 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 9703ce2

Browse files
lobsterkatielobsterkatie
authored
ref(nextjs): Small improvements to config tests (getsentry#3938)
In the process of reviewing getsentry#3922, I realized that there was some cleanup work I could do to make that PR work better. Specifically: - `withSentryConfig` had an incorrect type for one of the arguments to the `userNextConfig` function. - The mock for that same argument was missing its outer layer, and therefore wasn't really what it said it was. - In real life, the `config` property of `buildContext` is equal to the combination of next's default config and the user-provided config. The mock wasn't reflecting this. - That PR was having to compensate for the fact that the mocks for build context were static rather than dynamic, even though those test cases especially illustrate the ways in which the value of the mock needs to change depending on other values in the test. Where necessary, the build context is now calculated as part of the test.
1 parent 9c516f5 commit 9703ce2

File tree

2< 8000 !-- --> files changed

+33
-27
lines changed

2 files changed

+33
-27
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export function withSentryConfig(
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: { [key: string]: unknown } }): NextConfigObject {
18+
return function(phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject {
1919
const materializedUserNextConfig = userNextConfig(phase, defaults);
2020
return {
2121
...materializedUserNextConfig,

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

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const mockExistsSync = (path: fs.PathLike) => {
3333
const exitsSync = jest.spyOn(fs, 'existsSync').mockImplementation(mockExistsSync);
3434

3535
/** Mocks of the arguments passed to `withSentryConfig` */
36-
const userNextConfig = {
36+
const userNextConfig: Partial<NextConfigObject> = {
3737
publicRuntimeConfig: { location: 'dogpark', activities: ['fetch', 'chasing', 'digging'] },
3838
webpack: (config: WebpackConfigObject, _options: BuildContext) => ({
3939
...config,
@@ -51,7 +51,9 @@ process.env.SENTRY_RELEASE = 'doGsaREgReaT';
5151

5252
/** Mocks of the arguments passed to the result of `withSentryConfig` (when it's a function). */
5353
const runtimePhase = 'ball-fetching';
54-
const defaultNextConfig = { nappingHoursPerDay: 20, oversizeFeet: true, shouldChaseTail: true };
54+
// `defaultConfig` is the defaults for all nextjs options (we don't use these at all in the tests, so for our purposes
55+
// here the values don't matter)
56+
const defaultsObject = { defaultConfig: {} };
5557

5658
/** mocks of the arguments passed to `nextConfig.webpack` */
5759
const serverWebpackConfig = {
@@ -84,15 +86,21 @@ const clientWebpackConfig = {
8486
context: '/Users/Maisey/projects/squirrelChasingSimulator',
8587
};
8688

87-
const baseBuildContext = {
88-
dev: false,
89-
buildId: 'sItStAyLiEdOwN',
90-
dir: '/Users/Maisey/projects/squirrelChasingSimulator',
91-
config: { target: 'server' as const },
92-
webpack: { version: '5.4.15' },
93-
};
94-
const serverBuildContext = { isServer: true, ...baseBuildContext };
95-
const clientBuildContext = { isServer: false, ...baseBuildContext };
89+
// In real life, next will copy the `userNextConfig` into the `buildContext`. Since we're providing mocks for both of
90+
// those, we need to mimic that behavior, and since `userNextConfig` can vary per test, we need to have the option do it
91+
// dynamically.
92+
function getBuildContext(buildTarget: 'server' | 'client', userNextConfig: Partial<NextConfigObject>): BuildContext {
93+
return {
94+
dev: false,
95+
buildId: 'sItStAyLiEdOwN',
96+
dir: '/Users/Maisey/projects/squirrelChasingSimulator',
97+
config: { target: 'server', ...userNextConfig },
98+
webpack: { version: '5.4.15' },
99+
isServer: buildTarget === 'server',
100+
};
101+
}
102+
const serverBuildContext = getBuildContext('server', userNextConfig);
103+
const clientBuildContext = getBuildContext('client', userNextConfig);
96104

97105
/**
98106
* Derive the final values of all next config options, by first applying `withSentryConfig` and then, if it returns a
@@ -114,9 +122,7 @@ function materializeFinalNextConfig(
114122
if (typeof sentrifiedConfig === 'function') {
115123
// for some reason TS won't recognize that `finalConfigValues` is now a NextConfigObject, which is why the cast
116124
// below is necessary
117-
finalConfigValues = sentrifiedConfig(runtimePhase, {
118-
defaultConfig: defaultNextConfig,
119-
});
125+
finalConfigValues = sentrifiedConfig(runtimePhase, defaultsObject);
120126
}
121127

122128
return finalConfigValues as NextConfigObject;
@@ -145,11 +151,7 @@ async function materializeFinalWebpackConfig(options: {
145151

146152
// if the user's next config is a function, run it so we have access to the values
147153
const materializedUserNextConfig =
148-
typeof userNextConfig === 'function'
149-
? userNextConfig('phase-production-build', {
150-
defaultConfig: {},
151-
})
152-
: userNextConfig;
154+
typeof userNextConfig === 'function' ? userNextConfig('phase-production-build', defaultsObject) : userNextConfig;
153155

154156
// get the webpack config function we'd normally pass back to next
155157
const webpackConfigFunction = constructWebpackConfigFunction(
@@ -211,9 +213,7 @@ describe('withSentryConfig', () => {
211213

212214
materializeFinalNextConfig(userNextConfigFunction);
213215

214-
expect(userNextConfigFunction).toHaveBeenCalledWith(runtimePhase, {
215-
defaultConfig: defaultNextConfig,
216-
});
216+
expect(userNextConfigFunction).toHaveBeenCalledWith(runtimePhase, defaultsObject);
217217
});
218218
});
219219

@@ -243,7 +243,7 @@ describe('webpack config', () => {
243243

244244
// Run the user's webpack config function, so we can check the results against ours. Delete `entry` because we'll
245245
// test it separately, and besides, it's one that we *should* be overwriting.
246-
const materializedUserWebpackConfig = userNextConfig.webpack(serverWebpackConfig, serverBuildContext);
246+
const materializedUserWebpackConfig = userNextConfig.webpack!(serverWebpackConfig, serverBuildContext);
247247
// @ts-ignore `entry` may be required in real life, but we don't need it for our tests
248248
delete materializedUserWebpackConfig.entry;
249249

@@ -377,10 +377,13 @@ describe('Sentry webpack plugin config', () => {
377377
});
378378

379379
it('has the correct value when building serverless server bundles', async () => {
380+
const userNextConfigServerless = { ...userNextConfig };
381+
userNextConfigServerless.target = 'experimental-serverless-trace';
382+
380383
const finalWebpackConfig = await materializeFinalWebpackConfig({
381-
userNextConfig,
384+
userNextConfig: userNextConfigServerless,
382385
incomingWebpackConfig: serverWebpackConfig,
383-
incomingWebpackBuildContext: { ...serverBuildContext, config: { target: 'experimental-serverless-trace' } },
386+
incomingWebpackBuildContext: getBuildContext('server', userNextConfigServerless),
384387
});
385388

386389
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;
@@ -391,10 +394,13 @@ describe('Sentry webpack plugin config', () => {
391394
});
392395

393396
it('has the correct value when building serverful server bundles using webpack 4', async () => {
397+
const serverBuildContextWebpack4 = getBuildContext('server', userNextConfig);
398+
serverBuildContextWebpack4.webpack.version = '4.15.13';
399+
394400
const finalWebpackConfig = await materializeFinalWebpackConfig({
395401
userNextConfig,
396402
incomingWebpackConfig: serverWebpackConfig,
397-
incomingWebpackBuildContext: { ...serverBuildContext, webpack: { version: '4.15.13' } },
403+
incomingWebpackBuildContext: serverBuildContextWebpack4,
398404
});
399405

400406
const sentryWebpackPlugin = finalWebpackConfig.plugins?.[0] as SentryWebpackPluginType;

0 commit comments

Comments
 (0)
0