From f311209089b1708d46503e8898fba4ff32c9272b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Aug 2022 12:57:26 -0700 Subject: [PATCH 01/13] add `rollup` and `@rollup/plugin-sucrase` as dependencies --- packages/nextjs/package.json | 2 + packages/nextjs/test/run-integration-tests.sh | 5 ++- yarn.lock | 45 ++++++++++++++++--- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index f7817129ce80..9ef8698b9a47 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -18,6 +18,7 @@ }, "dependencies": { "@babel/parser": "^7.18.10", + "@rollup/plugin-sucrase": "4.0.4", "@sentry/core": "7.11.1", "@sentry/hub": "7.11.1", "@sentry/integrations": "7.11.1", @@ -28,6 +29,7 @@ "@sentry/utils": "7.11.1", "@sentry/webpack-plugin": "1.19.0", "jscodeshift": "^0.13.1", + "rollup": "2.78.0", "tslib": "^1.9.3" }, "devDependencies": { diff --git a/packages/nextjs/test/run-integration-tests.sh b/packages/nextjs/test/run-integration-tests.sh index 715273afb42c..7253c7b66130 100755 --- a/packages/nextjs/test/run-integration-tests.sh +++ b/packages/nextjs/test/run-integration-tests.sh @@ -61,7 +61,9 @@ for NEXTJS_VERSION in 10 11 12; do else sed -i /"next.*latest"/s/latest/"${NEXTJS_VERSION}.x"/ package.json fi - yarn --no-lockfile --silent >/dev/null 2>&1 + # We have to use `--ignore-engines` because sucrase claims to need Node 12, even though tests pass just fine on Node + # 10 + yarn --no-lockfile --ignore-engines --silent >/dev/null 2>&1 # if applicable, use local versions of `@sentry/cli` and/or `@sentry/webpack-plugin` (these commands no-op unless # LINKED_CLI_REPO and/or LINKED_PLUGIN_REPO are set) linkcli && linkplugin @@ -90,7 +92,6 @@ for NEXTJS_VERSION in 10 11 12; do exit 0 fi - # next 10 defaults to webpack 4 and next 11 defaults to webpack 5, but each can use either based on settings if [ "$NEXTJS_VERSION" -eq "10" ]; then sed "s/%RUN_WEBPACK_5%/$RUN_WEBPACK_5/g" next.config.js diff --git a/yarn.lock b/yarn.lock index e2ae1044a9b0..23fdaff9c3d6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4765,6 +4765,14 @@ "@rollup/pluginutils" "^3.1.0" magic-string "^0.25.7" +"@rollup/plugin-sucrase@4.0.4": + version "4.0.4" + resolved "https://registry.yarnpkg.com/@rollup/plugin-sucrase/-/plugin-sucrase-4.0.4.tgz#0a3b3d97cdc239ec3399f5a10711f751e9f95d98" + integrity sha512-YH4J8yoJb5EVnLhAwWxYAQNh2SJOR+SdZ6XdgoKEv6Kxm33riYkM8MlMaggN87UoISP52qAFyZ5ey56wu6umGg== + dependencies: + "@rollup/pluginutils" "^4.1.1" + sucrase "^3.20.0" + "@rollup/plugin-sucrase@^4.0.3": version "4.0.3" resolved "https://registry.yarnpkg.com/@rollup/plugin-sucrase/-/plugin-sucrase-4.0.3.tgz#b972ba61db0faaba397e09daaffcdbd38c167e2c" @@ -7644,7 +7652,7 @@ babel-plugin-syntax-exponentiation-operator@^6.8.0: babel-plugin-syntax-jsx@6.18.0: version "6.18.0" resolved "https://registry.yarnpkg.com/babel-plugin-syntax-jsx/-/babel-plugin-syntax-jsx-6.18.0.tgz#0af32a9a6e13ca7a3fd5069e62d7b0f58d0d8946" - integrity sha1-CvMqmm4Tyno/1QaeYtew9Y0NiUY= + integrity sha512-qrPaCSo9c8RHNRHIotaufGbuOBN8rtdC4QrrFFc43vyWCCz7Kl7GL1PGaXtMGQZUXrkCjNEgxDfmAuAabr/rlw== babel-plugin-syntax-trailing-function-commas@^6.22.0: version "6.22.0" @@ -9378,11 +9386,16 @@ caniuse-api@^3.0.0: lodash.memoize "^4.1.2" lodash.uniq "^4.5.0" -caniuse-lite@^1.0.0, caniuse-lite@^1.0.30000844, caniuse-lite@^1.0.30001032, caniuse-lite@^1.0.30001109, caniuse-lite@^1.0.30001173, caniuse-lite@^1.0.30001179, caniuse-lite@^1.0.30001254, caniuse-lite@^1.0.30001274, caniuse-lite@^1.0.30001280, caniuse-lite@^1.0.30001317: +caniuse-lite@^1.0.0, caniuse-lite@^1.0.30000844, caniuse-lite@^1.0.30001032, caniuse-lite@^1.0.30001109, caniuse-lite@^1.0.30001254, caniuse-lite@^1.0.30001274, caniuse-lite@^1.0.30001280, caniuse-lite@^1.0.30001317: version "1.0.30001339" resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001339.tgz" integrity sha512-Es8PiVqCe+uXdms0Gu5xP5PF2bxLR7OBp3wUzUnuO7OHzhOfCyg3hdiGWVPVxhiuniOzng+hTc1u3fEQ0TlkSQ== +caniuse-lite@^1.0.30001173, caniuse-lite@^1.0.30001179: + version "1.0.30001378" + resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001378.tgz#3d2159bf5a8f9ca093275b0d3ecc717b00f27b67" + integrity sha512-JVQnfoO7FK7WvU4ZkBRbPjaot4+YqxogSDosHv0Hv5mWpUESmN+UubMU6L/hGz8QlQ2aY5U0vR6MOs6j/CXpNA== + canonical-path@1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/canonical-path/-/canonical-path-1.0.0.tgz#fcb470c23958def85081856be7a86e904f180d1d" @@ -11572,11 +11585,16 @@ ee-first@1.1.1: resolved "https://registry.yarnpkg.com/ee-first/-/ee-first-1.1.1.tgz#590c61156b0ae2f4f0255732a158b266bc56b21d" integrity sha1-WQxhFWsK4vTwJVcyoViyZrxWsh0= -electron-to-chromium@^1.3.47, electron-to-chromium@^1.3.634, electron-to-chromium@^1.3.830: +electron-to-chromium@^1.3.47, electron-to-chromium@^1.3.830: version "1.3.839" resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.839.tgz#27a5b21468e9fefb0e328a029403617f20acec9c" integrity sha512-0O7uPs9LJNjQ/U5mW78qW8gXv9H6Ba3DHZ5/yt8aBsvomOWDkV3MddT7enUYvLQEUVOURjWmgJJWVZ3K98tIwQ== +electron-to-chromium@^1.3.634: + version "1.4.222" + resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.222.tgz#2ba24bef613fc1985dbffea85df8f62f2dec6448" + integrity sha512-gEM2awN5HZknWdLbngk4uQCVfhucFAfFzuchP3wM3NN6eow1eDU0dFy2kts43FB20ZfhVFF0jmFSTb1h5OhyIg== + electron-to-chromium@^1.3.886: version "1.3.894" resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.894.tgz#54554ecb40d40ddac7241c4a42887e86180015d8" @@ -19272,7 +19290,12 @@ nan@^2.12.1: resolved "https://registry.yarnpkg.com/nan/-/nan-2.14.2.tgz#f5376400695168f4cc694ac9393d0c9585eeea19" integrity sha512-M2ufzIiINKCuDfBSAUr1vWQ+vuVcA9kqx8JJUsbQi6yf1uGRyb7HfpdfUr5qLXf3B/t8dPvcjhKMmlfnP47EzQ== -nanoid@^3.1.16, nanoid@^3.1.20, nanoid@^3.1.23, nanoid@^3.1.30: +nanoid@^3.1.16: + version "3.3.4" + resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.3.4.tgz#730b67e3cd09e2deacf03c027c81c9d9dbc5e8ab" + integrity sha512-MqBkQh/OHTS2egovRtLk45wEyNXwF+cokD+1YPf9u5VfJiRdAiRwB2froX5Co9Rh20xs4siNPm8naNotSD6RBw== + +nanoid@^3.1.20, nanoid@^3.1.23, nanoid@^3.1.30: version "3.2.0" resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.2.0.tgz#62667522da6673971cca916a6d3eff3f415ff80c" integrity sha512-fmsZYa9lpn69Ad5eDn7FMcnnSR+8R34W9qJEijxYhTbfOWzr22n1QxCMzXLK+ODyW2973V3Fux959iQoUxzUIA== @@ -19610,7 +19633,12 @@ node-notifier@^9.0.1: uuid "^8.3.0" which "^2.0.2" -node-releases@^1.1.69, node-releases@^1.1.75: +node-releases@^1.1.69: + version "1.1.77" + resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.77.tgz#50b0cfede855dd374e7585bf228ff34e57c1c32e" + integrity sha512-rB1DUFUNAN4Gn9keO2K1efO35IDK7yKHCdCaIMvFO7yUYmmZYeDjnGKle26G4rwj+LKRQpjyUUvMkPglwGCYNQ== + +node-releases@^1.1.75: version "1.1.75" resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.75.tgz#6dd8c876b9897a1b8e5a02de26afa79bb54ebbfe" integrity sha512-Qe5OUajvqrqDSy6wrWFmMwfJ0jVgwiw4T3KqmbTcZ62qW0gQkheXYhcFM1+lOVcGUoRxcEcfyvFMAnDgaF1VWw== @@ -23171,6 +23199,13 @@ rollup@2.26.5: optionalDependencies: fsevents "~2.1.2" +rollup@2.78.0: + version "2.78.0" + resolved "https://registry.yarnpkg.com/rollup/-/rollup-2.78.0.tgz#00995deae70c0f712ea79ad904d5f6b033209d9e" + integrity sha512-4+YfbQC9QEVvKTanHhIAFVUFSRsezvQF8vFOJwtGfb9Bb+r014S+qryr9PSmw8x6sMnPkmFBGAvIFVQxvJxjtg== + optionalDependencies: + fsevents "~2.3.2" + rollup@^2.67.1: version "2.67.1" resolved "https://registry.yarnpkg.com/rollup/-/rollup-2.67.1.tgz#4402665706fa00f321d446ce45f880e02cf54f01" From e189fcbacf580b4a47c83cff123df6faa9004ce9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Aug 2022 17:48:59 -0700 Subject: [PATCH 02/13] add proxy module template --- packages/nextjs/package.json | 1 + packages/nextjs/rollup.npm.config.js | 9 +++- .../config/templates/proxyLoaderTemplate.ts | 51 +++++++++++++++++++ packages/nextjs/tsconfig.types.json | 4 ++ 4 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 packages/nextjs/src/config/templates/proxyLoaderTemplate.ts diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 9ef8698b9a47..2be9eac99558 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -34,6 +34,7 @@ }, "devDependencies": { "@babel/types": "7.18.10", + "@sentry/nextjs": "7.11.1", "@types/jscodeshift": "^0.11.5", "@types/webpack": "^4.41.31", "next": "10.1.3" diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 472ed5349095..0d01085c2b10 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -1,4 +1,4 @@ -import { makeBaseNPMConfig, makeNPMConfigVariants } from '../../rollup/index.js'; +import { makeBaseNPMConfig, makeNPMConfigVariants, plugins } from '../../rollup/index.js'; export default [ ...makeNPMConfigVariants( @@ -16,10 +16,12 @@ export default [ makeBaseNPMConfig({ entrypoints: [ 'src/config/templates/prefixLoaderTemplate.ts', + 'src/config/templates/proxyLoaderTemplate.ts', 'src/config/templates/dataFetchersLoaderTemplate.ts', ], packageSpecificConfig: { + plugins: [plugins.makeRemoveMultiLineCommentsPlugin()], output: { // Preserve the original file structure (i.e., so that everything is still relative to `src`). (Not entirely // clear why this is necessary here and not for other entrypoints in this file.) @@ -29,8 +31,11 @@ export default [ // shouldn't have them, lest they muck with the module to which we're adding it) sourcemap: false, esModule: false, + + // make it so Rollup calms down about the fact that we're combining default and named exports + exports: 'named', }, - external: ['@sentry/nextjs'], + external: ['@sentry/nextjs', '__RESOURCE_PATH__'], }, }), ), diff --git a/packages/nextjs/src/config/templates/proxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/proxyLoaderTemplate.ts new file mode 100644 index 000000000000..e303ebc24ea0 --- /dev/null +++ b/packages/nextjs/src/config/templates/proxyLoaderTemplate.ts @@ -0,0 +1,51 @@ +/** + * This file is a template for the code which will be substituted when our webpack loader handles non-API files in the + * `pages/` directory. + * + * We use `__RESOURCE_PATH__` as a placeholder for the path to the file being wrapped. Because it's not a real package, + * this causes both TS and ESLint to complain, hence the pragma comments below. + */ + +// @ts-ignore See above +// eslint-disable-next-line import/no-unresolved +import * as wrapee from '__RESOURCE_PATH__'; +import * as Sentry from '@sentry/nextjs'; +import type { GetServerSideProps, GetStaticProps, NextPage as NextPageComponent } from 'next'; + +type NextPageModule = { + default: { getInitialProps?: NextPageComponent['getInitialProps'] }; + getStaticProps?: GetStaticProps; + getServerSideProps?: GetServerSideProps; +}; + +const userPageModule = wrapee as NextPageModule; + +const pageComponent = userPageModule.default; + +const origGetInitialProps = pageComponent.getInitialProps; +const origGetStaticProps = userPageModule.getStaticProps; +const origGetServerSideProps = userPageModule.getServerSideProps; + +if (typeof origGetInitialProps === 'function') { + pageComponent.getInitialProps = Sentry.withSentryGetInitialProps( + origGetInitialProps, + '__ROUTE__', + ) as NextPageComponent['getInitialProps']; +} + +export const getStaticProps = + typeof origGetStaticProps === 'function' + ? Sentry.withSentryGetStaticProps(origGetStaticProps, '__ROUTE__') + : undefined; +export const getServerSideProps = + typeof origGetServerSideProps === 'function' + ? Sentry.withSentryGetServerSideProps(origGetServerSideProps, '__ROUTE__') + : undefined; + +export default pageComponent; + +// Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to +// not include anything whose name matchs something we've explicitly exported above. +// @ts-ignore See above +// eslint-disable-next-line import/no-unresolved +export * from '__RESOURCE_PATH__'; diff --git a/packages/nextjs/tsconfig.types.json b/packages/nextjs/tsconfig.types.json index 47e38534d75e..978b51b8e126 100644 --- a/packages/nextjs/tsconfig.types.json +++ b/packages/nextjs/tsconfig.types.json @@ -1,6 +1,10 @@ { "extends": "./tsconfig.json", + // Some of the templates for code we inject into a user's app include an import from `@sentry/nextjs`. This makes + // creating types for these template files a circular exercise, which causes `tsc` to crash. Fortunately, since the + // templates aren't consumed as modules (they're essentially just text files which happen to contain code), we don't + // actually need to create types for them. "exclude": ["src/config/templates/*"], "compilerOptions": { From 9f4a2b1406945f472c36a3d94dbf67c7b470e0a1 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Aug 2022 17:49:16 -0700 Subject: [PATCH 03/13] add rollup plugin to delete multiline comments --- rollup/plugins/npmPlugins.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/rollup/plugins/npmPlugins.js b/rollup/plugins/npmPlugins.js index 87e2927d53bd..23c9e4fff864 100644 --- a/rollup/plugins/npmPlugins.js +++ b/rollup/plugins/npmPlugins.js @@ -127,6 +127,23 @@ export function makeRemoveBlankLinesPlugin() { }); } +/** + * Create a plugin to strip multi-line comments from the output. + * + * @returns A `rollup-plugin-re` instance. + */ +export function makeRemoveMultiLineCommentsPlugin() { + return regexReplace({ + patterns: [ + { + // The `s` flag makes the dot match newlines + test: /^\/\*.*\*\//gs, + replace: '', + }, + ], + }); +} + /** * Creates a plugin to replace all instances of "__DEBUG_BUILD__" with a safe statement that * a) evaluates to `true` From 848f03f9baf61f7b4cb68a5b925d94f774f0916b Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Aug 2022 17:50:15 -0700 Subject: [PATCH 04/13] add proxy loader --- packages/nextjs/rollup.npm.config.js | 5 +- packages/nextjs/src/config/loaders/index.ts | 1 + .../nextjs/src/config/loaders/proxyLoader.ts | 91 ++++++++++++++++ packages/nextjs/src/config/loaders/rollup.ts | 103 ++++++++++++++++++ 4 files changed, 199 insertions(+), 1 deletion(-) create mode 100644 packages/nextjs/src/config/loaders/proxyLoader.ts create mode 100644 packages/nextjs/src/config/loaders/rollup.ts diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 0d01085c2b10..46224cfd600e 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -42,12 +42,15 @@ export default [ ...makeNPMConfigVariants( makeBaseNPMConfig({ entrypoints: ['src/config/loaders/index.ts'], + // Needed in order to successfully import sucrase + esModuleInterop: true, packageSpecificConfig: { output: { - // make it so Rollup calms down about the fact that we're doing `export { loader as default }` + // make it so Rollup calms down about the fact that we're combining default and named exports exports: 'named', }, + external: ['@rollup/plugin-sucrase', 'rollup'], }, }), ), diff --git a/packages/nextjs/src/config/loaders/index.ts b/packages/nextjs/src/config/loaders/index.ts index 9bda9db7cd2b..bdd71281707d 100644 --- a/packages/nextjs/src/config/loaders/index.ts +++ b/packages/nextjs/src/config/loaders/index.ts @@ -1,2 +1,3 @@ export { default as prefixLoader } from './prefixLoader'; export { default as dataFetchersLoader } from './dataFetchersLoader'; +export { default as proxyLoader } from './proxyLoader'; diff --git a/packages/nextjs/src/config/loaders/proxyLoader.ts b/packages/nextjs/src/config/loaders/proxyLoader.ts new file mode 100644 index 000000000000..f0cf96b11651 --- /dev/null +++ b/packages/nextjs/src/config/loaders/proxyLoader.ts @@ -0,0 +1,91 @@ +import { escapeStringForRegex } from '@sentry/utils'; +import * as fs from 'fs'; +import * as path from 'path'; + +import { rollupize } from './rollup'; +import { LoaderThis } from './types'; + +type LoaderOptions = { + pagesDir: string; +}; + +/** + * Replace the loaded file with a proxy module "wrapping" the original file. In the proxy, the original file is loaded, + * any data-fetching functions (`getInitialProps`, `getStaticProps`, and `getServerSideProps`) it contains are wrapped, + * and then everything is re-exported. + */ +export default async function proxyLoader(this: LoaderThis, userCode: string): Promise { + // We know one or the other will be defined, depending on the version of webpack being used + const { pagesDir } = 'getOptions' in this ? this.getOptions() : this.query; + + // Get the parameterized route name from this page's filepath + const parameterizedRoute = path + // Get the path of the file insde of the pages directory + .relative(pagesDir, this.resourcePath) + // Add a slash at the beginning + .replace(/(.*)/, '/$1') + // Pull off the file extension + .replace(/\.(jsx?|tsx?)/, '') + // Any page file named `index` corresponds to root of the directory its in, URL-wise, so turn `/xyz/index` into + // just `/xyz` + .replace(/\/index$/, '') + // In case all of the above have left us with an empty string (which will happen if we're dealing with the + // homepage), sub back in the root route + .replace(/^$/, '/'); + + // TODO: For the moment we skip API routes. Those will need to be handled slightly differently because of the manual + // wrapping we've already been having people do using `withSentry`. + if (parameterizedRoute.startsWith('api')) { + return userCode; + } + + // We don't want to wrap twice (or infinitely), so in the proxy we add this query string onto references to the + // wrapped file, so that we know that it's already been processed. (Adding this query string is also necessary to + // convince webpack that it's a different file than the one it's in the middle of loading now, so that the originals + // themselves will have a chance to load.) + if (this.resourceQuery.includes('__sentry_wrapped__')) { + return userCode; + } + + const templatePath = path.resolve(__dirname, '../templates/proxyLoaderTemplate.js'); + let templateCode = fs.readFileSync(templatePath).toString(); + // Make sure the template is included when runing `webpack watch` + this.addDependency(templatePath); + + // Inject the route into the template + templateCode = templateCode.replace(/__ROUTE__/g, parameterizedRoute); + + // Fill in the path to the file we're wrapping and save the result as a temporary file in the same folder (so that + // relative imports and exports are calculated correctly). + // + // TODO: We're saving the filled-in template to disk, however temporarily, because Rollup expects a path to a code + // file, not code itself. There is a rollup plugin which can fake this (`@rollup/plugin-virtual`) but the virtual file + // seems to be inside of a virtual directory (in other words, one level down from where you'd expect it) and that + // messes up relative imports and exports. Presumably there's a way to make it work, though, and if we can, it would + // be cleaner than having to first write and then delete a temporary file each time we run this loader. + templateCode = templateCode.replace(/__RESOURCE_PATH__/g, this.resourcePath); + const tempFilePath = path.resolve(path.dirname(this.resourcePath), `temp${Math.random()}.js`); + fs.writeFileSync(tempFilePath, templateCode); + + // Run the proxy module code through Rollup, in order to split the `export * from ''` out into + // individual exports (which nextjs seems to require), then delete the tempoary file. + let proxyCode = await rollupize(tempFilePath, this.resourcePath); + fs.unlinkSync(tempFilePath); + + if (!proxyCode) { + // We will already have thrown a warning in `rollupize`, so no need to do it again here + return userCode; + } + + // Add a query string onto all references to the wrapped file, so that webpack will consider it different from the + // non-query-stringged version (which we're already in the middle of loading as we speak), and load it separately from + // this. When the second load happens this loader will run again, but we'll be able to see the query string and will + // know to immediately return without processing. This avoids an infinite loop. + const resourceFilename = path.basename(this.resourcePath); + proxyCode = proxyCode.replace( + new RegExp(`/${escapeStringForRegex(resourceFilename)}'`, 'g'), + `/${resourceFilename}?__sentry_wrapped__'`, + ); + + return proxyCode; +} diff --git a/packages/nextjs/src/config/loaders/rollup.ts b/packages/nextjs/src/config/loaders/rollup.ts new file mode 100644 index 000000000000..67daf63b0eca --- /dev/null +++ b/packages/nextjs/src/config/loaders/rollup.ts @@ -0,0 +1,103 @@ +import type { RollupSucraseOptions } from '@rollup/plugin-sucrase'; +import sucrase from '@rollup/plugin-sucrase'; +import { logger } from '@sentry/utils'; +import * as path from 'path'; +import type { InputOptions as RollupInputOptions, OutputOptions as RollupOutputOptions } from 'rollup'; +import { rollup } from 'rollup'; + +const getRollupInputOptions: (proxyPath: string, resourcePath: string) => RollupInputOptions = ( + proxyPath, + resourcePath, +) => ({ + input: proxyPath, + plugins: [ + // For some reason, even though everything in `RollupSucraseOptions` besides `transforms` is supposed to be + // optional, TS complains that there are a bunch of missing properties (hence the typecast). Similar to + // https://github.com/microsoft/TypeScript/issues/20722, though that's been fixed. (In this case it's an interface + // exporting a `Pick` picking optional properties which is turning them required somehow.)' + sucrase({ + transforms: ['jsx', 'typescript'], + } as unknown as RollupSucraseOptions), + ], + + // We want to process as few files as possible, so as not to slow down the build any more than we have to. We need the + // proxy module (living in the temporary file we've created) and the file we're wrapping not to be external, because + // otherwise they won't be processed. (We need Rollup to process the former so that we can use the code, and we need + // it to process the latter so it knows what exports to re-export from the proxy module.) Past that, we don't care, so + // don't bother to process anything else. + external: importPath => importPath !== proxyPath && importPath !== resourcePath, + + // Prevent rollup from stressing out about TS's use of global `this` when polyfilling await. (TS will polyfill if the + // user's tsconfig `target` is set to anything before `es2017`. See https://stackoverflow.com/a/72822340 and + // https://stackoverflow.com/a/60347490.) + context: 'this', + + // Rollup's path-resolution logic when handling re-exports can go wrong when wrapping pages which aren't at the root + // level of the `pages` directory. This may be a bug, as it doesn't match the behavior described in the docs, but what + // seems to happen is this: + // + // - We try to wrap `pages/xyz/userPage.js`, which contains `export { helperFunc } from '../../utils/helper'` + // - Rollup converts '../../utils/helper' into an absolute path + // - We mark the helper module as external + // - Rollup then converts it back to a relative path, but relative to `pages/` rather than `pages/xyz/`. (This is + // the part which doesn't match the docs. They say that Rollup will use the common ancestor of all modules in the + // bundle as the basis for the relative path calculation, but both our temporary file and the page being wrapped + // live in `pages/xyz/`, and they're the only two files in the bundle, so `pages/xyz/`` should be used as the + // root. Unclear why it's not.) + // - As a result of the miscalculation, our proxy module will include `export { helperFunc } from '../utils/helper'` + // rather than the expected `export { helperFunc } from '../../utils/helper'`, thereby causing a build error in + // nextjs.. + // + // It's not 100% clear why, but telling it not to do the conversion back from absolute to relative (by setting + // `makeAbsoluteExternalsRelative` to `false`) seems to also prevent it from going from relative to absolute in the + // first place, with the result that the path remains untouched (which is what we want.) + makeAbsoluteExternalsRelative: false, +}); + +const rollupOutputOptions: RollupOutputOptions = { + format: 'esm', + + // Don't create a bundle - we just want the transformed entrypoint file + preserveModules: true, +}; + +/** + * Use Rollup to process the proxy module file (located at `tempProxyFilePath`) in order to split its `export * from + * ''` call into individual exports (which nextjs seems to need). + * + * @param tempProxyFilePath The path to the temporary file containing the proxy module code + * @param resourcePath The path to the file being wrapped + * @returns The processed proxy module code, or undefined if an error occurs + */ +export async function rollupize(tempProxyFilePath: string, resourcePath: string): Promise { + let finalBundle; + + try { + const intermediateBundle = await rollup(getRollupInputOptions(tempProxyFilePath, resourcePath)); + finalBundle = await intermediateBundle.generate(rollupOutputOptions); + } catch (err) { + __DEBUG_BUILD__ && + logger.warn( + `Could not wrap ${resourcePath}. An error occurred while processing the proxy module template:\n${err}`, + ); + return undefined; + } + + // The module at index 0 is always the entrypoint, which in this case is the proxy module. + let { code } = finalBundle.output[0]; + + // Rollup does a few things to the code we *don't* want. Undo those changes before returning the code. + // + // Nextjs uses square brackets surrounding a path segment to denote a parameter in the route, but Rollup turns those + // square brackets into underscores. Further, Rollup adds file extensions to bare-path-type import and export sources. + // Because it assumes that everything will have already been processed, it always uses `.js` as the added extension. + // We need to restore the original name and extension so that Webpack will be able to find the wrapped file. + const resourceFilename = path.basename(resourcePath); + const mutatedResourceFilename = resourceFilename + // `[\\[\\]]` is the character class containing `[` and `]` + .replace(new RegExp('[\\[\\]]', 'g'), '_') + .replace(/(jsx?|tsx?)$/, 'js'); + code = code.replace(new RegExp(mutatedResourceFilename, 'g'), resourceFilename); + + return code; +} From fe67cdbf35040ba80be6fe634427a511261c15ef Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Aug 2022 17:50:57 -0700 Subject: [PATCH 05/13] switch to using proxy loader --- packages/nextjs/src/config/webpack.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 93b79a1c7313..dcc1b909ab0d 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -86,8 +86,8 @@ export function constructWebpackConfigFunction( test: new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages/.*\\.(jsx?|tsx?)`), use: [ { - loader: path.resolve(__dirname, 'loaders/dataFetchersLoader.js'), - options: { projectDir, pagesDir }, + loader: path.resolve(__dirname, 'loaders/proxyLoader.js'), + options: { pagesDir }, }, ], }); From 4205f31cc0f9573ccb60018485e916689eab06f3 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 18 Aug 2022 07:13:05 +0000 Subject: [PATCH 06/13] Make wrapper name a little bit less annoying lol --- .../wrappers/withSentryGetServerSideProps.ts | 19 +++- .../withSentryServerSideGetInitialProps.ts | 36 ++++++ .../src/config/wrappers/wrapperUtils.ts | 106 ++++++++++++++++++ 3 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index dce840606e39..b7df1e45075f 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -1,6 +1,6 @@ import { GetServerSideProps } from 'next'; -import { callDataFetcherTraced } from './wrapperUtils'; +import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; /** * Create a wrapped version of the user's exported `getServerSideProps` function @@ -16,9 +16,18 @@ export function withSentryGetServerSideProps( return async function ( ...getServerSidePropsArguments: Parameters ): ReturnType { - return callDataFetcherTraced(origGetServerSideProps, getServerSidePropsArguments, { - parameterizedRoute, - dataFetchingMethodName: 'getServerSideProps', - }); + const [context] = getServerSidePropsArguments; + const { req, res } = context; + + const errorWrappedGetServerSideProps = withErrorInstrumentation(origGetServerSideProps); + + if (hasTracingEnabled()) { + return callTracedServerSideDataFetcher(errorWrappedGetServerSideProps, getServerSidePropsArguments, req, res, { + parameterizedRoute, + functionName: 'getServerSideProps', + }); + } else { + return errorWrappedGetServerSideProps(...getServerSidePropsArguments); + } }; } diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts new file mode 100644 index 000000000000..2a6088d4888c --- /dev/null +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -0,0 +1,36 @@ +import { hasTracingEnabled } from '@sentry/tracing'; +import { NextPage } from 'next'; + +import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; + +type GetInitialProps = Required['getInitialProps']; + +/** + * Create a wrapped version of the user's exported `getInitialProps` function + * + * @param origGetInitialProps The user's `getInitialProps` function + * @param parameterizedRoute The page's parameterized route + * @returns A wrapped version of the function + */ +export function withSentryServerSideGetInitialProps( + origGetInitialProps: GetInitialProps, + parameterizedRoute: string, +): GetInitialProps { + return async function ( + ...getInitialPropsArguments: Parameters + ): Promise> { + const [context] = getInitialPropsArguments; + const { req, res } = context; + + const errorWrappedGetInitialProps = withErrorInstrumentation(origGetInitialProps); + + if (req && res && hasTracingEnabled()) { + return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req, res, { + parameterizedRoute, + functionName: 'getInitialProps', + }); + } else { + return errorWrappedGetInitialProps(...getInitialPropsArguments); + } + }; +} diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index ab216f34aeaf..0f2a3d9d37bc 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -1,5 +1,111 @@ import { captureException } from '@sentry/core'; import { getActiveTransaction } from '@sentry/tracing'; +import { Transaction } from '@sentry/types'; +import { fill } from '@sentry/utils'; +import * as domain from 'domain'; +import { IncomingMessage, ServerResponse } from 'http'; + +declare module 'http' { + interface IncomingMessage { + _sentryTransaction?: Transaction; + } +} + +function getTransactionFromRequest(req: IncomingMessage): Transaction | undefined { + return req._sentryTransaction; +} + +function setTransactionOnRequest(transaction: Transaction, req: IncomingMessage): void { + req._sentryTransaction = transaction; +} + +function autoEndTransactionOnResponseEnd(transaction: Transaction, res: ServerResponse): void { + fill(res, 'end', (originalEnd: ServerResponse['end']) => { + return function (this: unknown, ...endArguments: Parameters) { + transaction.finish(); + return originalEnd.call(this, ...endArguments); + }; + }); +} + +/** + * Wraps a function that potentially throws. If it does, the error is passed to `captureException` and retrhrown. + */ +export function withErrorInstrumentation any>( + origFunction: F, +): (...params: Parameters) => ReturnType { + return function (this: unknown, ...origFunctionArguments: Parameters): ReturnType { + const potentialPromiseResult = origFunction.call(this, ...origFunctionArguments); + // We do this instead of await so we do not change the method signature of the passed function from `() => unknown` to `() => Promise` + Promise.resolve(potentialPromiseResult).catch(err => { + // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. + captureException(err); + }); + return potentialPromiseResult; + }; +} + +/** + * Calls a server-side data fetching function (that takes a `req` and `res` object in its context) with tracing + * instrumentation. A transaction will be created for the incoming request (if it doesn't already exist) in addition to + * a span for the wrapped data fetching function. + * + * All of the above happens in an isolated domain, meaning all thrown errors will be associated with the correct span. + * + * @param origFunction The data fetching method to call. + * @param origFunctionArguments The arguments to call the data fetching method with. + * @param req The data fetching function's request object. + * @param res The data fetching function's response object. + * @param options Options providing details for the created transaction and span. + * @returns what the data fetching method call returned. + */ +export function callTracedServerSideDataFetcher Promise | any>( + origFunction: F, + origFunctionArguments: Parameters, + req: IncomingMessage, + res: ServerResponse, + options: { + parameterizedRoute: string; + functionName: string; + }, +): Promise> { + return domain.create().bind(async () => { + let requestTransaction: Transaction | undefined = getTransactionFromRequest(req); + + if (requestTransaction === undefined) { + // TODO: Extract trace data from `req` object (trace and baggage headers) and attach it to transaction + + const newTransaction = startTransaction({ + op: 'nextjs.data', + name: options.parameterizedRoute, + metadata: { + source: 'route', + }, + }); + + requestTransaction = newTransaction; + autoEndTransactionOnResponseEnd(newTransaction, res); + setTransactionOnRequest(newTransaction, req); + } + + const dataFetcherSpan = requestTransaction.startChild({ + op: 'nextjs.data', + description: `${options.functionName} (${options.parameterizedRoute})`, + }); + + const currentScope = getCurrentHub().getScope(); + if (currentScope) { + currentScope.setSpan(dataFetcherSpan); + } + + try { + // TODO: Inject trace data into returned props + return await origFunction(...origFunctionArguments); + } finally { + dataFetcherSpan.finish(); + } + })(); +} /** * Call a data fetcher and trace it. Only traces the function if there is an active transaction on the scope. From 74edafb2fa3a3c736067fb0d8d8c3657101f987e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 18 Aug 2022 07:22:27 +0000 Subject: [PATCH 07/13] Add no-null assertion about req and res in getInitialProps --- .../config/wrappers/withSentryServerSideGetInitialProps.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts index 2a6088d4888c..5ff6f8456543 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -24,8 +24,11 @@ export function withSentryServerSideGetInitialProps( const errorWrappedGetInitialProps = withErrorInstrumentation(origGetInitialProps); - if (req && res && hasTracingEnabled()) { - return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req, res, { + if (hasTracingEnabled()) { + // Since this wrapper is only applied to `getInitialProps` running on the server, we can assert that `req` and + // `res` are always defined: https://nextjs.org/docs/api-reference/data-fetching/get-initial-props#context-object + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return callTracedServerSideDataFetcher(errorWrappedGetInitialProps, getInitialPropsArguments, req!, res!, { parameterizedRoute, functionName: 'getInitialProps', }); From cf229324e9fdcaa024bd9560379f39f64f02dd16 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 18 Aug 2022 07:28:20 +0000 Subject: [PATCH 08/13] Rethrow error in `withErrorInstumentation` --- .../src/config/wrappers/wrapperUtils.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index 0f2a3d9d37bc..a07366301f80 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -35,13 +35,20 @@ export function withErrorInstrumentation any>( origFunction: F, ): (...params: Parameters) => ReturnType { return function (this: unknown, ...origFunctionArguments: Parameters): ReturnType { - const potentialPromiseResult = origFunction.call(this, ...origFunctionArguments); - // We do this instead of await so we do not change the method signature of the passed function from `() => unknown` to `() => Promise` - Promise.resolve(potentialPromiseResult).catch(err => { + try { + const potentialPromiseResult = origFunction.call(this, ...origFunctionArguments); + // First of all, we need to capture promise rejections so we have the following check, as well as the try-catch block. + // Additionally, we do the following instead of `await`-ing so we do not change the method signature of the passed function from `() => unknown` to `() => Promise. + Promise.resolve(potentialPromiseResult).catch(err => { + // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. + captureException(err); + }); + return potentialPromiseResult; + } catch (e) { // TODO: Extract error logic from `withSentry` in here or create a new wrapper with said logic or something like that. - captureException(err); - }); - return potentialPromiseResult; + captureException(e); + throw e; + } }; } From afbe522916630a3820b00160f454c75404ec9242 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 18 Aug 2022 07:34:14 +0000 Subject: [PATCH 09/13] Introduce isBuild check --- .../src/config/wrappers/withSentryGetServerSideProps.ts | 5 +++++ .../config/wrappers/withSentryServerSideGetInitialProps.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index b7df1e45075f..ca88e8dc3fcc 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -1,5 +1,6 @@ import { GetServerSideProps } from 'next'; +import { isBuild } from '../../utils/isBuild'; import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; /** @@ -16,6 +17,10 @@ export function withSentryGetServerSideProps( return async function ( ...getServerSidePropsArguments: Parameters ): ReturnType { + if (isBuild()) { + return origGetServerSideProps(...getServerSidePropsArguments); + } + const [context] = getServerSidePropsArguments; const { req, res } = context; diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts index 5ff6f8456543..19be4b8e9a6f 100644 --- a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts @@ -1,6 +1,7 @@ import { hasTracingEnabled } from '@sentry/tracing'; import { NextPage } from 'next'; +import { isBuild } from '../../utils/isBuild'; import { callTracedServerSideDataFetcher, withErrorInstrumentation } from './wrapperUtils'; type GetInitialProps = Required['getInitialProps']; @@ -19,6 +20,10 @@ export function withSentryServerSideGetInitialProps( return async function ( ...getInitialPropsArguments: Parameters ): Promise> { + if (isBuild()) { + return origGetInitialProps(...getInitialPropsArguments); + } + const [context] = getInitialPropsArguments; const { req, res } = context; From 59bb0d8e0037d372c6493c0dcaf62e8a9c9959b4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 18 Aug 2022 07:35:55 +0000 Subject: [PATCH 10/13] Fix typo --- packages/nextjs/src/config/wrappers/wrapperUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index a07366301f80..07022ee6fc6e 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -29,7 +29,7 @@ function autoEndTransactionOnResponseEnd(transaction: Transaction, res: ServerRe } /** - * Wraps a function that potentially throws. If it does, the error is passed to `captureException` and retrhrown. + * Wraps a function that potentially throws. If it does, the error is passed to `captureException` and rethrown. */ export function withErrorInstrumentation any>( origFunction: F, From 29ee43fe4d949d2b525e35c9d712e63cbc2d9d81 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 18 Aug 2022 07:55:33 +0000 Subject: [PATCH 11/13] Add isBuildCheck to getStaticProps --- .../src/config/wrappers/withSentryGetStaticProps.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts index 0d2cc5e161dd..396c57c0652c 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts @@ -1,6 +1,7 @@ import { GetStaticProps } from 'next'; -import { callDataFetcherTraced } from './wrapperUtils'; +import { isBuild } from '../../utils/isBuild'; +import { callDataFetcherTraced, withErrorInstrumentation } from './wrapperUtils'; type Props = { [key: string]: unknown }; @@ -18,7 +19,13 @@ export function withSentryGetStaticProps( return async function ( ...getStaticPropsArguments: Parameters> ): ReturnType> { - return callDataFetcherTraced(origGetStaticProps, getStaticPropsArguments, { + if (isBuild()) { + return origGetStaticProps(...getStaticPropsArguments); + } + + const errorWrappedGetStaticProps = withErrorInstrumentation(origGetStaticProps); + + return callDataFetcherTraced(errorWrappedGetStaticProps, getStaticPropsArguments, { parameterizedRoute, dataFetchingMethodName: 'getStaticProps', }); From dc9988c95f24e976078389938c9eb6eb71c31137 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 19 Aug 2022 13:08:12 +0000 Subject: [PATCH 12/13] Add event procesor adding request data to event --- packages/nextjs/src/config/wrappers/wrapperUtils.ts | 13 ++++++++++++- packages/nextjs/src/index.client.ts | 2 -- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/config/wrappers/wrapperUtils.ts index 07022ee6fc6e..115d663350cc 100644 --- a/packages/nextjs/src/config/wrappers/wrapperUtils.ts +++ b/packages/nextjs/src/config/wrappers/wrapperUtils.ts @@ -1,4 +1,5 @@ -import { captureException } from '@sentry/core'; +import { captureException, getCurrentHub, startTransaction } from '@sentry/core'; +import { addRequestDataToEvent } from '@sentry/node'; import { getActiveTransaction } from '@sentry/tracing'; import { Transaction } from '@sentry/types'; import { fill } from '@sentry/utils'; @@ -103,6 +104,16 @@ export function callTracedServerSideDataFetcher Pr const currentScope = getCurrentHub().getScope(); if (currentScope) { currentScope.setSpan(dataFetcherSpan); + currentScope.addEventProcessor(event => + addRequestDataToEvent(event, req, { + include: { + // When the `transaction` option is set to true, it tries to extract a transaction name from the request + // object. We don't want this since we already have a high-quality transaction name with a parameterized + // route. Setting `transaction` to `true` will clobber that transaction name. + transaction: false, + }, + }), + ); } try { diff --git a/packages/nextjs/src/index.client.ts b/packages/nextjs/src/index.client.ts index 1ff546c5248d..a7bfca1f2660 100644 --- a/packages/nextjs/src/index.client.ts +++ b/packages/nextjs/src/index.client.ts @@ -11,8 +11,6 @@ export * from '@sentry/react'; export { nextRouterInstrumentation } from './performance/client'; export { captureUnderscoreErrorException } from './utils/_error'; -export { withSentryGetInitialProps } from './config/wrappers'; - export { Integrations }; // Previously we expected users to import `BrowserTracing` like this: From a5616973dc618f2d9a30224d1651cd80644abede Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 19 Aug 2022 13:32:45 +0000 Subject: [PATCH 13/13] Patch up rebase --- .../config/templates/proxyLoaderTemplate.ts | 2 +- packages/nextjs/src/config/wrappers/index.ts | 2 +- .../wrappers/withSentryGetInitialProps.ts | 26 ------------------- .../wrappers/withSentryGetServerSideProps.ts | 1 + packages/nextjs/src/index.server.ts | 6 ++++- 5 files changed, 8 insertions(+), 29 deletions(-) delete mode 100644 packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts diff --git a/packages/nextjs/src/config/templates/proxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/proxyLoaderTemplate.ts index e303ebc24ea0..43630f52daae 100644 --- a/packages/nextjs/src/config/templates/proxyLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/proxyLoaderTemplate.ts @@ -27,7 +27,7 @@ const origGetStaticProps = userPageModule.getStaticProps; const origGetServerSideProps = userPageModule.getServerSideProps; if (typeof origGetInitialProps === 'function') { - pageComponent.getInitialProps = Sentry.withSentryGetInitialProps( + pageComponent.getInitialProps = Sentry.withSentryServerSideGetInitialProps( origGetInitialProps, '__ROUTE__', ) as NextPageComponent['getInitialProps']; diff --git a/packages/nextjs/src/config/wrappers/index.ts b/packages/nextjs/src/config/wrappers/index.ts index 347f8c99f875..bd47765ae04f 100644 --- a/packages/nextjs/src/config/wrappers/index.ts +++ b/packages/nextjs/src/config/wrappers/index.ts @@ -1,3 +1,3 @@ export { withSentryGetStaticProps } from './withSentryGetStaticProps'; -export { withSentryGetInitialProps } from './withSentryGetInitialProps'; export { withSentryGetServerSideProps } from './withSentryGetServerSideProps'; +export { withSentryServerSideGetInitialProps } from './withSentryServerSideGetInitialProps'; diff --git a/packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts deleted file mode 100644 index 9abf2c7a7f0a..000000000000 --- a/packages/nextjs/src/config/wrappers/withSentryGetInitialProps.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { NextPage } from 'next'; - -import { callDataFetcherTraced } from './wrapperUtils'; - -type GetInitialProps = Required>['getInitialProps']; - -/** - * Create a wrapped version of the user's exported `getInitialProps` function - * - * @param origGetInitialProps The user's `getInitialProps` function - * @param parameterizedRoute The page's parameterized route - * @returns A wrapped version of the function - */ -export function withSentryGetInitialProps( - origGetInitialProps: GetInitialProps, - parameterizedRoute: string, -): GetInitialProps { - return async function ( - ...getInitialPropsArguments: Parameters - ): Promise> { - return callDataFetcherTraced(origGetInitialProps, getInitialPropsArguments, { - parameterizedRoute, - dataFetchingMethodName: 'getInitialProps', - }); - }; -} diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts index ca88e8dc3fcc..a9d06f17cf4e 100644 --- a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts @@ -1,3 +1,4 @@ +import { hasTracingEnabled } from '@sentry/tracing'; import { GetServerSideProps } from 'next'; import { isBuild } from '../../utils/isBuild'; diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index 0c1935b84d83..4fbe95bb8ee6 100644 --- a/packages/nextjs/src/index.server.ts +++ b/packages/nextjs/src/index.server.ts @@ -125,7 +125,11 @@ function addServerIntegrations(options: NextjsOptions): void { export type { SentryWebpackPluginOptions } from './config/types'; export { withSentryConfig } from './config'; export { isBuild } from './utils/isBuild'; -export { withSentryGetServerSideProps, withSentryGetStaticProps, withSentryGetInitialProps } from './config/wrappers'; +export { + withSentryGetServerSideProps, + withSentryGetStaticProps, + withSentryServerSideGetInitialProps, +} from './config/wrappers'; export { withSentry } from './utils/withSentry'; // Wrap various server methods to enable error monitoring and tracing. (Note: This only happens for non-Vercel