From adf6f3c71036b0a29fe3c83f21f3a5b0e9772e16 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 14 Nov 2022 23:02:56 -0800 Subject: [PATCH 1/2] inject sentry into all serverside entrypoints besides `_app` and `_document` --- packages/nextjs/src/config/webpack.ts | 36 +++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 52a48e8d04f9..83818aeb3cff 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -380,11 +380,37 @@ function checkWebpackPluginOverrides( * @returns `true` if sentry code should be injected, and `false` otherwise */ function shouldAddSentryToEntryPoint(entryPointName: string, isServer: boolean): boolean { - return ( - entryPointName === 'pages/_app' || - (entryPointName.includes('pages/api') && !entryPointName.includes('_middleware')) || - (isServer && entryPointName === 'pages/_error') - ); + // On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions). + if (isServer) { + const entryPointRoute = entryPointName.replace(/^pages/, ''); + if ( + // All non-API pages contain both of these components, and we don't want to inject more than once, so as long as + // we're doing the individual pages, it's fine to skip these. (Note: Even if a given user doesn't have either or + // both of these in their `pages/` folder, they'll exist as entrypoints because nextjs will supply default + // versions.) + entryPointRoute === '/_app' || + entryPointRoute === '/_document' || + // While middleware was in beta, it could be anywhere (at any level) in the `pages` directory, and would be called + // `_middleware.js`. Until the SDK runs successfully in the lambda edge environment, we have to exclude these. + entryPointName.includes('_middleware') || + // Newer versions of nextjs are starting to introduce things outside the `pages/` folder (middleware, an `app/` + // directory, etc), but until those features are stable and we know how we want to support them, the safest bet is + // not to inject anywhere but inside `pages/`. + !entryPointName.startsWith('pages/') + ) { + return false; + } + + // We want to inject Sentry into all other pages + return true; + } + + // On the client side, we only want to inject into `_app`, because that guarantees there'll be only one copy of the + // SDK in the eventual bundle. Since `_app` is the (effectively) the root component for every nextjs app, inclusing + // Sentry there means it will be available for every front end page. + else { + return entryPointName === 'pages/_app'; + } } /** From 3eeff43cdb414adfbec8a206dc7cb1a388d7e5af Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Mon, 14 Nov 2022 23:09:25 -0800 Subject: [PATCH 2/2] fix tests for injection into entrypoints --- packages/nextjs/test/config/fixtures.ts | 11 +++-- .../webpack/constructWebpackConfig.test.ts | 47 +++++++++++-------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/packages/nextjs/test/config/fixtures.ts b/packages/nextjs/test/config/fixtures.ts index 3e9542d85a95..be1d06a696d6 100644 --- a/packages/nextjs/test/config/fixtures.ts +++ b/packages/nextjs/test/config/fixtures.ts @@ -41,11 +41,12 @@ export const serverWebpackConfig: WebpackConfigObject = { entry: () => Promise.resolve({ 'pages/_error': 'private-next-pages/_error.js', - 'pages/_app': ['./node_modules/smellOVision/index.js', 'private-next-pages/_app.js'], + 'pages/_app': 'private-next-pages/_app.js', + 'pages/sniffTour': ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'], 'pages/api/_middleware': 'private-next-pages/api/_middleware.js', 'pages/api/simulator/dogStats/[name]': { import: 'private-next-pages/api/simulator/dogStats/[name].js' }, - 'pages/api/simulator/leaderboard': { - import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/api/simulator/leaderboard.js'], + 'pages/simulator/leaderboard': { + import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'], }, 'pages/api/tricks/[trickName]': { import: 'private-next-pages/api/tricks/[trickName].js', @@ -64,6 +65,10 @@ export const clientWebpackConfig: WebpackConfigObject = { main: './src/index.ts', 'pages/_app': 'next-client-pages-loader?page=%2F_app', 'pages/_error': 'next-client-pages-loader?page=%2F_error', + 'pages/sniffTour': ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'], + 'pages/simulator/leaderboard': { + import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'], + }, }), output: { filename: 'static/chunks/[name].js', path: '/Users/Maisey/projects/squirrelChasingSimulator/.next' }, target: 'web', diff --git a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts index 20883103a5e6..0646c00e0c9b 100644 --- a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts +++ b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts @@ -102,8 +102,12 @@ describe('constructWebpackConfigFunction()', () => { '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': [serverConfigFilePath, './node_modules/smellOVision/index.js', 'private-next-pages/_app.js'], + // (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' }) @@ -112,12 +116,12 @@ describe('constructWebpackConfigFunction()', () => { }, // 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': { + // (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/api/simulator/leaderboard.js', + 'private-next-pages/simulator/leaderboard.js', ], }, @@ -131,7 +135,7 @@ describe('constructWebpackConfigFunction()', () => { ); }); - it('injects user config file into `_app` in both server and client bundles', async () => { + it('injects user config file into `_app` in client bundle but not in server bundle', async () => { const finalServerWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, incomingWebpackConfig: serverWebpackConfig, @@ -145,7 +149,7 @@ describe('constructWebpackConfigFunction()', () => { expect(finalServerWebpackConfig.entry).toEqual( expect.objectContaining({ - 'pages/_app': expect.arrayContaining([serverConfigFilePath]), + 'pages/_app': expect.not.arrayContaining([serverConfigFilePath]), }), ); expect(finalClientWebpackConfig.entry).toEqual( @@ -179,7 +183,7 @@ describe('constructWebpackConfigFunction()', () => { ); }); - it('injects user config file into API routes', async () => { + it('injects user config file into both API routes and non-API routes', async () => { const finalWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, incomingWebpackConfig: serverWebpackConfig, @@ -192,13 +196,13 @@ describe('constructWebpackConfigFunction()', () => { import: expect.arrayContaining([serverConfigFilePath]), }, - 'pages/api/simulator/leaderboard': { - import: expect.arrayContaining([serverConfigFilePath]), - }, - 'pages/api/tricks/[trickName]': expect.objectContaining({ import: expect.arrayContaining([serverConfigFilePath]), }), + + 'pages/simulator/leaderboard': { + import: expect.arrayContaining([serverConfigFilePath]), + }, }), ); }); @@ -218,19 +222,24 @@ describe('constructWebpackConfigFunction()', () => { ); }); - it('does not inject anything into non-_app, non-_error, non-API routes', async () => { + it('does not inject anything into non-_app pages during client build', async () => { const finalWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, incomingWebpackConfig: clientWebpackConfig, incomingWebpackBuildContext: clientBuildContext, }); - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - // no injected file - main: './src/index.ts', - }), - ); + expect(finalWebpackConfig.entry).toEqual({ + main: './src/index.ts', + // only _app has config file injected + 'pages/_app': [clientConfigFilePath, 'next-client-pages-loader?page=%2F_app'], + 'pages/_error': 'next-client-pages-loader?page=%2F_error', + 'pages/sniffTour': ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js'], + 'pages/simulator/leaderboard': { + import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'], + }, + simulatorBundle: './src/simulator/index.ts', + }); }); }); });