diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 0134c41ca665..7ee3e3b83c41 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -1,14 +1,7 @@ /* eslint-disable complexity */ /* eslint-disable max-lines */ import { getSentryRelease } from '@sentry/node'; -import { - arrayify, - dropUndefinedKeys, - escapeStringForRegex, - GLOBAL_OBJ, - logger, - stringMatchesSomePattern, -} from '@sentry/utils'; +import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils'; import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin'; import * as chalk from 'chalk'; import * as fs from 'fs'; @@ -27,7 +20,6 @@ import type { WebpackConfigObjectWithModuleRules, WebpackEntryProperty, WebpackModuleRule, - WebpackPluginInstance, } from './types'; export { SentryWebpackPlugin }; @@ -36,14 +28,6 @@ export { SentryWebpackPlugin }; // TODO: merge default SentryWebpackPlugin include with their SentryWebpackPlugin include // TODO: drop merged keys from override check? `includeDefaults` option? -// In order to make sure that build-time code isn't getting bundled in runtime bundles (specifically, in the serverless -// functions into which nextjs converts a user's routes), we exclude this module (and all of its dependencies) from the -// nft file manifests nextjs generates. (See the code dealing with the `TraceEntryPointsPlugin` below.) So that we don't -// crash people, we therefore need to make sure nothing tries to either require this file or import from it. Setting -// this env variable allows us to test whether or not that's happened. -const _global = GLOBAL_OBJ as typeof GLOBAL_OBJ & { _sentryWebpackModuleLoaded?: true }; -_global._sentryWebpackModuleLoaded = true; - /** * Construct the function which will be used as the nextjs config's `webpack` value. * @@ -159,31 +143,6 @@ export function constructWebpackConfigFunction( ], }); } - - // Prevent `@vercel/nft` (which nextjs uses to determine which files are needed when packaging up a lambda) from - // including any of our build-time code or dependencies. (Otherwise it'll include files like this one and even the - // entirety of rollup and sucrase.) Since this file is the root of that dependency tree, it's enough to just - // exclude it and the rest will be excluded as well. - const nftPlugin = newConfig.plugins?.find((plugin: WebpackPluginInstance) => { - const proto = Object.getPrototypeOf(plugin) as WebpackPluginInstance; - return proto.constructor.name === 'TraceEntryPointsPlugin'; - }) as WebpackPluginInstance & { excludeFiles: string[] }; - if (nftPlugin) { - if (Array.isArray(nftPlugin.excludeFiles)) { - nftPlugin.excludeFiles.push(__filename); - } else { - __DEBUG_BUILD__ && - logger.warn( - 'Unable to exclude Sentry build-time helpers from nft files. `TraceEntryPointsPlugin.excludeFiles` is not ' + - 'an array. This is a bug; please report this to Sentry: https://github.com/getsentry/sentry-javascript/issues/.', - ); - } - } else { - __DEBUG_BUILD__ && - logger.warn( - 'Unable to exclude Sentry build-time helpers from nft files. Could not find `TraceEntryPointsPlugin`.', - ); - } } // The SDK uses syntax (ES6 and ES6+ features like object spread) which isn't supported by older browsers. For users diff --git a/packages/nextjs/src/config/withSentryConfig.ts b/packages/nextjs/src/config/withSentryConfig.ts index f10f801e1702..a27597669c3a 100644 --- a/packages/nextjs/src/config/withSentryConfig.ts +++ b/packages/nextjs/src/config/withSentryConfig.ts @@ -1,5 +1,3 @@ -import { PHASE_DEVELOPMENT_SERVER, PHASE_PRODUCTION_BUILD } from 'next/constants'; - import type { ExportedNextConfig, NextConfigFunction, @@ -8,6 +6,7 @@ import type { SentryWebpackPluginOptions, UserSentryOptions, } from './types'; +import { constructWebpackConfigFunction } from './webpack'; /** * Add Sentry options to the config to be exported from the user's `next.config.js` file. @@ -22,49 +21,44 @@ export function withSentryConfig( userSentryWebpackPluginOptions: Partial = {}, sentryOptions?: UserSentryOptions, ): NextConfigFunction | NextConfigObject { - return function (phase: string, defaults: { defaultConfig: NextConfigObject }): NextConfigObject { - const userNextConfigObject = - typeof exportedUserNextConfig === 'function' ? exportedUserNextConfig(phase, defaults) : exportedUserNextConfig; - // Inserts additional `sentry` options into the existing config, allows for backwards compatability - // in case nothing is passed into the optional `sentryOptions` argument - userNextConfigObject.sentry = { ...userNextConfigObject.sentry, ...sentryOptions }; - return getFinalConfigObject(phase, userNextConfigObject, userSentryWebpackPluginOptions); - }; + if (typeof exportedUserNextConfig === 'function') { + return function (this: unknown, ...webpackConfigFunctionArgs: unknown[]): NextConfigObject { + const userNextConfigObject: NextConfigObjectWithSentry = exportedUserNextConfig.apply( + this, + webpackConfigFunctionArgs, + ); + + const userSentryOptions = { ...userNextConfigObject.sentry, ...sentryOptions }; + return getFinalConfigObject(userNextConfigObject, userSentryOptions, userSentryWebpackPluginOptions); + }; + } else { + const userSentryOptions = { ...exportedUserNextConfig.sentry, ...sentryOptions }; + return getFinalConfigObject(exportedUserNextConfig, userSentryOptions, userSentryWebpackPluginOptions); + } } // Modify the materialized object form of the user's next config by deleting the `sentry` property and wrapping the // `webpack` property function getFinalConfigObject( - phase: string, incomingUserNextConfigObject: NextConfigObjectWithSentry, + userSentryOptions: UserSentryOptions, userSentryWebpackPluginOptions: Partial, ): NextConfigObject { - // Next 12.2.3+ warns about non-canonical properties on `userNextConfig`, so grab and then remove the `sentry` - // property there. Where we actually need it is in the webpack config function we're going to create, so pass it - // to `constructWebpackConfigFunction` so that it can live in the returned function's closure. - const { sentry: userSentryOptions } = incomingUserNextConfigObject; + // Next 12.2.3+ warns about non-canonical properties on `userNextConfig`. delete incomingUserNextConfigObject.sentry; - // Remind TS that there's now no `sentry` property - const userNextConfigObject = incomingUserNextConfigObject as NextConfigObject; if (userSentryOptions?.tunnelRoute) { - setUpTunnelRewriteRules(userNextConfigObject, userSentryOptions.tunnelRoute); + setUpTunnelRewriteRules(incomingUserNextConfigObject, userSentryOptions.tunnelRoute); } - // In order to prevent all of our build-time code from being bundled in people's route-handling serverless functions, - // we exclude `webpack.ts` and all of its dependencies from nextjs's `@vercel/nft` filetracing. We therefore need to - // make sure that we only require it at build time or in development mode. - if (phase === PHASE_PRODUCTION_BUILD || phase === PHASE_DEVELOPMENT_SERVER) { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { constructWebpackConfigFunction } = require('./webpack'); - return { - ...userNextConfigObject, - webpack: constructWebpackConfigFunction(userNextConfigObject, userSentryWebpackPluginOptions, userSentryOptions), - }; - } - - // At runtime, we just return the user's config untouched. - return userNextConfigObject; + return { + ...incomingUserNextConfigObject, + webpack: constructWebpackConfigFunction( + incomingUserNextConfigObject, + userSentryWebpackPluginOptions, + userSentryOptions, + ), + }; } /** diff --git a/packages/nextjs/test/config/withSentryConfig.test.ts b/packages/nextjs/test/config/withSentryConfig.test.ts index 3bf70a5107f8..4ed6f2d40770 100644 --- a/packages/nextjs/test/config/withSentryConfig.test.ts +++ b/packages/nextjs/test/config/withSentryConfig.test.ts @@ -59,46 +59,4 @@ describe('withSentryConfig', () => { // directly expect('sentry' in finalConfig).toBe(false); }); - - describe('conditional use of `constructWebpackConfigFunction`', () => { - // Note: In these tests, it would be nice to be able to spy on `constructWebpackConfigFunction` to see whether or - // not it's called, but that sets up a catch-22: If you import or require the module to spy on the function, it gets - // cached and the `require` call we care about (inside of `withSentryConfig`) doesn't actually run the module code. - // Alternatively, if we call `jest.resetModules()` after setting up the spy, then the module code *is* run a second - // time, but the spy belongs to the first instance of the module and therefore never registers a call. Thus we have - // to test whether or not the file is required instead. - - it('imports from `webpack.ts` if build phase is "phase-production-build"', () => { - jest.isolateModules(() => { - // In case this is still set from elsewhere, reset it - delete (global as any)._sentryWebpackModuleLoaded; - - materializeFinalNextConfig(exportedNextConfig, undefined, 'phase-production-build'); - - expect((global as any)._sentryWebpackModuleLoaded).toBe(true); - }); - }); - - it('imports from `webpack.ts` if build phase is "phase-development-server"', () => { - jest.isolateModules(() => { - // In case this is still set from elsewhere, reset it - delete (global as any)._sentryWebpackModuleLoaded; - - materializeFinalNextConfig(exportedNextConfig, undefined, 'phase-production-build'); - - expect((global as any)._sentryWebpackModuleLoaded).toBe(true); - }); - }); - - it('Doesn\'t import from `webpack.ts` if build phase is "phase-production-server"', () => { - jest.isolateModules(() => { - // In case this is still set from elsewhere, reset it - delete (global as any)._sentryWebpackModuleLoaded; - - materializeFinalNextConfig(exportedNextConfig, undefined, 'phase-production-server'); - - expect((global as any)._sentryWebpackModuleLoaded).toBeUndefined(); - }); - }); - }); });