fix(nextjs): Exclude build-time files from page dependency manifests

packages/nextjs/.eslintrc.js

@@ -18,5 +18,19 @@ module.exports = {
         project: ['../../tsconfig.dev.json'],
       },
     },
+    {
+      files: ['test/buildProcess/**'],
+      parserOptions: {
+        sourceType: 'module',
+      },
+      plugins: ['react'],
+      extends: ['../../.eslintrc.js', 'plugin:react/recommended'],
+      rules: {
+        // Prop types validation is not useful in test environments
+        'react/prop-types': 'off',
+        // Nextjs takes care of including react, so we don't explicitly need to
+        'react/react-in-jsx-scope': 'off',
+      },
+    },
   ],
 };

packages/nextjs/jest.config.js

@@ -1 +1,8 @@
-module.exports = require('../../jest/jest.config.js');
+const baseConfig = require('../../jest/jest.config.js');
+
+module.exports = {
+  ...baseConfig,
+  // This prevents the build tests from running when unit tests run. (If they do, they fail, because the build being
+  // tested hasn't necessarily run yet.)
+  testPathIgnorePatterns: ['<rootDir>/test/buildProcess/'],
+};

packages/nextjs/package.json

@@ -34,6 +34,7 @@
   "devDependencies": {
     "@sentry/nextjs": "7.20.1",
     "@types/webpack": "^4.41.31",
+    "eslint-plugin-react": "^7.31.11",
     "next": "10.1.3"
   },
   "peerDependencies": {
@@ -65,7 +66,8 @@
     "lint:eslint": "eslint . --cache --cache-location '../../eslintcache/' --format stylish",
     "lint:prettier": "prettier --check \"{src,test,scripts}/**/*.ts\"",
     "test": "run-s test:unit",
-    "test:all": "run-s test:unit test:integration",
+    "test:all": "run-s test:unit test:integration test:build",
+    "test:build": "yarn ts-node test/buildProcess/runTest.ts",
     "test:unit": "jest",
     "test:integration": "test/run-integration-tests.sh && yarn test:types",
     "test:types": "cd test/types && yarn test",

packages/nextjs/src/config/types.ts

@@ -3,6 +3,11 @@ import { WebpackPluginInstance } from 'webpack';
 
 export type SentryWebpackPluginOptions = SentryCliPluginOptions;
 export type SentryWebpackPlugin = WebpackPluginInstance & { options: SentryWebpackPluginOptions };
+// Export this from here because importing something from Webpack (the library) in `webpack.ts` confuses the heck out of
+// madge, which we use for circular dependency checking. We've manually excluded this file from the check (which is
+// safe, since it only includes types), so we can import it here without causing madge to fail. See
+// https://github.com/pahen/madge/issues/306.
+export type { WebpackPluginInstance };
 
 /**
  * Overall Nextjs config

packages/nextjs/src/config/webpack.ts

@@ -6,7 +6,9 @@ import * as chalk from 'chalk';
 import * as fs from 'fs';
 import * as path from 'path';
 
-import {
+// Note: If you need to import a type from Webpack, do it in `types.ts` and export it from there. Otherwise, our
+// circular dependency check thinks this file is importing from itself. See https://github.com/pahen/madge/issues/306.
+import type {
   BuildContext,
   EntryPropertyObject,
   NextConfigObject,
@@ -16,6 +18,7 @@ import {
   WebpackConfigObject,
   WebpackEntryProperty,
   WebpackModuleRule,
+  WebpackPluginInstance,
 } from './types';
 
 export { SentryWebpackPlugin };
@@ -100,6 +103,30 @@ 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.)
+      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(path.join(__dirname, 'withSentryConfig.js'));
+        } 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

packages/nextjs/src/index.server.ts

@@ -145,7 +145,7 @@ const deprecatedIsBuild = (): boolean => isBuild();
 export { deprecatedIsBuild as isBuild };
 
 export type { SentryWebpackPluginOptions } from './config/types';
-export { withSentryConfig } from './config';
+export { withSentryConfig } from './config/withSentryConfig';
 export {
   withSentryGetServerSideProps,
   withSentryGetStaticProps,

packages/nextjs/test/buildProcess/jest.config.js

@@ -0,0 +1,4 @@
+// In order to not have the build tests run as part of the unit test suite, we exclude them in
+// `packages/nextjs/jest.config.js`. The resets the test-matching regex so that when `runTest.ts` calls `yarn jest` with
+// ths config file, jest will find the tests in `tests`.
+module.exports = require('../../../../jest/jest.config.js');

packages/nextjs/test/buildProcess/runTest.ts

@@ -0,0 +1,42 @@
+/* eslint-disable no-console */
+import * as childProcess from 'child_process';
+import * as path from 'path';
+import { sync as rimrafSync } from 'rimraf';
+
+const TEST_APP_DIR = 'test/buildProcess/testApp';
+
+/**
+ * Run the given shell command, piping the shell process's `stdin`, `stdout`, and `stderr` to that of the current
+ * process if this script is run with the `--debug` flag. Returns contents of `stdout`.
+ */
+function run(cmd: string, options?: childProcess.ExecSyncOptions): string {
+  return String(
+    childProcess.execSync(cmd, {
+      stdio: process.argv.includes('--debug') ? 'inherit' : 'ignore',
+      ...options,
+    }),
+  );
+}
+
+// Note: We use a file dependency for the SDK, rather than linking it, because if it's linked, nextjs rolls the entire
+// SDK and all of its dependencies into a bundle, making it impossible to tell (from the NFT file, at least) what's
+// being included.
+console.log('Installing dependencies...');
+process.chdir(TEST_APP_DIR);
+rimrafSync('node_modules');
+run('yarn');
+
+console.log('Building app...');
+rimrafSync('.next');
+run('yarn build');
+
+console.log('App built. Running tests...');
+process.chdir('..');
+const jestConfigFile = path.resolve(process.cwd(), 'jest.config.js');
+try {
+  // We have to specify the config file explicitly because otherwise it'll use the one at the root level of
+  // `packages/nextjs`, since that's where the closest `package.json` file is
+  run(`yarn jest --config ${jestConfigFile} tests`, { stdio: 'inherit' });
+} catch (err) {
+  console.log('\nNot all build process tests passed.');
+}

packages/nextjs/test/buildProcess/testApp/.gitignore

@@ -0,0 +1,36 @@
+# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.
+
+# dependencies
+/node_modules
+/.pnp
+.pnp.js
+
+# testing
+/coverage
+
+# next.js
+/.next/
+/out/
+
+# production
+/build
+
+# misc
+.DS_Store
+*.pem
+
+# debug
+npm-debug.log*
+yarn-debug.log*
+yarn-error.log*
+.pnpm-debug.log*
+
+# local env files
+.env*.local
+
+# vercel
+.vercel
+
+# typescript
+*.tsbuildinfo
+next-env.d.ts

packages/nextjs/test/buildProcess/testApp/next.config.js

@@ -0,0 +1,25 @@
+// const nextConfig = {
+// }
+//
+// module.exports = nextConfig
+
+const { withSentryConfig } = require('@sentry/nextjs');
+
+const moduleExports = {
+  reactStrictMode: true,
+  swcMinify: true,
+  eslint: {
+    ignoreDuringBuilds: true,
+  },
+  sentry: {
+    // Suppress the warning message from `handleSourcemapHidingOptionWarning` in `src/config/webpack.ts`
+    // TODO (v8): This can come out in v8, because this option will get a default value
+    hideSourceMaps: false,
+  },
+};
+const SentryWebpackPluginOptions = {
+  dryRun: true,
+  silent: true,
+};
+
+module.exports = withSentryConfig(moduleExports, SentryWebpackPluginOptions);

packages/nextjs/test/buildProcess/testApp/package.json

@@ -0,0 +1,18 @@
+{
+  "name": "testapp",
+  "version": "0.1.0",
+  "private": true,
+  "scripts": {
+    "dev": "next dev",
+    "build": "next build",
+    "start": "next start",
+    "lint": "next lint"
+  },
+  "dependencies": {
+    "@sentry/nextjs": "file:../../../",
+    "@vercel/nft": "latest",
+    "next": "13.0.1",
+    "react": "18.2.0",
+    "react-dom": "18.2.0"
+  }
+}

packages/nextjs/test/buildProcess/testApp/sentry.client.config.js

@@ -0,0 +1,5 @@
+import * as Sentry from '@sentry/nextjs';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+});

packages/nextjs/test/buildProcess/testApp/sentry.server.config.js

@@ -0,0 +1,5 @@
+import * as Sentry from '@sentry/nextjs';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+});

packages/nextjs/test/buildProcess/testApp/src/dogs.json

@@ -0,0 +1,3 @@
+{
+  "dogs": "are great"
+}

packages/nextjs/test/buildProcess/testApp/src/pages/_app.js

@@ -0,0 +1,5 @@
+function MyApp({ Component, pageProps }) {
+  return <Component {...pageProps} />;
+}
+
+export default MyApp;

packages/nextjs/test/buildProcess/testApp/src/pages/api/dogs.js

@@ -0,0 +1,8 @@
+import * as fs from 'fs';
+import * as path from 'path';
+
+export default function handler(req, res) {
+  const dogData = JSON.parse(fs.readFileSync(path.resolve('../../dogs.json')));
+  dogData.test = 'something';
+  res.status(200).json(dogData);
+}

packages/nextjs/test/buildProcess/testApp/src/pages/index.js

@@ -0,0 +1,3 @@
+export default function Home() {
+  return <div>This is the homepage.</div>;
+}