ref(nextjs): Use loader to set RewriteFrames helper value

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',
+        },
+      },
+    }),
+  ),
+];

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 };

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 {};

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<string, unknown>;
+      }>;
+    }>;
+  };
 } & {
   // other webpack options
   [key: string]: unknown;

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)) {

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);

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