diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index b0ecb2b94b05..88249bb2458a 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -1,12 +1,47 @@ import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'; -export default makeNPMConfigVariants( - makeBaseNPMConfig({ - // We need to include `instrumentServer.ts` separately because it's only conditionally required, and so rollup - // doesn't automatically include it when calculating the module dependency tree. - entrypoints: ['src/index.server.ts', 'src/index.client.ts', 'src/utils/instrumentServer.ts'], - // prevent this nextjs code from ending up in our built package (this doesn't happen automatially because the name - // doesn't match an SDK dependency) - packageSpecificConfig: { external: ['next/router'] }, - }), -); +export default [ + ...makeNPMConfigVariants( + makeBaseNPMConfig({ + // We need to include `instrumentServer.ts` separately because it's only conditionally required, and so rollup + // doesn't automatically include it when calculating the module dependency tree. + entrypoints: ['src/index.server.ts', 'src/index.client.ts', 'src/utils/instrumentServer.ts'], + + // prevent this internal nextjs code from ending up in our built package (this doesn't happen automatially because + // the name doesn't match an SDK dependency) + packageSpecificConfig: { external: ['next/router'] }, + }), + ), + ...makeNPMConfigVariants( + makeBaseNPMConfig({ + entrypoints: ['src/config/prefixLoaderTemplate.ts'], + + packageSpecificConfig: { + output: { + // preserve the original file structure (i.e., so that everything is still relative to `src`) + entryFileNames: 'config/[name].js', + + // this is going to be add-on code, so it doesn't need the trappings of a full module (and in fact actively + // shouldn't have them, lest they muck with the module to which we're adding it) + sourcemap: false, + esModule: false, + }, + }, + }), + ), + ...makeNPMConfigVariants( + makeBaseNPMConfig({ + entrypoints: ['src/config/prefixLoader.ts'], + + packageSpecificConfig: { + output: { + // make it so Rollup calms down about the fact that we're doing `export { loader as default }` + exports: 'default', + + // preserve the original file structure (i.e., so that everything is still relative to `src`) + entryFileNames: 'config/[name].js', + }, + }, + }), + ), +]; diff --git a/packages/nextjs/src/config/prefixLoader.ts b/packages/nextjs/src/config/prefixLoader.ts new file mode 100644 index 000000000000..29a1e66c73bd --- /dev/null +++ b/packages/nextjs/src/config/prefixLoader.ts @@ -0,0 +1,34 @@ +import * as fs from 'fs'; +import * as path from 'path'; + +type LoaderOptions = { + distDir: string; +}; +// TODO Use real webpack types +type LoaderThis = { + // Webpack 4 + query?: LoaderOptions; + // Webpack 5 + getOptions?: () => LoaderOptions; + addDependency: (filepath: string) => void; +}; + +/** + * Inject templated code into the beginning of a module. + */ +function prefixLoader(this: LoaderThis, userCode: string): string { + // We know one or the other will be defined, depending on the version of webpack being used + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const { distDir } = this.getOptions ? this.getOptions() : this.query!; + + const templatePath = path.resolve(__dirname, 'prefixLoaderTemplate.js'); + this.addDependency(templatePath); + + // Fill in the placeholder + let templateCode = fs.readFileSync(templatePath).toString(); + templateCode = templateCode.replace('__DIST_DIR__', distDir); + + return `${templateCode}\n${userCode}`; +} + +export { prefixLoader as default }; diff --git a/packages/nextjs/src/config/prefixLoaderTemplate.ts b/packages/nextjs/src/config/prefixLoaderTemplate.ts new file mode 100644 index 000000000000..7f529262e53f --- /dev/null +++ b/packages/nextjs/src/config/prefixLoaderTemplate.ts @@ -0,0 +1,5 @@ +// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any +(global as any).__rewriteFramesDistDir__ = '__DIST_DIR__'; + +// We need this to make this file an ESM module, which TS requires when using `isolatedModules`. +export {}; diff --git a/packages/nextjs/src/config/types.ts b/packages/nextjs/src/config/types.ts index 6acd4d65d6f8..e0c60e4484e8 100644 --- a/packages/nextjs/src/config/types.ts +++ b/packages/nextjs/src/config/types.ts @@ -56,6 +56,15 @@ export type WebpackConfigObject = { resolve?: { alias?: { [key: string]: string | boolean }; }; + module?: { + rules: Array<{ + test: string | RegExp; + use: Array<{ + loader: string; + options: Record; + }>; + }>; + }; } & { // other webpack options [key: string]: unknown; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index e2f8a7f9a947..5f98421b8d1b 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -3,7 +3,6 @@ import { getSentryRelease } from '@sentry/node'; import { dropUndefinedKeys, logger } from '@sentry/utils'; import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin'; import * as fs from 'fs'; -import * as os from 'os'; import * as path from 'path'; import { @@ -42,6 +41,7 @@ export function constructWebpackConfigFunction( // we're building server or client, whether we're in dev, what version of webpack we're using, etc). Note that // `incomingConfig` and `buildContext` are referred to as `config` and `options` in the nextjs docs. const newWebpackFunction = (incomingConfig: WebpackConfigObject, buildContext: BuildContext): WebpackConfigObject => { + const { isServer, dev: isDev } = buildContext; let newConfig = { ...incomingConfig }; // if user has custom webpack config (which always takes the form of a function), run it so we have actual values to @@ -50,6 +50,29 @@ export function constructWebpackConfigFunction( newConfig = userNextConfig.webpack(newConfig, buildContext); } + if (isServer) { + newConfig.module = { + ...newConfig.module, + rules: [ + ...(newConfig.module?.rules || []), + { + test: /sentry\.server\.config\.(jsx?|tsx?)/, + use: [ + { + // Support non-default output directories by making the output path (easy to get here at build-time) + // available to the server SDK's default `RewriteFrames` instance (which needs it at runtime), by + // injecting code to attach it to `global`. + loader: path.resolve(__dirname, 'prefixLoader.js'), + options: { + distDir: userNextConfig.distDir || '.next', + }, + }, + ], + }, + ], + }; + } + // 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`. @@ -70,7 +93,7 @@ export function constructWebpackConfigFunction( // with the `--ignore-scripts` option, this will be blocked and the missing binary will cause an error when users // try to build their apps.) ensureCLIBinaryExists() && - (buildContext.isServer + (isServer ? !userNextConfig.sentry?.disableServerWebpackPlugin : !userNextConfig.sentry?.disableClientWebpackPlugin); @@ -80,14 +103,13 @@ export function constructWebpackConfigFunction( // Next doesn't let you change `devtool` in dev even if you want to, so don't bother trying - see // https://github.com/vercel/next.js/blob/master/errors/improper-devtool.md - if (!buildContext.dev) { + if (!isDev) { // `hidden-source-map` produces the same sourcemaps as `source-map`, but doesn't include the `sourceMappingURL` // comment at the bottom. For folks who aren't publicly hosting their sourcemaps, this is helpful because then // the browser won't look for them and throw errors into the console when it can't find them. Because this is a // front-end-only problem, and because `sentry-cli` handles sourcemaps more reliably with the comment than // without, the option to use `hidden-source-map` only applies to the client-side build. - newConfig.devtool = - userNextConfig.sentry?.hideSourceMaps && !buildContext.isServer ? 'hidden-source-map' : 'source-map'; + newConfig.devtool = userNextConfig.sentry?.hideSourceMaps && !isServer ? 'hidden-source-map' : 'source-map'; } newConfig.plugins = newConfig.plugins || []; @@ -121,7 +143,7 @@ async function addSentryToEntryProperty( // 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, dev: isDev, config: userNextConfig } = buildContext; + const { isServer, dir: projectDir } = buildContext; const newEntryProperty = typeof currentEntryProperty === 'function' ? await currentEntryProperty() : { ...currentEntryProperty }; @@ -132,21 +154,6 @@ async function addSentryToEntryProperty( // we need to turn the filename into a path so webpack can find it const filesToInject = [`./${userConfigFile}`]; - // Support non-default output directories by making the output path (easy to get here at build-time) available to the - // server SDK's default `RewriteFrames` instance (which needs it at runtime). Doesn't work when using the dev server - // because it somehow tricks the file watcher into thinking that compilation itself is a file change, triggering an - // infinite recompiling loop. (This should be fine because we don't upload sourcemaps in dev in any case.) - if (isServer && !isDev) { - const rewriteFramesHelper = path.resolve( - fs.mkdtempSync(path.resolve(os.tmpdir(), 'sentry-')), - 'rewriteFramesHelper.js', - ); - fs.writeFileSync(rewriteFramesHelper, `global.__rewriteFramesDistDir__ = '${userNextConfig.distDir}';\n`); - // stick our helper file ahead of the user's config file so the value is in the global namespace *before* - // `Sentry.init()` is called - filesToInject.unshift(rewriteFramesHelper); - } - // inject into all entry points which might contain user's code for (const entryPointName in newEntryProperty) { if (shouldAddSentryToEntryPoint(entryPointName, isServer)) { diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index 76094b8deb0b..c4109e6598e5 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -110,7 +110,7 @@ function sdkAlreadyInitialized(): boolean { function addServerIntegrations(options: NextjsOptions): void { // This value is injected at build time, based on the output directory specified in the build config - const distDirName = (global as GlobalWithDistDir).__rewriteFramesDistDir__ || '.next'; + const distDirName = (global as GlobalWithDistDir).__rewriteFramesDistDir__; // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so // we can read in the project directory from the currently running process const distDirAbsPath = path.resolve(process.cwd(), distDirName); diff --git a/packages/nextjs/test/config.test.ts b/packages/nextjs/test/config.test.ts index ababfd533ed2..33e8533b89ce 100644 --- a/packages/nextjs/test/config.test.ts +++ b/packages/nextjs/test/config.test.ts @@ -332,35 +332,26 @@ describe('webpack config', () => { incomingWebpackBuildContext: serverBuildContext, }); - const tempDir = mkdtempSyncSpy.mock.results[0].value; - const rewriteFramesHelper = path.join(tempDir, 'rewriteFramesHelper.js'); - expect(finalWebpackConfig.entry).toEqual( expect.objectContaining({ // original entrypoint value is a string // (was 'private-next-pages/_error.js') - 'pages/_error': [rewriteFramesHelper, serverConfigFilePath, '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/_app.js']) - 'pages/_app': [ - rewriteFramesHelper, - serverConfigFilePath, - './node_modules/smellOVision/index.js', - 'private-next-pages/_app.js', - ], + 'pages/_app': [serverConfigFilePath, './node_modules/smellOVision/index.js', 'private-next-pages/_app.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: [rewriteFramesHelper, serverConfigFilePath, 'private-next-pages/api/simulator/dogStats/[name].js'], + 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/api/simulator/leaderboard.js'] }) 'pages/api/simulator/leaderboard': { import: [ - rewriteFramesHelper, serverConfigFilePath, './node_modules/dogPoints/converter.js', 'private-next-pages/api/simulator/leaderboard.js', @@ -370,7 +361,7 @@ describe('webpack config', () => { // 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: [rewriteFramesHelper, serverConfigFilePath, 'private-next-pages/api/tricks/[trickName].js'], + import: [serverConfigFilePath, 'private-next-pages/api/tricks/[trickName].js'], dependOn: 'treats', // untouched }, }), @@ -478,53 +469,45 @@ describe('webpack config', () => { }), ); }); + }); - it('does not inject `RewriteFrames` helper into client routes', async () => { + describe('webpack loaders', () => { + it('adds loader to server config', async () => { + const finalWebpackConfig = await materializeFinalWebpackConfig({ + userNextConfig, + incomingWebpackConfig: serverWebpackConfig, + incomingWebpackBuildContext: serverBuildContext, + }); + + expect(finalWebpackConfig.module!.rules).toEqual( + expect.arrayContaining([ + { + test: expect.any(RegExp), + use: [ + { + loader: expect.any(String), + // Having no criteria for what the object contains is better than using `expect.any(Object)`, because that + // could be anything + options: expect.objectContaining({}), + }, + ], + }, + ]), + ); + }); + + it("doesn't add loader to client config", async () => { const finalWebpackConfig = await materializeFinalWebpackConfig({ userNextConfig, incomingWebpackConfig: clientWebpackConfig, incomingWebpackBuildContext: clientBuildContext, }); - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - // was 'next-client-pages-loader?page=%2F_app', and now has client config but not`RewriteFrames` helper injected - 'pages/_app': [clientConfigFilePath, 'next-client-pages-loader?page=%2F_app'], - }), - ); + expect(finalWebpackConfig.module).toBeUndefined(); }); }); describe('`distDir` value in default server-side `RewriteFrames` integration', () => { - it.each([ - ['no custom `distDir`', undefined, '.next'], - ['custom `distDir`', 'dist', 'dist'], - ])( - 'creates file injecting `distDir` value into `global` - %s', - async (_name, customDistDir, expectedInjectedValue) => { - // Note: the fact that the file tested here gets injected correctly is covered in the 'webpack `entry` property - // config' tests above - - const userNextConfigDistDir = { - ...userNextConfig, - ...(customDistDir && { distDir: customDistDir }), - }; - await materializeFinalWebpackConfig({ - userNextConfig: userNextConfigDistDir, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: getBuildContext('server', userNextConfigDistDir), - }); - - const tempDir = mkdtempSyncSpy.mock.results[0].value; - const rewriteFramesHelper = path.join(tempDir, 'rewriteFramesHelper.js'); - - expect(fs.existsSync(rewriteFramesHelper)).toBe(true); - - const injectedCode = fs.readFileSync(rewriteFramesHelper).toString(); - expect(injectedCode).toEqual(`global.__rewriteFramesDistDir__ = '${expectedInjectedValue}';\n`); - }, - ); - describe('`RewriteFrames` ends up with correct `distDir` value', () => { // TODO: this, along with any number of other parts of the build process, should be tested with an integration // test which actually runs webpack and inspects the resulting bundles (and that integration test should test