From 39635591075cb94f69209bae95f05618dcd83b04 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 09:03:56 +0000 Subject: [PATCH 1/8] YEET --- .../src/config/loaders/wrappingLoader.ts | 20 ++- packages/nextjs/src/config/webpack.ts | 160 +----------------- 2 files changed, 14 insertions(+), 166 deletions(-) diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 3157d41df71f..8f6ec9715b2d 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -201,21 +201,23 @@ export default function wrappingLoader( } else { templateCode = templateCode.replace(/__COMPONENT_TYPE__/g, 'Unknown'); } - - // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, - // however we can only create relative paths to the sentry config from absolute paths.Examples where this could possibly be non - absolute are virtual modules. - if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { - const sentryConfigImportPath = path - .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 - .replace(/\\/g, '/'); - templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); - } } else if (wrappingTargetKind === 'middleware') { templateCode = middlewareWrapperTemplateCode; } else { throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`); } + // Inject init code from `sentry.*.config` files into the wrapping template + if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { + // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, + // however we can only create relative paths to the sentry config from absolute paths. + // Examples where this could possibly be non - absolute are virtual modules. + const sentryConfigImportPath = path + .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 + .replace(/\\/g, '/'); + templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); + } + // Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand. templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME); diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 0bb42f98b7ec..dd4470207085 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -1,7 +1,7 @@ /* eslint-disable complexity */ /* eslint-disable max-lines */ import { getSentryRelease } from '@sentry/node'; -import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils'; +import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger } from '@sentry/utils'; import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin'; import * as chalk from 'chalk'; import * as fs from 'fs'; @@ -12,14 +12,12 @@ import type { VercelCronsConfig } from '../common/types'; // circular dependency check thinks this file is importing from itself. See https://github.com/pahen/madge/issues/306. import type { BuildContext, - EntryPropertyObject, NextConfigObject, SentryWebpackPluginOptions, UserSentryOptions, WebpackConfigFunction, WebpackConfigObject, WebpackConfigObjectWithModuleRules, - WebpackEntryProperty, WebpackModuleRule, } from './types'; @@ -291,8 +289,8 @@ export function constructWebpackConfigFunction( // than its original value. So calling it will call the callback which will call `f` which will call `x.y` which // will call the callback which will call `f` which will call `x.y`... and on and on. Theoretically this could also // be fixed by using `bind`, but this is way simpler.) - const origEntryProperty = newConfig.entry; - newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext, userSentryOptions); + // const origEntryProperty = newConfig.entry; + // newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext, userSentryOptions); // Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default if (shouldEnableWebpackPlugin(buildContext, userSentryOptions)) { @@ -402,63 +400,6 @@ function findTranspilationRules(rules: WebpackModuleRule[] | undefined, projectD return matchingRules; } -/** - * Modify the webpack `entry` property so that the code in `sentry.server.config.js` and `sentry.client.config.js` is - * included in the the necessary bundles. - * - * @param currentEntryProperty The value of the property before Sentry code has been injected - * @param buildContext Object passed by nextjs containing metadata about the build - * @returns The value which the new `entry` property (which will be a function) will return (TODO: this should return - * the function, rather than the function's return value) - */ -async function addSentryToEntryProperty( - currentEntryProperty: WebpackEntryProperty, - buildContext: BuildContext, - userSentryOptions: UserSentryOptions, -): Promise { - // The `entry` entry in a webpack config can be a string, array of strings, object, or function. By default, nextjs - // sets it to an async function which returns the promise of an object of string arrays. Because we don't know whether - // someone else has come along before us and changed that, we need to check a few things along the way. The one thing - // we know is that it won't have gotten *simpler* in form, so we only need to worry about the object and function - // options. See https://webpack.js.org/configuration/entry-context/#entry. - - const { isServer, dir: projectDir, nextRuntime } = buildContext; - const runtime = isServer ? (buildContext.nextRuntime === 'edge' ? 'edge' : 'node') : 'browser'; - - const newEntryProperty = - typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty }; - - // `sentry.server.config.js` or `sentry.client.config.js` (or their TS equivalents) - const userConfigFile = - nextRuntime === 'edge' - ? getUserConfigFile(projectDir, 'edge') - : isServer - ? getUserConfigFile(projectDir, 'server') - : getUserConfigFile(projectDir, 'client'); - - // we need to turn the filename into a path so webpack can find it - const filesToInject = userConfigFile ? [`./${userConfigFile}`] : []; - - // inject into all entry points which might contain user's code - for (const entryPointName in newEntryProperty) { - if (shouldAddSentryToEntryPoint(entryPointName, runtime, userSentryOptions.excludeServerRoutes ?? [])) { - addFilesToExistingEntryPoint(newEntryProperty, entryPointName, filesToInject); - } else { - if ( - isServer && - // If the user has asked to exclude pages, confirm for them that it's worked - userSentryOptions.excludeServerRoutes && - // We always skip these, so it's not worth telling the user that we've done so - !['pages/_app', 'pages/_document'].includes(entryPointName) - ) { - __DEBUG_BUILD__ && logger.log(`Skipping Sentry injection for ${entryPointName.replace(/^pages/, '')}`); - } - } - } - - return newEntryProperty; -} - /** * Search the project directory for a valid user config file for the given platform, allowing for it to be either a * TypeScript or JavaScript file. @@ -505,58 +446,6 @@ export function getUserConfigFilePath(projectDir: string, platform: 'server' | ' return undefined; } -/** - * Add files to a specific element of the given `entry` webpack config property. - * - * @param entryProperty The existing `entry` config object - * @param entryPointName The key where the file should be injected - * @param filepaths An array of paths to the injected files - */ -function addFilesToExistingEntryPoint( - entryProperty: EntryPropertyObject, - entryPointName: string, - filepaths: string[], -): void { - // can be a string, array of strings, or object whose `import` property is one of those two - const currentEntryPoint = entryProperty[entryPointName]; - let newEntryPoint = currentEntryPoint; - - if (typeof currentEntryPoint === 'string') { - newEntryPoint = [...filepaths, currentEntryPoint]; - } else if (Array.isArray(currentEntryPoint)) { - newEntryPoint = [...filepaths, ...currentEntryPoint]; - } - // descriptor object (webpack 5+) - else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) { - const currentImportValue = currentEntryPoint.import; - let newImportValue; - - if (typeof currentImportValue === 'string') { - newImportValue = [...filepaths, currentImportValue]; - } else { - newImportValue = [...filepaths, ...currentImportValue]; - } - - newEntryPoint = { - ...currentEntryPoint, - import: newImportValue, - }; - } - // malformed entry point (use `console.error` rather than `logger.error` because it will always be printed, regardless - // of SDK settings) - else { - // eslint-disable-next-line no-console - console.error( - 'Sentry Logger [Error]:', - `Could not inject SDK initialization code into entry point ${entryPointName}, as its current value is not in a recognized format.\n`, - 'Expected: string | Array | { [key:string]: any, import: string | Array }\n', - `Got: ${currentEntryPoint}`, - ); - } - - entryProperty[entryPointName] = newEntryPoint; -} - /** * Check the SentryWebpackPlugin options provided by the user against the options we set by default, and warn if any of * our default options are getting overridden. (Note: If any of our default values is undefined, it won't be included in @@ -581,49 +470,6 @@ function checkWebpackPluginOverrides( } } -/** - * Determine if this is an entry point into which both `Sentry.init()` code and the release value should be injected - * - * @param entryPointName The name of the entry point in question - * @param isServer Whether or not this function is being called in the context of a server build - * @param excludeServerRoutes A list of excluded serverside entrypoints provided by the user - * @returns `true` if sentry code should be injected, and `false` otherwise - */ -function shouldAddSentryToEntryPoint( - entryPointName: string, - runtime: 'node' | 'browser' | 'edge', - excludeServerRoutes: Array, -): boolean { - // On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions). - if (runtime === 'node') { - // User-specified pages to skip. (Note: For ease of use, `excludeServerRoutes` is specified in terms of routes, - // which don't have the `pages` prefix.) - const entryPointRoute = entryPointName.replace(/^pages/, ''); - if (stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true)) { - return false; - } - - // This expression will implicitly include `pages/_app` which is called for all serverside routes and pages - // regardless whether or not the user has a`_app` file. - return entryPointName.startsWith('pages/'); - } else if (runtime === 'browser') { - return ( - // entrypoint for `/pages` pages - this is included on all clientside pages - // It's important that we inject the SDK into this file and not into 'main' because in 'main' - // some important Next.js code (like the setup code for getCongig()) is located and some users - // may need this code inside their Sentry configs - entryPointName === 'pages/_app' || - // entrypoint for `/app` pages - entryPointName === 'main-app' - ); - } else { - // User-specified pages to skip. (Note: For ease of use, `excludeServerRoutes` is specified in terms of routes, - // which don't have the `pages` prefix.) - const entryPointRoute = entryPointName.replace(/^pages/, ''); - return !stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true); - } -} - /** * Combine default and user-provided SentryWebpackPlugin options, accounting for whether we're building server files or * client files. From 10bc5b3d9070b04a3fd1be4aa9db8880f983663a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 10:44:31 +0000 Subject: [PATCH 2/8] Separate loader --- .../config/loaders/configInjectionLoader.ts | 31 +++++++++++++++++++ packages/nextjs/src/config/loaders/index.ts | 1 + .../src/config/loaders/wrappingLoader.ts | 13 -------- packages/nextjs/src/config/webpack.ts | 27 +++++++++++++++- 4 files changed, 58 insertions(+), 14 deletions(-) create mode 100644 packages/nextjs/src/config/loaders/configInjectionLoader.ts diff --git a/packages/nextjs/src/config/loaders/configInjectionLoader.ts b/packages/nextjs/src/config/loaders/configInjectionLoader.ts new file mode 100644 index 000000000000..e4f76141d701 --- /dev/null +++ b/packages/nextjs/src/config/loaders/configInjectionLoader.ts @@ -0,0 +1,31 @@ +import * as path from 'path'; + +import type { LoaderThis } from './types'; + +type LoaderOptions = { + sentryConfigFilePath: string; +}; + +/** + * Injects `sentry.*.config.ts` and `sentry.*.config.ts` files into user code. + */ +export default function configInjectionLoader(this: LoaderThis, userCode: string): string { + // We know one or the other will be defined, depending on the version of webpack being used + const { sentryConfigFilePath } = 'getOptions' in this ? this.getOptions() : this.query; + + // We do not want to cache injected inits across builds + this.cacheable(false); + + // Inject init code from `sentry.*.config` files into the wrapping template + if (path.isAbsolute(this.resourcePath)) { + // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, + // however we can only create relative paths to the sentry config from absolute paths. + // Examples where this could possibly be non - absolute are virtual modules. + const sentryConfigImportPath = path + .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 + .replace(/\\/g, '/'); + return `import "${sentryConfigImportPath}";\n`.concat(userCode); + } else { + return userCode; + } +} diff --git a/packages/nextjs/src/config/loaders/index.ts b/packages/nextjs/src/config/loaders/index.ts index 27620e004f39..56d8c599c753 100644 --- a/packages/nextjs/src/config/loaders/index.ts +++ b/packages/nextjs/src/config/loaders/index.ts @@ -1,4 +1,5 @@ export { default as valueInjectionLoader } from './valueInjectionLoader'; export { default as prefixLoader } from './prefixLoader'; export { default as wrappingLoader } from './wrappingLoader'; +export { default as configInjectionLoader } from './configInjectionLoader'; export { default as sdkMultiplexerLoader } from './sdkMultiplexerLoader'; diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 8f6ec9715b2d..d20fa5d9e1d8 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -44,7 +44,6 @@ type LoaderOptions = { pageExtensionRegex: string; excludeServerRoutes: Array; wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component'; - sentryConfigFilePath?: string; vercelCronsConfig?: VercelCronsConfig; }; @@ -75,7 +74,6 @@ export default function wrappingLoader( pageExtensionRegex, excludeServerRoutes = [], wrappingTargetKind, - sentryConfigFilePath, vercelCronsConfig, } = 'getOptions' in this ? this.getOptions() : this.query; @@ -207,17 +205,6 @@ export default function wrappingLoader( throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`); } - // Inject init code from `sentry.*.config` files into the wrapping template - if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { - // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, - // however we can only create relative paths to the sentry config from absolute paths. - // Examples where this could possibly be non - absolute are virtual modules. - const sentryConfigImportPath = path - .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 - .replace(/\\/g, '/'); - templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); - } - // Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand. templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME); diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index dd4470207085..5885205045c3 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -123,7 +123,6 @@ export function constructWebpackConfigFunction( pagesDir: pagesDirPath, pageExtensionRegex, excludeServerRoutes: userSentryOptions.excludeServerRoutes, - sentryConfigFilePath: getUserConfigFilePath(projectDir, runtime), }; const normalizeLoaderResourcePath = (resourcePath: string): string => { @@ -253,6 +252,32 @@ export function constructWebpackConfigFunction( }); } + const sentryConfigFilePath = getUserConfigFilePath(projectDir, runtime); + if (sentryConfigFilePath) { + newConfig.module.rules.unshift({ + test: resourcePath => { + const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); + const isPagesRouterComponent = + normalizedAbsoluteResourcePath.startsWith(pagesDirPath) && + dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)); + const isAppRouterComponent = + normalizedAbsoluteResourcePath.startsWith(appDirPath) && + !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/); + const isMiddleware = + normalizedAbsoluteResourcePath === middlewareJsPath || normalizedAbsoluteResourcePath === middlewareTsPath; + return isPagesRouterComponent || isAppRouterComponent || isMiddleware; + }, + use: [ + { + loader: path.resolve(__dirname, 'loaders', 'configInjectionLoader.js'), + options: { + sentryConfigFilePath, + }, + }, + ], + }); + } + // The SDK uses syntax (ES6 and ES6+ features like object spread) which isn't supported by older browsers. For users // who want to support such browsers, `transpileClientSDK` allows them to force the SDK code to go through the same // transpilation that their code goes through. We don't turn this on by default because it increases bundle size From e2da3150a1ee2cc752dabd4dbf568ba33c1e507e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 11:27:49 +0000 Subject: [PATCH 3/8] . --- .../src/config/loaders/configInjectionLoader.ts | 12 +++++++++--- packages/nextjs/src/config/webpack.ts | 13 +------------ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/packages/nextjs/src/config/loaders/configInjectionLoader.ts b/packages/nextjs/src/config/loaders/configInjectionLoader.ts index e4f76141d701..09f74786dad6 100644 --- a/packages/nextjs/src/config/loaders/configInjectionLoader.ts +++ b/packages/nextjs/src/config/loaders/configInjectionLoader.ts @@ -9,13 +9,19 @@ type LoaderOptions = { /** * Injects `sentry.*.config.ts` and `sentry.*.config.ts` files into user code. */ -export default function configInjectionLoader(this: LoaderThis, userCode: string): string { +export default function configInjectionLoader( + this: LoaderThis, + userCode: string, + userModuleSourceMap: any, +): void { // We know one or the other will be defined, depending on the version of webpack being used const { sentryConfigFilePath } = 'getOptions' in this ? this.getOptions() : this.query; // We do not want to cache injected inits across builds this.cacheable(false); + this.async(); + // Inject init code from `sentry.*.config` files into the wrapping template if (path.isAbsolute(this.resourcePath)) { // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, @@ -24,8 +30,8 @@ export default function configInjectionLoader(this: LoaderThis, u const sentryConfigImportPath = path .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 .replace(/\\/g, '/'); - return `import "${sentryConfigImportPath}";\n`.concat(userCode); + this.callback(null, userCode.concat(`;import "${sentryConfigImportPath}";\n`), userModuleSourceMap); } else { - return userCode; + this.callback(null, userCode, userModuleSourceMap); } } diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 5885205045c3..290be621e465 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -254,7 +254,7 @@ export function constructWebpackConfigFunction( const sentryConfigFilePath = getUserConfigFilePath(projectDir, runtime); if (sentryConfigFilePath) { - newConfig.module.rules.unshift({ + newConfig.module.rules.push({ test: resourcePath => { const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); const isPagesRouterComponent = @@ -306,17 +306,6 @@ export function constructWebpackConfigFunction( }); } - // Tell webpack to inject user config files (containing the two `Sentry.init()` calls) into the appropriate output - // bundles. Store a separate reference to the original `entry` value to avoid an infinite loop. (If we don't do - // this, we'll have a statement of the form `x.y = () => f(x.y)`, where one of the things `f` does is call `x.y`. - // Since we're setting `x.y` to be a callback (which, by definition, won't run until some time later), by the time - // the function runs (causing `f` to run, causing `x.y` to run), `x.y` will point to the callback itself, rather - // than its original value. So calling it will call the callback which will call `f` which will call `x.y` which - // will call the callback which will call `f` which will call `x.y`... and on and on. Theoretically this could also - // be fixed by using `bind`, but this is way simpler.) - // const origEntryProperty = newConfig.entry; - // newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext, userSentryOptions); - // Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default if (shouldEnableWebpackPlugin(buildContext, userSentryOptions)) { // TODO Handle possibility that user is using `SourceMapDevToolPlugin` (see From 168272ed471c1e457287be7cb51a0a52ad2821de Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 11:59:43 +0000 Subject: [PATCH 4/8] .. --- packages/nextjs/src/config/webpack.ts | 142 +++++++++++++++++++++++++- 1 file changed, 141 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 290be621e465..f0aee954aca5 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -12,12 +12,14 @@ import type { VercelCronsConfig } from '../common/types'; // circular dependency check thinks this file is importing from itself. See https://github.com/pahen/madge/issues/306. import type { BuildContext, + EntryPropertyObject, NextConfigObject, SentryWebpackPluginOptions, UserSentryOptions, WebpackConfigFunction, WebpackConfigObject, WebpackConfigObjectWithModuleRules, + WebpackEntryProperty, WebpackModuleRule, } from './types'; @@ -123,6 +125,7 @@ export function constructWebpackConfigFunction( pagesDir: pagesDirPath, pageExtensionRegex, excludeServerRoutes: userSentryOptions.excludeServerRoutes, + sentryConfigFilePath: getUserConfigFilePath(projectDir, runtime), }; const normalizeLoaderResourcePath = (resourcePath: string): string => { @@ -253,7 +256,7 @@ export function constructWebpackConfigFunction( } const sentryConfigFilePath = getUserConfigFilePath(projectDir, runtime); - if (sentryConfigFilePath) { + if (isServer && sentryConfigFilePath) { newConfig.module.rules.push({ test: resourcePath => { const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); @@ -306,6 +309,17 @@ export function constructWebpackConfigFunction( }); } + // Tell webpack to inject user config files (containing the two `Sentry.init()` calls) into the appropriate output + // bundles. Store a separate reference to the original `entry` value to avoid an infinite loop. (If we don't do + // this, we'll have a statement of the form `x.y = () => f(x.y)`, where one of the things `f` does is call `x.y`. + // Since we're setting `x.y` to be a callback (which, by definition, won't run until some time later), by the time + // the function runs (causing `f` to run, causing `x.y` to run), `x.y` will point to the callback itself, rather + // than its original value. So calling it will call the callback which will call `f` which will call `x.y` which + // will call the callback which will call `f` which will call `x.y`... and on and on. Theoretically this could also + // be fixed by using `bind`, but this is way simpler.) + const origEntryProperty = newConfig.entry; + newConfig.entry = async () => addSentryToEntryProperty(origEntryProperty, buildContext, userSentryOptions); + // Enable the Sentry plugin (which uploads source maps to Sentry when not in dev) by default if (shouldEnableWebpackPlugin(buildContext, userSentryOptions)) { // TODO Handle possibility that user is using `SourceMapDevToolPlugin` (see @@ -414,6 +428,63 @@ function findTranspilationRules(rules: WebpackModuleRule[] | undefined, projectD return matchingRules; } +/** + * Modify the webpack `entry` property so that the code in `sentry.server.config.js` and `sentry.client.config.js` is + * included in the the necessary bundles. + * + * @param currentEntryProperty The value of the property before Sentry code has been injected + * @param buildContext Object passed by nextjs containing metadata about the build + * @returns The value which the new `entry` property (which will be a function) will return (TODO: this should return + * the function, rather than the function's return value) + */ +async function addSentryToEntryProperty( + currentEntryProperty: WebpackEntryProperty, + buildContext: BuildContext, + userSentryOptions: UserSentryOptions, +): Promise { + // The `entry` entry in a webpack config can be a string, array of strings, object, or function. By default, nextjs + // sets it to an async function which returns the promise of an object of string arrays. Because we don't know whether + // someone else has come along before us and changed that, we need to check a few things along the way. The one thing + // we know is that it won't have gotten *simpler* in form, so we only need to worry about the object and function + // options. See https://webpack.js.org/configuration/entry-context/#entry. + + const { isServer, dir: projectDir, nextRuntime } = buildContext; + const runtime = isServer ? (buildContext.nextRuntime === 'edge' ? 'edge' : 'node') : 'browser'; + + const newEntryProperty = + typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty }; + + // `sentry.server.config.js` or `sentry.client.config.js` (or their TS equivalents) + const userConfigFile = + nextRuntime === 'edge' + ? getUserConfigFile(projectDir, 'edge') + : isServer + ? getUserConfigFile(projectDir, 'server') + : getUserConfigFile(projectDir, 'client'); + + // we need to turn the filename into a path so webpack can find it + const filesToInject = userConfigFile ? [`./${userConfigFile}`] : []; + + // inject into all entry points which might contain user's code + for (const entryPointName in newEntryProperty) { + if (shouldAddSentryToEntryPoint(entryPointName, runtime)) { + addFilesToExistingEntryPoint(newEntryProperty, entryPointName, filesToInject); + } else { + if ( + isServer && + // If the user has asked to exclude pages, confirm for them that it's worked + userSentryOptions.excludeServerRoutes && + // We always skip these, so it's not worth telling the user that we've done so + !['pages/_app', 'pages/_document'].includes(entryPointName) + ) { + __DEBUG_BUILD__ && logger.log(`Skipping Sentry injection for ${entryPointName.replace(/^pages/, '')}`); + } + } + } + + return newEntryProperty; +} + /** * Search the project directory for a valid user config file for the given platform, allowing for it to be either a * TypeScript or JavaScript file. @@ -460,6 +531,58 @@ export function getUserConfigFilePath(projectDir: string, platform: 'server' | ' return undefined; } +/** + * Add files to a specific element of the given `entry` webpack config property. + * + * @param entryProperty The existing `entry` config object + * @param entryPointName The key where the file should be injected + * @param filepaths An array of paths to the injected files + */ +function addFilesToExistingEntryPoint( + entryProperty: EntryPropertyObject, + entryPointName: string, + filepaths: string[], +): void { + // can be a string, array of strings, or object whose `import` property is one of those two + const currentEntryPoint = entryProperty[entryPointName]; + let newEntryPoint = currentEntryPoint; + + if (typeof currentEntryPoint === 'string') { + newEntryPoint = [...filepaths, currentEntryPoint]; + } else if (Array.isArray(currentEntryPoint)) { + newEntryPoint = [...filepaths, ...currentEntryPoint]; + } + // descriptor object (webpack 5+) + else if (typeof currentEntryPoint === 'object' && 'import' in currentEntryPoint) { + const currentImportValue = currentEntryPoint.import; + let newImportValue; + + if (typeof currentImportValue === 'string') { + newImportValue = [...filepaths, currentImportValue]; + } else { + newImportValue = [...filepaths, ...currentImportValue]; + } + + newEntryPoint = { + ...currentEntryPoint, + import: newImportValue, + }; + } + // malformed entry point (use `console.error` rather than `logger.error` because it will always be printed, regardless + // of SDK settings) + else { + // eslint-disable-next-line no-console + console.error( + 'Sentry Logger [Error]:', + `Could not inject SDK initialization code into entry point ${entryPointName}, as its current value is not in a recognized format.\n`, + 'Expected: string | Array | { [key:string]: any, import: string | Array }\n', + `Got: ${currentEntryPoint}`, + ); + } + + entryProperty[entryPointName] = newEntryPoint; +} + /** * Check the SentryWebpackPlugin options provided by the user against the options we set by default, and warn if any of * our default options are getting overridden. (Note: If any of our default values is undefined, it won't be included in @@ -484,6 +607,23 @@ function checkWebpackPluginOverrides( } } +/** + * Determine if this is an entry point into which both `Sentry.init()` code and the release value should be injected + * + * @param entryPointName The name of the entry point in question + * @param isServer Whether or not this function is being called in the context of a server build + * @param excludeServerRoutes A list of excluded serverside entrypoints provided by the user + * @returns `true` if sentry code should be injected, and `false` otherwise + */ +function shouldAddSentryToEntryPoint(entryPointName: string, runtime: 'node' | 'browser' | 'edge'): boolean { + return ( + runtime === 'browser' && + (entryPointName === 'pages/_app' || + // entrypoint for `/app` pages + entryPointName === 'main-app') + ); +} + /** * Combine default and user-provided SentryWebpackPlugin options, accounting for whether we're building server files or * client files. From f281e30e0ced50de9e424497b8dc34f8ab2171a4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 12:24:26 +0000 Subject: [PATCH 5/8] Fix tests --- .../webpack/constructWebpackConfig.test.ts | 146 ------------------ 1 file changed, 146 deletions(-) diff --git a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts index 01f89bed1077..f336444fb117 100644 --- a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts +++ b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts @@ -88,74 +88,15 @@ describe('constructWebpackConfigFunction()', () => { }); describe('webpack `entry` property config', () => { - const serverConfigFilePath = `./${SERVER_SDK_CONFIG_FILE}`; const clientConfigFilePath = `./${CLIENT_SDK_CONFIG_FILE}`; - const edgeConfigFilePath = `./${EDGE_SDK_CONFIG_FILE}`; - - it('handles various entrypoint shapes', async () => { - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - // original entrypoint value is a string - // (was 'private-next-pages/_error.js') - 'pages/_error': [serverConfigFilePath, 'private-next-pages/_error.js'], - - // original entrypoint value is a string array - // (was ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js']) - 'pages/sniffTour': [ - serverConfigFilePath, - './node_modules/smellOVision/index.js', - 'private-next-pages/sniffTour.js', - ], - - // original entrypoint value is an object containing a string `import` value - // (was { import: 'private-next-pages/api/simulator/dogStats/[name].js' }) - 'pages/api/simulator/dogStats/[name]': { - import: [serverConfigFilePath, 'private-next-pages/api/simulator/dogStats/[name].js'], - }, - - // original entrypoint value is an object containing a string array `import` value - // (was { import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'] }) - 'pages/simulator/leaderboard': { - import: [ - serverConfigFilePath, - './node_modules/dogPoints/converter.js', - 'private-next-pages/simulator/leaderboard.js', - ], - }, - - // original entrypoint value is an object containg properties besides `import` - // (was { import: 'private-next-pages/api/tricks/[trickName].js', dependOn: 'treats', }) - 'pages/api/tricks/[trickName]': { - import: [serverConfigFilePath, 'private-next-pages/api/tricks/[trickName].js'], - dependOn: 'treats', // untouched - }, - }), - ); - }); it('injects user config file into `_app` in server bundle and in the client bundle', async () => { - const finalServerWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); const finalClientWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, incomingWebpackConfig: clientWebpackConfig, incomingWebpackBuildContext: clientBuildContext, }); - expect(finalServerWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/_app': expect.arrayContaining([serverConfigFilePath]), - }), - ); expect(finalClientWebpackConfig.entry).toEqual( expect.objectContaining({ 'pages/_app': expect.arrayContaining([clientConfigFilePath]), @@ -163,68 +104,6 @@ describe('constructWebpackConfigFunction()', () => { ); }); - it('injects user config file into `_error` in server bundle but not client bundle', async () => { - const finalServerWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - const finalClientWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: clientWebpackConfig, - incomingWebpackBuildContext: clientBuildContext, - }); - - expect(finalServerWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/_error': expect.arrayContaining([serverConfigFilePath]), - }), - ); - expect(finalClientWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/_error': expect.not.arrayContaining([clientConfigFilePath]), - }), - ); - }); - - it('injects user config file into both API routes and non-API routes', async () => { - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/api/simulator/dogStats/[name]': { - import: expect.arrayContaining([serverConfigFilePath]), - }, - - 'pages/api/tricks/[trickName]': expect.objectContaining({ - import: expect.arrayContaining([serverConfigFilePath]), - }), - - 'pages/simulator/leaderboard': { - import: expect.arrayContaining([serverConfigFilePath]), - }, - }), - ); - }); - - it('injects user config file into API middleware', async () => { - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: edgeBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - middleware: [edgeConfigFilePath, 'private-next-pages/middleware.js'], - }), - ); - }); - it('does not inject anything into non-_app pages during client build', async () => { const finalWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, @@ -244,30 +123,5 @@ describe('constructWebpackConfigFunction()', () => { simulatorBundle: './src/simulator/index.ts', }); }); - - it('does not inject into routes included in `excludeServerRoutes`', async () => { - const nextConfigWithExcludedRoutes = { - ...exportedNextConfig, - sentry: { - excludeServerRoutes: [/simulator/], - }, - }; - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig: nextConfigWithExcludedRoutes, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/simulator/leaderboard': { - import: expect.not.arrayContaining([serverConfigFilePath]), - }, - 'pages/api/simulator/dogStats/[name]': { - import: expect.not.arrayContaining([serverConfigFilePath]), - }, - }), - ); - }); }); }); From 49b85e35bc6d8c6e4ea3dc09a2cfb8b5a3698cee Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 12:30:03 +0000 Subject: [PATCH 6/8] ... --- .../config/loaders/configInjectionLoader.ts | 37 ------------------- packages/nextjs/src/config/loaders/index.ts | 1 - .../src/config/loaders/wrappingLoader.ts | 11 ++++++ packages/nextjs/src/config/webpack.ts | 26 ------------- 4 files changed, 11 insertions(+), 64 deletions(-) delete mode 100644 packages/nextjs/src/config/loaders/configInjectionLoader.ts diff --git a/packages/nextjs/src/config/loaders/configInjectionLoader.ts b/packages/nextjs/src/config/loaders/configInjectionLoader.ts deleted file mode 100644 index 09f74786dad6..000000000000 --- a/packages/nextjs/src/config/loaders/configInjectionLoader.ts +++ /dev/null @@ -1,37 +0,0 @@ -import * as path from 'path'; - -import type { LoaderThis } from './types'; - -type LoaderOptions = { - sentryConfigFilePath: string; -}; - -/** - * Injects `sentry.*.config.ts` and `sentry.*.config.ts` files into user code. - */ -export default function configInjectionLoader( - this: LoaderThis, - userCode: string, - userModuleSourceMap: any, -): void { - // We know one or the other will be defined, depending on the version of webpack being used - const { sentryConfigFilePath } = 'getOptions' in this ? this.getOptions() : this.query; - - // We do not want to cache injected inits across builds - this.cacheable(false); - - this.async(); - - // Inject init code from `sentry.*.config` files into the wrapping template - if (path.isAbsolute(this.resourcePath)) { - // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, - // however we can only create relative paths to the sentry config from absolute paths. - // Examples where this could possibly be non - absolute are virtual modules. - const sentryConfigImportPath = path - .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 - .replace(/\\/g, '/'); - this.callback(null, userCode.concat(`;import "${sentryConfigImportPath}";\n`), userModuleSourceMap); - } else { - this.callback(null, userCode, userModuleSourceMap); - } -} diff --git a/packages/nextjs/src/config/loaders/index.ts b/packages/nextjs/src/config/loaders/index.ts index 56d8c599c753..27620e004f39 100644 --- a/packages/nextjs/src/config/loaders/index.ts +++ b/packages/nextjs/src/config/loaders/index.ts @@ -1,5 +1,4 @@ export { default as valueInjectionLoader } from './valueInjectionLoader'; export { default as prefixLoader } from './prefixLoader'; export { default as wrappingLoader } from './wrappingLoader'; -export { default as configInjectionLoader } from './configInjectionLoader'; export { default as sdkMultiplexerLoader } from './sdkMultiplexerLoader'; diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index d20fa5d9e1d8..e4d58c579420 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -44,6 +44,7 @@ type LoaderOptions = { pageExtensionRegex: string; excludeServerRoutes: Array; wrappingTargetKind: 'page' | 'api-route' | 'middleware' | 'server-component'; + sentryConfigFilePath?: string; vercelCronsConfig?: VercelCronsConfig; }; @@ -74,6 +75,7 @@ export default function wrappingLoader( pageExtensionRegex, excludeServerRoutes = [], wrappingTargetKind, + sentryConfigFilePath, vercelCronsConfig, } = 'getOptions' in this ? this.getOptions() : this.query; @@ -205,6 +207,15 @@ export default function wrappingLoader( throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`); } + // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, + // however we can only create relative paths to the sentry config from absolute paths.Examples where this could possibly be non - absolute are virtual modules. + if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { + const sentryConfigImportPath = path + .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 + .replace(/\\/g, '/'); + templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); + } + // Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand. templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME); diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index f0aee954aca5..7237d8ce24be 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -255,32 +255,6 @@ export function constructWebpackConfigFunction( }); } - const sentryConfigFilePath = getUserConfigFilePath(projectDir, runtime); - if (isServer && sentryConfigFilePath) { - newConfig.module.rules.push({ - test: resourcePath => { - const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - const isPagesRouterComponent = - normalizedAbsoluteResourcePath.startsWith(pagesDirPath) && - dotPrefixedPageExtensions.some(ext => normalizedAbsoluteResourcePath.endsWith(ext)); - const isAppRouterComponent = - normalizedAbsoluteResourcePath.startsWith(appDirPath) && - !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/); - const isMiddleware = - normalizedAbsoluteResourcePath === middlewareJsPath || normalizedAbsoluteResourcePath === middlewareTsPath; - return isPagesRouterComponent || isAppRouterComponent || isMiddleware; - }, - use: [ - { - loader: path.resolve(__dirname, 'loaders', 'configInjectionLoader.js'), - options: { - sentryConfigFilePath, - }, - }, - ], - }); - } - // The SDK uses syntax (ES6 and ES6+ features like object spread) which isn't supported by older browsers. For users // who want to support such browsers, `transpileClientSDK` allows them to force the SDK code to go through the same // transpilation that their code goes through. We don't turn this on by default because it increases bundle size From beb04de977fcdd3cb737134192318851a9faac99 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 12:47:40 +0000 Subject: [PATCH 7/8] lint --- .../nextjs/test/config/webpack/constructWebpackConfig.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts index f336444fb117..86bb2d03d3fd 100644 --- a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts +++ b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts @@ -7,10 +7,7 @@ import { CLIENT_SDK_CONFIG_FILE, clientBuildContext, clientWebpackConfig, - EDGE_SDK_CONFIG_FILE, - edgeBuildContext, exportedNextConfig, - SERVER_SDK_CONFIG_FILE, serverBuildContext, serverWebpackConfig, userNextConfig, From 1d928a5abb0f81b349074caa2fdbecd6693fd777 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 20 Jun 2023 13:21:27 +0000 Subject: [PATCH 8/8] Bump timeout a bit --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 990361d6ae7c..3bc95b171d94 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -424,7 +424,7 @@ jobs: name: Nextjs (Node ${{ matrix.node }}) Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_nextjs == 'true' || github.event_name != 'pull_request' - timeout-minutes: 15 + timeout-minutes: 25 runs-on: ubuntu-20.04 strategy: fail-fast: false