From 7e5765a48a70e0873d8c8e9e719f67c16164778e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 8 Jan 2023 12:25:48 +0000 Subject: [PATCH 01/21] ref(nextjs): Use bundling instead of proxying to wrap pages and API routes --- packages/nextjs/package.json | 2 +- packages/nextjs/rollup.npm.config.js | 4 +- .../nextjs/src/config/loaders/proxyLoader.ts | 37 +---- packages/nextjs/src/config/loaders/rollup.ts | 148 ++++++++---------- .../templates/apiProxyLoaderTemplate.ts | 4 +- .../templates/pageProxyLoaderTemplate.ts | 4 +- packages/nextjs/src/config/webpack.ts | 3 +- yarn.lock | 32 +++- 8 files changed, 107 insertions(+), 127 deletions(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 453cc7327238..86778341eb79 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -17,7 +17,7 @@ "access": "public" }, "dependencies": { - "@rollup/plugin-sucrase": "4.0.4", + "@rollup/plugin-commonjs": "24.0.0", "@rollup/plugin-virtual": "3.0.0", "@sentry/core": "7.29.0", "@sentry/integrations": "7.29.0", diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index f9bdfbf1bcbc..6827a9beb9a3 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -32,7 +32,7 @@ export default [ // make it so Rollup calms down about the fact that we're combining default and named exports exports: 'named', }, - external: ['@sentry/nextjs', '__RESOURCE_PATH__'], + external: ['@sentry/nextjs', '__SENTRY_WRAPEE__.cjs'], }, }), ), @@ -50,7 +50,7 @@ export 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'], + external: ['@rollup/plugin-commonjs', '@rollup/plugin-virtual', 'rollup'], }, }), ), diff --git a/packages/nextjs/src/config/loaders/proxyLoader.ts b/packages/nextjs/src/config/loaders/proxyLoader.ts index c6bee0ec9f3a..67a21f11d095 100644 --- a/packages/nextjs/src/config/loaders/proxyLoader.ts +++ b/packages/nextjs/src/config/loaders/proxyLoader.ts @@ -1,4 +1,4 @@ -import { escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils'; +import { stringMatchesSomePattern } from '@sentry/utils'; import * as fs from 'fs'; import * as path from 'path'; @@ -44,48 +44,27 @@ export default async function proxyLoader(this: LoaderThis, userC 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 templateFile = parameterizedRoute.startsWith('/api') ? 'apiProxyLoaderTemplate.js' : 'pageProxyLoaderTemplate.js'; const templatePath = path.resolve(__dirname, `../templates/${templateFile}`); - let templateCode = fs.readFileSync(templatePath).toString(); + let templateCode = fs.readFileSync(templatePath, { encoding: 'utf8' }); + // Make sure the template is included when running `webpack watch` this.addDependency(templatePath); // Inject the route and the path to the file we're wrapping into the template templateCode = templateCode.replace(/__ROUTE__/g, parameterizedRoute.replace(/\\/g, '\\\\')); - templateCode = templateCode.replace(/__RESOURCE_PATH__/g, this.resourcePath.replace(/\\/g, '\\\\')); // Run the proxy module code through Rollup, in order to split the `export * from ''` out into // individual exports (which nextjs seems to require). - let proxyCode; try { - proxyCode = await rollupize(templateCode, this.resourcePath); + return await rollupize(templateCode, userCode); } catch (err) { - __DEBUG_BUILD__ && - logger.warn( - `Could not wrap ${this.resourcePath}. An error occurred while processing the proxy module template:\n${err}`, - ); + // eslint-disable-next-line no-console + console.warn( + `[@sentry/nextjs] Could not instrument ${this.resourcePath}. An error occurred while auto-wrapping:\n${err}`, + ); 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 index 6c7861827b95..e1a9eacd4258 100644 --- a/packages/nextjs/src/config/loaders/rollup.ts +++ b/packages/nextjs/src/config/loaders/rollup.ts @@ -1,104 +1,78 @@ -import sucrase from '@rollup/plugin-sucrase'; +import commonjs from '@rollup/plugin-commonjs'; import virtual from '@rollup/plugin-virtual'; -import { escapeStringForRegex } from '@sentry/utils'; -import * as path from 'path'; -import type { InputOptions as RollupInputOptions, OutputOptions as RollupOutputOptions } from 'rollup'; import { rollup } from 'rollup'; -const SENTRY_PROXY_MODULE_NAME = 'sentry-proxy-module'; - -const getRollupInputOptions = (templateCode: string, userModulePath: string): RollupInputOptions => ({ - input: SENTRY_PROXY_MODULE_NAME, - - plugins: [ - virtual({ - [SENTRY_PROXY_MODULE_NAME]: templateCode, - }), - - sucrase({ - transforms: ['jsx', 'typescript'], - }), - ], - - // 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 !== SENTRY_PROXY_MODULE_NAME && importPath !== userModulePath, - - // 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.. - // - // Setting `makeAbsoluteExternalsRelative` to `false` prevents all of the above by causing Rollup to ignore imports of - // externals entirely, with the result that their paths remain 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, -}; +const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module'; +const WRAPEE_MODULE_NAME = '__SENTRY_WRAPEE__.cjs'; /** * Use Rollup to process the proxy module code, in order to split its `export * from ''` call into * individual exports (which nextjs seems to need). * - * Note: Any errors which occur are handled by the proxy loader which calls this function. + * Note: This function may throw in case something goes wrong while bundling. * - * @param templateCode The proxy module code - * @param userModulePath The path to the file being wrapped + * @param templateCode The wrapper module code + * @param userModuleCode The path to the file being wrapped * @returns The processed proxy module code */ -export async function rollupize(templateCode: string, userModulePath: string): Promise { - const intermediateBundle = await rollup(getRollupInputOptions(templateCode, userModulePath)); - const finalBundle = await intermediateBundle.generate(rollupOutputOptions); +export async function rollupize(templateCode: string, userModuleCode: string): Promise { + const rollupBuild = await rollup({ + input: SENTRY_WRAPPER_MODULE_NAME, - // The module at index 0 is always the entrypoint, which in this case is the proxy module. - let { code } = finalBundle.output[0]; + plugins: [ + // We're using virtual modules so we don't have to mess around with file paths + virtual({ + [SENTRY_WRAPPER_MODULE_NAME]: templateCode, + [WRAPEE_MODULE_NAME]: userModuleCode, + }), + + // People may use `module.exports` in their API routes or page files. Next.js allows that and we also need to + // handle that correctly so we let a plugin to take care of bundling cjs exports for us. + commonjs(), + ], + + // We only want to bundle our wrapper module and the wrappee module into one, so we mark everything else as external. + external: source => source !== SENTRY_WRAPPER_MODULE_NAME && source !== WRAPEE_MODULE_NAME, - // In addition to doing the desired work, Rollup also does a few things we *don't* want. Specifically, in messes up - // the path in both `import * as origModule from ''` and `export * from ''`. - // - // - It turns the square brackets surrounding each parameterized path segment into underscores. - // - It always adds `.js` to the end of the filename. - // - It converts the path from aboslute to relative, which would be fine except that when used with the virual plugin, - // it uses an incorrect (and not entirely predicable) base for that relative path. - // - // To fix this, we overwrite the messed up path with what we know it should be: `./`. (We can - // find the value of the messed up path by looking at what `import * as origModule from ''` becomes. - // Because it's the first line of the template, it's also the first line of the result, and is therefore easy to - // find.) + // 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', - const importStarStatement = code.split('\n')[0]; - // This regex should always match (we control both the input and the process which generates it, so we can guarantee - // the outcome of that processing), but just in case it somehow doesn't, we need it to throw an error so that the - // proxy loader will know to return the user's code untouched rather than returning proxy module code including a - // broken path. The non-null assertion asserts that a match has indeed been found. - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const messedUpPath = /^import \* as .* from '(.*)';$/.exec(importStarStatement)![1]; + // 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.. + // + // Setting `makeAbsoluteExternalsRelative` to `false` prevents all of the above by causing Rollup to ignore imports of + // externals entirely, with the result that their paths remain untouched (which is what we want). + makeAbsoluteExternalsRelative: false, - code = code.replace(new RegExp(escapeStringForRegex(messedUpPath), 'g'), `./${path.basename(userModulePath)}`); + onwarn: () => { + // Suppress all warnings - we don't want to bother people with this output + }, - return code; + output: { + interop: 'auto', + exports: 'named', + }, + }); + + const finalBundle = await rollupBuild.generate({ + format: 'esm', + }); + + // The module at index 0 is always the entrypoint, which in this case is the proxy module. + return finalBundle.output[0].code; } diff --git a/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts index 230eb55e457f..1f8ec2a3e412 100644 --- a/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts @@ -8,7 +8,7 @@ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved -import * as origModule from '__RESOURCE_PATH__'; +import * as origModule from '__SENTRY_WRAPEE__.cjs'; import * as Sentry from '@sentry/nextjs'; import type { PageConfig } from 'next'; @@ -73,4 +73,4 @@ export default exportedHandler; // 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__'; +export * from '__SENTRY_WRAPEE__.cjs'; diff --git a/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts index 9979ae814c49..2c09d8c4479c 100644 --- a/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts @@ -8,7 +8,7 @@ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved -import * as wrapee from '__RESOURCE_PATH__'; +import * as wrapee from '__SENTRY_WRAPEE__.cjs'; import * as Sentry from '@sentry/nextjs'; import type { GetServerSideProps, GetStaticProps, NextPage as NextPageComponent } from 'next'; @@ -53,4 +53,4 @@ export default pageComponent; // 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__'; +export * from '__SENTRY_WRAPEE__.cjs'; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 988c6cc8b4b3..4e64caa2966d 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -93,7 +93,8 @@ export function constructWebpackConfigFunction( const pageExtensions = userNextConfig.pageExtensions || ['tsx', 'ts', 'jsx', 'js']; const pageExtensionRegex = pageExtensions.map(escapeStringForRegex).join('|'); - newConfig.module.rules.push({ + // It is very important that we insert our loader at the beginning of the array because we expect any sort of transformations/transpilations (e.g. TS -> JS) to already have happened. + newConfig.module.rules.unshift({ test: new RegExp(`^${escapeStringForRegex(pagesDir)}.*\\.(${pageExtensionRegex})$`), use: [ { diff --git a/yarn.lock b/yarn.lock index d1bfebbebeba..7f4faf2247a1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3385,6 +3385,18 @@ dependencies: web-streams-polyfill "^3.1.1" +"@rollup/plugin-commonjs@24.0.0": + version "24.0.0" + resolved "https://registry.yarnpkg.com/@rollup/plugin-commonjs/-/plugin-commonjs-24.0.0.tgz#fb7cf4a6029f07ec42b25daa535c75b05a43f75c" + integrity sha512-0w0wyykzdyRRPHOb0cQt14mIBLujfAv6GgP6g8nvg/iBxEm112t3YPPq+Buqe2+imvElTka+bjNlJ/gB56TD8g== + dependencies: + "@rollup/pluginutils" "^5.0.1" + commondir "^1.0.1" + estree-walker "^2.0.2" + glob "^8.0.3" + is-reference "1.2.1" + magic-string "^0.27.0" + "@rollup/plugin-commonjs@^15.0.0": version "15.1.0" resolved "https://registry.yarnpkg.com/@rollup/plugin-commonjs/-/plugin-commonjs-15.1.0.tgz#1e7d076c4f1b2abf7e65248570e555defc37c238" @@ -3450,7 +3462,7 @@ "@rollup/pluginutils" "^3.1.0" magic-string "^0.25.7" -"@rollup/plugin-sucrase@4.0.4", "@rollup/plugin-sucrase@^4.0.3": +"@rollup/plugin-sucrase@^4.0.3": version "4.0.4" resolved "https://registry.yarnpkg.com/@rollup/plugin-sucrase/-/plugin-sucrase-4.0.4.tgz#0a3b3d97cdc239ec3399f5a10711f751e9f95d98" integrity sha512-YH4J8yoJb5EVnLhAwWxYAQNh2SJOR+SdZ6XdgoKEv6Kxm33riYkM8MlMaggN87UoISP52qAFyZ5ey56wu6umGg== @@ -3488,6 +3500,15 @@ estree-walker "^2.0.1" picomatch "^2.2.2" +"@rollup/pluginutils@^5.0.1": + version "5.0.2" + resolved "https://registry.yarnpkg.com/@rollup/pluginutils/-/pluginutils-5.0.2.tgz#012b8f53c71e4f6f9cb317e311df1404f56e7a33" + integrity sha512-pTd9rIsP92h+B6wWwFbW8RkZv4hiR/xKsqre4SIuAOaOEQRxi0lqLke9k2/7WegC85GgUs9pjmOjCUi3In4vwA== + dependencies: + "@types/estree" "^1.0.0" + estree-walker "^2.0.2" + picomatch "^2.3.1" + "@schematics/angular@10.2.4": version "10.2.4" resolved "https://registry.yarnpkg.com/@schematics/angular/-/angular-10.2.4.tgz#3b99b9da572b57381d221e2008804e6bb9c98b82" @@ -4081,6 +4102,11 @@ resolved "https://registry.yarnpkg.com/@types/estree/-/estree-0.0.39.tgz#e177e699ee1b8c22d23174caaa7422644389509f" integrity sha512-EYNwp3bU+98cpU4lAWYYL7Zz+2gryWH1qbdDTidVd6hkiR6weksdbMadyXKXNPEkQFhXM+hVO9ZygomHXp+AIw== +"@types/estree@^1.0.0": + version "1.0.0" + resolved "https://registry.yarnpkg.com/@types/estree/-/estree-1.0.0.tgz#5fb2e536c1ae9bf35366eed879e827fa59ca41c2" + integrity sha512-WulqXMDUTYAXCjZnk6JtIHPigp55cVtDgDrO2gHRwhyJto21+1zbVCtOYB2L1F9w4qCQ0rOGWBnBe0FNTiEJIQ== + "@types/express-serve-static-core@4.17.28", "@types/express-serve-static-core@4.17.30", "@types/express-serve-static-core@^4.17.18": version "4.17.30" resolved "https://registry.yarnpkg.com/@types/express-serve-static-core/-/express-serve-static-core-4.17.30.tgz#0f2f99617fa8f9696170c46152ccf7500b34ac04" @@ -12742,7 +12768,7 @@ glob@7.2.0: once "^1.3.0" path-is-absolute "^1.0.0" -glob@8.0.3: +glob@8.0.3, glob@^8.0.3: version "8.0.3" resolved "https://registry.yarnpkg.com/glob/-/glob-8.0.3.tgz#415c6eb2deed9e502c68fa44a272e6da6eeca42e" integrity sha512-ull455NHSHI/Y1FqGaaYFaLGkNMMJbavMrEGFXG/PGrg6y7sutWHUHrz6gy6WEBH6akM1M414dWKCNs+IhKdiQ== @@ -14359,7 +14385,7 @@ is-potential-custom-element-name@^1.0.1: resolved "https://registry.yarnpkg.com/is-potential-custom-element-name/-/is-potential-custom-element-name-1.0.1.tgz#171ed6f19e3ac554394edf78caa05784a45bebb5" integrity sha512-bCYeRA2rVibKZd+s2625gGnGF/t7DSqDs4dP7CrLA1m7jKWz6pps0LpYLJN8Q64HtmPKJ1hrN3nzPNKFEKOUiQ== -is-reference@^1.2.1: +is-reference@1.2.1, is-reference@^1.2.1: version "1.2.1" resolved "https://registry.yarnpkg.com/is-reference/-/is-reference-1.2.1.tgz#8b2dac0b371f4bc994fdeaba9eb542d03002d0b7" integrity sha512-U82MsXXiFIrjCK4otLT+o2NA2Cd2g5MLoOVXUZjIOhLurrRxpEXzI8O0KZHr3IjLvlAH1kTPYSuqer5T9ZVBKQ== From a91da1660f0b6da860dcba8a49f69fd62bc7eb48 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 8 Jan 2023 12:31:32 +0000 Subject: [PATCH 02/21] Rename templates --- packages/nextjs/rollup.npm.config.js | 5 +---- packages/nextjs/src/config/loaders/proxyLoader.ts | 4 +--- .../{apiProxyLoaderTemplate.ts => apiWrapperTemplate.ts} | 0 .../{pageProxyLoaderTemplate.ts => pageWrapperTemplate.ts} | 0 4 files changed, 2 insertions(+), 7 deletions(-) rename packages/nextjs/src/config/templates/{apiProxyLoaderTemplate.ts => apiWrapperTemplate.ts} (100%) rename packages/nextjs/src/config/templates/{pageProxyLoaderTemplate.ts => pageWrapperTemplate.ts} (100%) diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 6827a9beb9a3..c3da69daf595 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -14,10 +14,7 @@ export default [ ), ...makeNPMConfigVariants( makeBaseNPMConfig({ - entrypoints: [ - 'src/config/templates/pageProxyLoaderTemplate.ts', - 'src/config/templates/apiProxyLoaderTemplate.ts', - ], + entrypoints: ['src/config/templates/pageWrapperTemplate.ts', 'src/config/templates/apiWrapperTemplate.ts'], packageSpecificConfig: { output: { diff --git a/packages/nextjs/src/config/loaders/proxyLoader.ts b/packages/nextjs/src/config/loaders/proxyLoader.ts index 67a21f11d095..67844a766040 100644 --- a/packages/nextjs/src/config/loaders/proxyLoader.ts +++ b/packages/nextjs/src/config/loaders/proxyLoader.ts @@ -44,9 +44,7 @@ export default async function proxyLoader(this: LoaderThis, userC return userCode; } - const templateFile = parameterizedRoute.startsWith('/api') - ? 'apiProxyLoaderTemplate.js' - : 'pageProxyLoaderTemplate.js'; + const templateFile = parameterizedRoute.startsWith('/api') ? 'apiWrapperTemplate.js' : 'pageWrapperTemplate.js'; const templatePath = path.resolve(__dirname, `../templates/${templateFile}`); let templateCode = fs.readFileSync(templatePath, { encoding: 'utf8' }); diff --git a/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts similarity index 100% rename from packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts rename to packages/nextjs/src/config/templates/apiWrapperTemplate.ts diff --git a/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/pageWrapperTemplate.ts similarity index 100% rename from packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts rename to packages/nextjs/src/config/templates/pageWrapperTemplate.ts From 9fb471d47150111942d14defce69bf1c60a57df9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 8 Jan 2023 12:59:49 +0000 Subject: [PATCH 03/21] Move wrapping function into loader --- .../nextjs/src/config/loaders/proxyLoader.ts | 87 ++++++++++++++++++- packages/nextjs/src/config/loaders/rollup.ts | 78 ----------------- 2 files changed, 85 insertions(+), 80 deletions(-) delete mode 100644 packages/nextjs/src/config/loaders/rollup.ts diff --git a/packages/nextjs/src/config/loaders/proxyLoader.ts b/packages/nextjs/src/config/loaders/proxyLoader.ts index 67844a766040..e60d488c3dc2 100644 --- a/packages/nextjs/src/config/loaders/proxyLoader.ts +++ b/packages/nextjs/src/config/loaders/proxyLoader.ts @@ -1,10 +1,15 @@ +import commonjs from '@rollup/plugin-commonjs'; +import virtual from '@rollup/plugin-virtual'; import { stringMatchesSomePattern } from '@sentry/utils'; import * as fs from 'fs'; import * as path from 'path'; +import { rollup } from 'rollup'; -import { rollupize } from './rollup'; import { LoaderThis } from './types'; +const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module'; +const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPEE__.cjs'; + type LoaderOptions = { pagesDir: string; pageExtensionRegex: string; @@ -57,7 +62,7 @@ export default async function proxyLoader(this: LoaderThis, userC // Run the proxy module code through Rollup, in order to split the `export * from ''` out into // individual exports (which nextjs seems to require). try { - return await rollupize(templateCode, userCode); + return await wrapUserCode(templateCode, userCode); } catch (err) { // eslint-disable-next-line no-console console.warn( @@ -66,3 +71,81 @@ export default async function proxyLoader(this: LoaderThis, userC return userCode; } } + +/** + * Use Rollup to process the proxy module code, in order to split its `export * from ''` call into + * individual exports (which nextjs seems to need). + * + * Wraps provided user code (located under the import defined via WRAPPING_TARGET_MODULE_NAME) with provided wrapper + * code. Under the hood, this function uses rollup to bundle the modules together. Rollup is convenient for us because + * it turns `export * from ''` (which Next.js doesn't allow) into individual named exports. + * + * Note: This function may throw in case something goes wrong while bundling. + * + * @param wrapperCode The wrapper module code + * @param userModuleCode The user module code + * @returns The wrapped user code + */ +async function wrapUserCode(wrapperCode: string, userModuleCode: string): Promise { + const rollupBuild = await rollup({ + input: SENTRY_WRAPPER_MODULE_NAME, + + plugins: [ + // We're using virtual modules so we don't have to mess around with file paths + virtual({ + [SENTRY_WRAPPER_MODULE_NAME]: wrapperCode, + [WRAPPING_TARGET_MODULE_NAME]: userModuleCode, + }), + + // People may use `module.exports` in their API routes or page files. Next.js allows that and we also need to + // handle that correctly so we let a plugin to take care of bundling cjs exports for us. + commonjs(), + ], + + // We only want to bundle our wrapper module and the wrappee module into one, so we mark everything else as external. + external: source => source !== SENTRY_WRAPPER_MODULE_NAME && source !== WRAPPING_TARGET_MODULE_NAME, + + // 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.. + // + // Setting `makeAbsoluteExternalsRelative` to `false` prevents all of the above by causing Rollup to ignore imports of + // externals entirely, with the result that their paths remain untouched (which is what we want). + makeAbsoluteExternalsRelative: false, + + onwarn: () => { + // Suppress all warnings - we don't want to bother people with this output + }, + + output: { + // TODO + interop: 'auto', + // TODO + exports: 'named', + }, + }); + + const finalBundle = await rollupBuild.generate({ + format: 'esm', + }); + + // The module at index 0 is always the entrypoint, which in this case is the proxy module. + return finalBundle.output[0].code; +} diff --git a/packages/nextjs/src/config/loaders/rollup.ts b/packages/nextjs/src/config/loaders/rollup.ts deleted file mode 100644 index e1a9eacd4258..000000000000 --- a/packages/nextjs/src/config/loaders/rollup.ts +++ /dev/null @@ -1,78 +0,0 @@ -import commonjs from '@rollup/plugin-commonjs'; -import virtual from '@rollup/plugin-virtual'; -import { rollup } from 'rollup'; - -const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module'; -const WRAPEE_MODULE_NAME = '__SENTRY_WRAPEE__.cjs'; - -/** - * Use Rollup to process the proxy module code, in order to split its `export * from ''` call into - * individual exports (which nextjs seems to need). - * - * Note: This function may throw in case something goes wrong while bundling. - * - * @param templateCode The wrapper module code - * @param userModuleCode The path to the file being wrapped - * @returns The processed proxy module code - */ -export async function rollupize(templateCode: string, userModuleCode: string): Promise { - const rollupBuild = await rollup({ - input: SENTRY_WRAPPER_MODULE_NAME, - - plugins: [ - // We're using virtual modules so we don't have to mess around with file paths - virtual({ - [SENTRY_WRAPPER_MODULE_NAME]: templateCode, - [WRAPEE_MODULE_NAME]: userModuleCode, - }), - - // People may use `module.exports` in their API routes or page files. Next.js allows that and we also need to - // handle that correctly so we let a plugin to take care of bundling cjs exports for us. - commonjs(), - ], - - // We only want to bundle our wrapper module and the wrappee module into one, so we mark everything else as external. - external: source => source !== SENTRY_WRAPPER_MODULE_NAME && source !== WRAPEE_MODULE_NAME, - - // 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.. - // - // Setting `makeAbsoluteExternalsRelative` to `false` prevents all of the above by causing Rollup to ignore imports of - // externals entirely, with the result that their paths remain untouched (which is what we want). - makeAbsoluteExternalsRelative: false, - - onwarn: () => { - // Suppress all warnings - we don't want to bother people with this output - }, - - output: { - interop: 'auto', - exports: 'named', - }, - }); - - const finalBundle = await rollupBuild.generate({ - format: 'esm', - }); - - // The module at index 0 is always the entrypoint, which in this case is the proxy module. - return finalBundle.output[0].code; -} From 89f5e79d94614c9ffe74b651180f46f3f4b37a93 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 8 Jan 2023 13:05:44 +0000 Subject: [PATCH 04/21] Explain target module name --- packages/nextjs/rollup.npm.config.js | 2 +- packages/nextjs/src/config/loaders/proxyLoader.ts | 9 ++++++++- .../nextjs/src/config/templates/apiWrapperTemplate.ts | 4 ++-- .../nextjs/src/config/templates/pageWrapperTemplate.ts | 4 ++-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index c3da69daf595..8899eac8851a 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -29,7 +29,7 @@ export default [ // make it so Rollup calms down about the fact that we're combining default and named exports exports: 'named', }, - external: ['@sentry/nextjs', '__SENTRY_WRAPEE__.cjs'], + external: ['@sentry/nextjs', '__SENTRY_WRAPPING_TARGET__'], }, }), ), diff --git a/packages/nextjs/src/config/loaders/proxyLoader.ts b/packages/nextjs/src/config/loaders/proxyLoader.ts index e60d488c3dc2..837aa6fe0de8 100644 --- a/packages/nextjs/src/config/loaders/proxyLoader.ts +++ b/packages/nextjs/src/config/loaders/proxyLoader.ts @@ -7,8 +7,12 @@ import { rollup } from 'rollup'; import { LoaderThis } from './types'; +// Just a simple placeholder to make referencing module consistent const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module'; -const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPEE__.cjs'; + +// needs to end in .cjs in order for the `commonjs` plugin to pick it up - unfortunately the plugin has no option to +// make this work in combination with the`virtual` plugin +const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPPING_TARGET__.cjs'; type LoaderOptions = { pagesDir: string; @@ -59,6 +63,9 @@ export default async function proxyLoader(this: LoaderThis, userC // Inject the route and the path to the file we're wrapping into the template templateCode = templateCode.replace(/__ROUTE__/g, parameterizedRoute.replace(/\\/g, '\\\\')); + // Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand. + templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET__/g, WRAPPING_TARGET_MODULE_NAME); + // Run the proxy module code through Rollup, in order to split the `export * from ''` out into // individual exports (which nextjs seems to require). try { diff --git a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts index 1f8ec2a3e412..db06550a3584 100644 --- a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts @@ -8,7 +8,7 @@ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved -import * as origModule from '__SENTRY_WRAPEE__.cjs'; +import * as origModule from '__SENTRY_WRAPPING_TARGET__'; import * as Sentry from '@sentry/nextjs'; import type { PageConfig } from 'next'; @@ -73,4 +73,4 @@ export default exportedHandler; // 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 '__SENTRY_WRAPEE__.cjs'; +export * from '__SENTRY_WRAPPING_TARGET__'; diff --git a/packages/nextjs/src/config/templates/pageWrapperTemplate.ts b/packages/nextjs/src/config/templates/pageWrapperTemplate.ts index 2c09d8c4479c..116c0503fbef 100644 --- a/packages/nextjs/src/config/templates/pageWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/pageWrapperTemplate.ts @@ -8,7 +8,7 @@ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved -import * as wrapee from '__SENTRY_WRAPEE__.cjs'; +import * as wrapee from '__SENTRY_WRAPPING_TARGET__'; import * as Sentry from '@sentry/nextjs'; import type { GetServerSideProps, GetStaticProps, NextPage as NextPageComponent } from 'next'; @@ -53,4 +53,4 @@ export default pageComponent; // 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 '__SENTRY_WRAPEE__.cjs'; +export * from '__SENTRY_WRAPPING_TARGET__'; From 0ec7960fcb7db76b13e0f5faa403e4f4f7371b00 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 8 Jan 2023 13:14:37 +0000 Subject: [PATCH 05/21] Rename proxyLoader to wrappingLoader --- packages/nextjs/src/config/loaders/index.ts | 2 +- .../config/loaders/{proxyLoader.ts => wrappingLoader.ts} | 8 ++++---- packages/nextjs/src/config/webpack.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) rename packages/nextjs/src/config/loaders/{proxyLoader.ts => wrappingLoader.ts} (95%) diff --git a/packages/nextjs/src/config/loaders/index.ts b/packages/nextjs/src/config/loaders/index.ts index 150e00bf1ca4..322567c1495b 100644 --- a/packages/nextjs/src/config/loaders/index.ts +++ b/packages/nextjs/src/config/loaders/index.ts @@ -1,3 +1,3 @@ export { default as valueInjectionLoader } from './valueInjectionLoader'; export { default as prefixLoader } from './prefixLoader'; -export { default as proxyLoader } from './proxyLoader'; +export { default as wrappingLoader } from './wrappingLoader'; diff --git a/packages/nextjs/src/config/loaders/proxyLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts similarity index 95% rename from packages/nextjs/src/config/loaders/proxyLoader.ts rename to packages/nextjs/src/config/loaders/wrappingLoader.ts index 837aa6fe0de8..0baa2c7a4a23 100644 --- a/packages/nextjs/src/config/loaders/proxyLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -21,11 +21,11 @@ type LoaderOptions = { }; /** - * 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. + * Replace the loaded file with a wrapped version the original file. In the wrapped version, the original file is loaded, + * any data-fetching functions (`getInitialProps`, `getStaticProps`, and `getServerSideProps`) or API routes it contains + * are wrapped, and then everything is re-exported. */ -export default async function proxyLoader(this: LoaderThis, userCode: string): Promise { +export default async function wrappingLoader(this: LoaderThis, userCode: string): Promise { // We know one or the other will be defined, depending on the version of webpack being used const { pagesDir, diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 4e64caa2966d..3997bf3f9674 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -98,7 +98,7 @@ export function constructWebpackConfigFunction( test: new RegExp(`^${escapeStringForRegex(pagesDir)}.*\\.(${pageExtensionRegex})$`), use: [ { - loader: path.resolve(__dirname, 'loaders/proxyLoader.js'), + loader: path.resolve(__dirname, 'loaders/wrappingLoader.js'), options: { pagesDir, pageExtensionRegex, From 7af6c7b9f58e36dd88e3dd8a4dbff6566a0ffb62 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 8 Jan 2023 13:20:07 +0000 Subject: [PATCH 06/21] Extract template reading out of loader --- .../nextjs/src/config/loaders/wrappingLoader.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 0baa2c7a4a23..2e01feadff73 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -7,6 +7,12 @@ import { rollup } from 'rollup'; import { LoaderThis } from './types'; +const apiWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'apiWrapperTemplate.js'); +const apiWrapperTemplateCode = fs.readFileSync(apiWrapperTemplatePath, { encoding: 'utf8' }); + +const pageWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'pageWrapperTemplate.js'); +const pageWrapperTemplateCode = fs.readFileSync(pageWrapperTemplatePath, { encoding: 'utf8' }); + // Just a simple placeholder to make referencing module consistent const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module'; @@ -53,12 +59,7 @@ export default async function wrappingLoader(this: LoaderThis, us return userCode; } - const templateFile = parameterizedRoute.startsWith('/api') ? 'apiWrapperTemplate.js' : 'pageWrapperTemplate.js'; - const templatePath = path.resolve(__dirname, `../templates/${templateFile}`); - let templateCode = fs.readFileSync(templatePath, { encoding: 'utf8' }); - - // Make sure the template is included when running `webpack watch` - this.addDependency(templatePath); + let templateCode = parameterizedRoute.startsWith('/api') ? apiWrapperTemplateCode : pageWrapperTemplateCode; // Inject the route and the path to the file we're wrapping into the template templateCode = templateCode.replace(/__ROUTE__/g, parameterizedRoute.replace(/\\/g, '\\\\')); From 14469f65284885eced651265409af0bcfdd39d3e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Sun, 8 Jan 2023 21:46:12 +0000 Subject: [PATCH 07/21] Fix source mapping --- packages/nextjs/package.json | 1 - packages/nextjs/rollup.npm.config.js | 4 +- packages/nextjs/src/config/loaders/types.ts | 8 ++ .../src/config/loaders/wrappingLoader.ts | 97 ++++++++++++------- yarn.lock | 5 - 5 files changed, 73 insertions(+), 42 deletions(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 86778341eb79..1e6ab034d851 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -18,7 +18,6 @@ }, "dependencies": { "@rollup/plugin-commonjs": "24.0.0", - "@rollup/plugin-virtual": "3.0.0", "@sentry/core": "7.29.0", "@sentry/integrations": "7.29.0", "@sentry/node": "7.29.0", diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index 8899eac8851a..d605dc65f3eb 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -36,8 +36,6 @@ export default [ ...makeNPMConfigVariants( makeBaseNPMConfig({ entrypoints: ['src/config/loaders/index.ts'], - // Needed in order to successfully import sucrase - esModuleInterop: true, packageSpecificConfig: { output: { @@ -47,7 +45,7 @@ export default [ // make it so Rollup calms down about the fact that we're combining default and named exports exports: 'named', }, - external: ['@rollup/plugin-commonjs', '@rollup/plugin-virtual', 'rollup'], + external: ['@rollup/plugin-commonjs', 'rollup'], }, }), ), diff --git a/packages/nextjs/src/config/loaders/types.ts b/packages/nextjs/src/config/loaders/types.ts index 5db902abeced..14766f077a12 100644 --- a/packages/nextjs/src/config/loaders/types.ts +++ b/packages/nextjs/src/config/loaders/types.ts @@ -1,3 +1,5 @@ +import type webpack from 'webpack'; + export type LoaderThis = { /** Path to the file being loaded */ resourcePath: string; @@ -7,6 +9,12 @@ export type LoaderThis = { // Function to add outside file used by loader to `watch` process addDependency: (filepath: string) => void; + + // Marks a loader as asynchronous + async: webpack.loader.LoaderContext['async']; + + // Return errors, code, and sourcemaps from an asynchronous loader + callback: webpack.loader.LoaderContext['callback']; } & ( | { // Loader options in Webpack 4 diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 2e01feadff73..d6f255cb9c82 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -1,11 +1,10 @@ import commonjs from '@rollup/plugin-commonjs'; -import virtual from '@rollup/plugin-virtual'; import { stringMatchesSomePattern } from '@sentry/utils'; import * as fs from 'fs'; import * as path from 'path'; import { rollup } from 'rollup'; -import { LoaderThis } from './types'; +import type { LoaderThis } from './types'; const apiWrapperTemplatePath = path.resolve(__dirname, '..', 'templates', 'apiWrapperTemplate.js'); const apiWrapperTemplateCode = fs.readFileSync(apiWrapperTemplatePath, { encoding: 'utf8' }); @@ -16,8 +15,7 @@ const pageWrapperTemplateCode = fs.readFileSync(pageWrapperTemplatePath, { encod // Just a simple placeholder to make referencing module consistent const SENTRY_WRAPPER_MODULE_NAME = 'sentry-wrapper-module'; -// needs to end in .cjs in order for the `commonjs` plugin to pick it up - unfortunately the plugin has no option to -// make this work in combination with the`virtual` plugin +// Needs to end in .cjs in order for the `commonjs` plugin to pick it up const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPPING_TARGET__.cjs'; type LoaderOptions = { @@ -31,7 +29,13 @@ type LoaderOptions = { * any data-fetching functions (`getInitialProps`, `getStaticProps`, and `getServerSideProps`) or API routes it contains * are wrapped, and then everything is re-exported. */ -export default async function wrappingLoader(this: LoaderThis, userCode: string): Promise { +export default function wrappingLoader( + this: LoaderThis, + userCode: string, + userModuleSourceMap: any, +): void { + this.async(); + // We know one or the other will be defined, depending on the version of webpack being used const { pagesDir, @@ -56,7 +60,8 @@ export default async function wrappingLoader(this: LoaderThis, us // Skip explicitly-ignored pages if (stringMatchesSomePattern(parameterizedRoute, excludeServerRoutes, true)) { - return userCode; + this.callback(null, userCode, userModuleSourceMap); + return; } let templateCode = parameterizedRoute.startsWith('/api') ? apiWrapperTemplateCode : pageWrapperTemplateCode; @@ -69,15 +74,18 @@ export default async function wrappingLoader(this: LoaderThis, us // Run the proxy module code through Rollup, in order to split the `export * from ''` out into // individual exports (which nextjs seems to require). - try { - return await wrapUserCode(templateCode, userCode); - } catch (err) { - // eslint-disable-next-line no-console - console.warn( - `[@sentry/nextjs] Could not instrument ${this.resourcePath}. An error occurred while auto-wrapping:\n${err}`, - ); - return userCode; - } + wrapUserCode(templateCode, userCode, userModuleSourceMap) + .then(({ code: wrappedCode, map: wrappedCodeSourceMap }) => { + this.callback(null, wrappedCode, wrappedCodeSourceMap); + }) + .catch(err => { + // eslint-disable-next-line no-console + console.warn( + `[@sentry/nextjs] Could not instrument ${this.resourcePath}. An error occurred while auto-wrapping:\n${err}`, + ); + this.callback(null, userCode, userModuleSourceMap); + return; + }); } /** @@ -92,26 +100,53 @@ export default async function wrappingLoader(this: LoaderThis, us * * @param wrapperCode The wrapper module code * @param userModuleCode The user module code - * @returns The wrapped user code + * @returns The wrapped user code and a source map that describes the transformations done by this function */ -async function wrapUserCode(wrapperCode: string, userModuleCode: string): Promise { +async function wrapUserCode( + wrapperCode: string, + userModuleCode: string, + userModuleSourceMap: any, +): Promise<{ code: string; map?: any }> { const rollupBuild = await rollup({ input: SENTRY_WRAPPER_MODULE_NAME, plugins: [ - // We're using virtual modules so we don't have to mess around with file paths - virtual({ - [SENTRY_WRAPPER_MODULE_NAME]: wrapperCode, - [WRAPPING_TARGET_MODULE_NAME]: userModuleCode, - }), + // We're using a simple custom plugin that virtualizes our wrapper module and the user module, so we don't have to + // mess around with file paths and so that we can pass the original user module source map to rollup so that + // rollup gives us a bundle with correct source mapping to the original file + { + name: 'virtualize-sentry-wrapper-modules', + resolveId: id => { + if (id === SENTRY_WRAPPER_MODULE_NAME || id === WRAPPING_TARGET_MODULE_NAME) { + return id; + } else { + return null; + } + }, + load(id) { + if (id === SENTRY_WRAPPER_MODULE_NAME) { + return wrapperCode; + } else if (id === WRAPPING_TARGET_MODULE_NAME) { + return { + code: userModuleCode, + map: userModuleSourceMap, // give rollup acces to original user module source map + }; + } else { + return null; + } + }, + }, // People may use `module.exports` in their API routes or page files. Next.js allows that and we also need to // handle that correctly so we let a plugin to take care of bundling cjs exports for us. - commonjs(), + commonjs({ + transformMixedEsModules: true, + sourceMap: true, + }), ], // We only want to bundle our wrapper module and the wrappee module into one, so we mark everything else as external. - external: source => source !== SENTRY_WRAPPER_MODULE_NAME && source !== WRAPPING_TARGET_MODULE_NAME, + external: sourceId => sourceId !== SENTRY_WRAPPER_MODULE_NAME && sourceId !== WRAPPING_TARGET_MODULE_NAME, // 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 @@ -138,22 +173,18 @@ async function wrapUserCode(wrapperCode: string, userModuleCode: string): Promis // externals entirely, with the result that their paths remain untouched (which is what we want). makeAbsoluteExternalsRelative: false, - onwarn: () => { + onwarn: (_warning, _warn) => { // Suppress all warnings - we don't want to bother people with this output - }, - - output: { - // TODO - interop: 'auto', - // TODO - exports: 'named', + // Might be stuff like "you have unused imports" + // _warn(_warning); // uncomment to debug }, }); const finalBundle = await rollupBuild.generate({ format: 'esm', + sourcemap: 'hidden', // put source map data in the bundle but don't generate a source map commment in the output }); // The module at index 0 is always the entrypoint, which in this case is the proxy module. - return finalBundle.output[0].code; + return finalBundle.output[0]; } diff --git a/yarn.lock b/yarn.lock index 7f4faf2247a1..fc5dbfc78b07 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3478,11 +3478,6 @@ "@rollup/pluginutils" "^3.1.0" resolve "^1.17.0" -"@rollup/plugin-virtual@3.0.0": - version "3.0.0" - resolved "https://registry.yarnpkg.com/@rollup/plugin-virtual/-/plugin-virtual-3.0.0.tgz#8c3f54b4ab4b267d9cd3dcbaedc58d4fd1deddca" - integrity sha512-K9KORe1myM62o0lKkNR4MmCxjwuAXsZEtIHpaILfv4kILXTOrXt/R2ha7PzMcCHPYdnkWPiBZK8ed4Zr3Ll5lQ== - "@rollup/pluginutils@^3.0.8", "@rollup/pluginutils@^3.0.9", "@rollup/pluginutils@^3.1.0": version "3.1.0" resolved "https://registry.yarnpkg.com/@rollup/pluginutils/-/pluginutils-3.1.0.tgz#706b4524ee6dc8b103b3c995533e5ad680c02b9b" From 77078caf8cbf70795ba0618638eb3583e28d7a29 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 9 Jan 2023 22:34:46 +0000 Subject: [PATCH 08/21] Move files around --- .../src/{index.client.ts => client/index.ts} | 0 .../client.ts => client/performance.ts} | 0 .../nextjs/src/{utils => client}/tunnelRoute.ts | 0 packages/nextjs/src/{utils => common}/_error.ts | 0 .../nextjs/src/{utils => common}/metadata.ts | 0 .../src/{utils => common}/userIntegrations.ts | 0 .../nextjs/src/{ => config}/utils/nextLogger.ts | 0 packages/nextjs/src/config/wrappers/index.ts | 9 --------- .../src/{index.server.ts => server/index.ts} | 0 .../src/{config/wrappers => server}/types.ts | 0 .../nextjs/src/{ => server}/utils/isBuild.ts | 0 .../utils/platformSupportsStreaming.ts | 0 .../wrappers => server}/utils/responseEnd.ts | 0 .../wrappers => server/utils}/wrapperUtils.ts | 0 .../wrappers => server}/withSentryAPI.ts | 0 .../withSentryGetServerSideProps.ts | 0 .../withSentryGetStaticProps.ts | 0 .../withSentryServerSideAppGetInitialProps.ts | 0 ...thSentryServerSideDocumentGetInitialProps.ts | 0 .../withSentryServerSideErrorGetInitialProps.ts | 0 .../withSentryServerSideGetInitialProps.ts | 0 packages/nextjs/src/utils/isESM.ts | 17 ----------------- packages/nextjs/src/utils/nextjsOptions.ts | 5 ----- packages/nextjs/src/utils/phases.ts | 3 --- 24 files changed, 34 deletions(-) rename packages/nextjs/src/{index.client.ts => client/index.ts} (100%) rename packages/nextjs/src/{performance/client.ts => client/performance.ts} (100%) rename packages/nextjs/src/{utils => client}/tunnelRoute.ts (100%) rename packages/nextjs/src/{utils => common}/_error.ts (100%) rename packages/nextjs/src/{utils => common}/metadata.ts (100%) rename packages/nextjs/src/{utils => common}/userIntegrations.ts (100%) rename packages/nextjs/src/{ => config}/utils/nextLogger.ts (100%) delete mode 100644 packages/nextjs/src/config/wrappers/index.ts rename packages/nextjs/src/{index.server.ts => server/index.ts} (100%) rename packages/nextjs/src/{config/wrappers => server}/types.ts (100%) rename packages/nextjs/src/{ => server}/utils/isBuild.ts (100%) rename packages/nextjs/src/{ => server}/utils/platformSupportsStreaming.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/utils/responseEnd.ts (100%) rename packages/nextjs/src/{config/wrappers => server/utils}/wrapperUtils.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/withSentryAPI.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/withSentryGetServerSideProps.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/withSentryGetStaticProps.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/withSentryServerSideAppGetInitialProps.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/withSentryServerSideDocumentGetInitialProps.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/withSentryServerSideErrorGetInitialProps.ts (100%) rename packages/nextjs/src/{config/wrappers => server}/withSentryServerSideGetInitialProps.ts (100%) delete mode 100644 packages/nextjs/src/utils/isESM.ts delete mode 100644 packages/nextjs/src/utils/nextjsOptions.ts delete mode 100644 packages/nextjs/src/utils/phases.ts diff --git a/packages/nextjs/src/index.client.ts b/packages/nextjs/src/client/index.ts similarity index 100% rename from packages/nextjs/src/index.client.ts rename to packages/nextjs/src/client/index.ts diff --git a/packages/nextjs/src/performance/client.ts b/packages/nextjs/src/client/performance.ts similarity index 100% rename from packages/nextjs/src/performance/client.ts rename to packages/nextjs/src/client/performance.ts diff --git a/packages/nextjs/src/utils/tunnelRoute.ts b/packages/nextjs/src/client/tunnelRoute.ts similarity index 100% rename from packages/nextjs/src/utils/tunnelRoute.ts rename to packages/nextjs/src/client/tunnelRoute.ts diff --git a/packages/nextjs/src/utils/_error.ts b/packages/nextjs/src/common/_error.ts similarity index 100% rename from packages/nextjs/src/utils/_error.ts rename to packages/nextjs/src/common/_error.ts diff --git a/packages/nextjs/src/utils/metadata.ts b/packages/nextjs/src/common/metadata.ts similarity index 100% rename from packages/nextjs/src/utils/metadata.ts rename to packages/nextjs/src/common/metadata.ts diff --git a/packages/nextjs/src/utils/userIntegrations.ts b/packages/nextjs/src/common/userIntegrations.ts similarity index 100% rename from packages/nextjs/src/utils/userIntegrations.ts rename to packages/nextjs/src/common/userIntegrations.ts diff --git a/packages/nextjs/src/utils/nextLogger.ts b/packages/nextjs/src/config/utils/nextLogger.ts similarity index 100% rename from packages/nextjs/src/utils/nextLogger.ts rename to packages/nextjs/src/config/utils/nextLogger.ts diff --git a/packages/nextjs/src/config/wrappers/index.ts b/packages/nextjs/src/config/wrappers/index.ts deleted file mode 100644 index aa406b970e77..000000000000 --- a/packages/nextjs/src/config/wrappers/index.ts +++ /dev/null @@ -1,9 +0,0 @@ -export type { AugmentedNextApiResponse, NextApiHandler, WrappedNextApiHandler } from './types'; - -export { withSentryGetStaticProps } from './withSentryGetStaticProps'; -export { withSentryServerSideGetInitialProps } from './withSentryServerSideGetInitialProps'; -export { withSentryServerSideAppGetInitialProps } from './withSentryServerSideAppGetInitialProps'; -export { withSentryServerSideDocumentGetInitialProps } from './withSentryServerSideDocumentGetInitialProps'; -export { withSentryServerSideErrorGetInitialProps } from './withSentryServerSideErrorGetInitialProps'; -export { withSentryGetServerSideProps } from './withSentryGetServerSideProps'; -export { withSentry, withSentryAPI } from './withSentryAPI'; diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/server/index.ts similarity index 100% rename from packages/nextjs/src/index.server.ts rename to packages/nextjs/src/server/index.ts diff --git a/packages/nextjs/src/config/wrappers/types.ts b/packages/nextjs/src/server/types.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/types.ts rename to packages/nextjs/src/server/types.ts diff --git a/packages/nextjs/src/utils/isBuild.ts b/packages/nextjs/src/server/utils/isBuild.ts similarity index 100% rename from packages/nextjs/src/utils/isBuild.ts rename to packages/nextjs/src/server/utils/isBuild.ts diff --git a/packages/nextjs/src/utils/platformSupportsStreaming.ts b/packages/nextjs/src/server/utils/platformSupportsStreaming.ts similarity index 100% rename from packages/nextjs/src/utils/platformSupportsStreaming.ts rename to packages/nextjs/src/server/utils/platformSupportsStreaming.ts diff --git a/packages/nextjs/src/config/wrappers/utils/responseEnd.ts b/packages/nextjs/src/server/utils/responseEnd.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/utils/responseEnd.ts rename to packages/nextjs/src/server/utils/responseEnd.ts diff --git a/packages/nextjs/src/config/wrappers/wrapperUtils.ts b/packages/nextjs/src/server/utils/wrapperUtils.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/wrapperUtils.ts rename to packages/nextjs/src/server/utils/wrapperUtils.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryAPI.ts b/packages/nextjs/src/server/withSentryAPI.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/withSentryAPI.ts rename to packages/nextjs/src/server/withSentryAPI.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts b/packages/nextjs/src/server/withSentryGetServerSideProps.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/withSentryGetServerSideProps.ts rename to packages/nextjs/src/server/withSentryGetServerSideProps.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts b/packages/nextjs/src/server/withSentryGetStaticProps.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/withSentryGetStaticProps.ts rename to packages/nextjs/src/server/withSentryGetStaticProps.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideAppGetInitialProps.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/withSentryServerSideAppGetInitialProps.ts rename to packages/nextjs/src/server/withSentryServerSideAppGetInitialProps.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideDocumentGetInitialProps.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/withSentryServerSideDocumentGetInitialProps.ts rename to packages/nextjs/src/server/withSentryServerSideDocumentGetInitialProps.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideErrorGetInitialProps.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/withSentryServerSideErrorGetInitialProps.ts rename to packages/nextjs/src/server/withSentryServerSideErrorGetInitialProps.ts diff --git a/packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideGetInitialProps.ts similarity index 100% rename from packages/nextjs/src/config/wrappers/withSentryServerSideGetInitialProps.ts rename to packages/nextjs/src/server/withSentryServerSideGetInitialProps.ts diff --git a/packages/nextjs/src/utils/isESM.ts b/packages/nextjs/src/utils/isESM.ts deleted file mode 100644 index 30ce7f4e87de..000000000000 --- a/packages/nextjs/src/utils/isESM.ts +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Determine if the given source code represents a file written using ES6 modules. - * - * The regexes used are from https://github.com/component/is-module, which got them from - * https://github.com/formatjs/js-module-formats, which says it got them from - * https://github.com/ModuleLoader/es-module-loader, though the originals are now nowhere to be found. - * - * @param moduleSource The source code of the module - * @returns True if the module contains ESM-patterned code, false otherwise. - */ -export function isESM(moduleSource: string): boolean { - const importExportRegex = - /(?:^\s*|[}{();,\n]\s*)(import\s+['"]|(import|module)\s+[^"'()\n;]+\s+from\s+['"]|export\s+(\*|\{|default|function|var|const|let|[_$a-zA-Z\xA0-\uFFFF][_$a-zA-Z0-9\xA0-\uFFFF]*))/; - const exportStarRegex = /(?:^\s*|[}{();,\n]\s*)(export\s*\*\s*from\s*(?:'([^']+)'|"([^"]+)"))/; - - return importExportRegex.test(moduleSource) || exportStarRegex.test(moduleSource); -} diff --git a/packages/nextjs/src/utils/nextjsOptions.ts b/packages/nextjs/src/utils/nextjsOptions.ts deleted file mode 100644 index ded1fe3f1d23..000000000000 --- a/packages/nextjs/src/utils/nextjsOptions.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { NodeOptions } from '@sentry/node'; -import { BrowserOptions } from '@sentry/react'; -import { Options } from '@sentry/types'; - -export type NextjsOptions = Options | BrowserOptions | NodeOptions; diff --git a/packages/nextjs/src/utils/phases.ts b/packages/nextjs/src/utils/phases.ts deleted file mode 100644 index 17e11d404be1..000000000000 --- a/packages/nextjs/src/utils/phases.ts +++ /dev/null @@ -1,3 +0,0 @@ -export const NEXT_PHASE_PRODUCTION_BUILD = 'phase-production-build'; -export const NEXT_PHASE_PRODUCTION_SERVER = 'phase-production-server'; -export const NEXT_PHASE_DEVELOPMENT_SERVER = 'phase-development-server'; From 7417c541e3db3a62c14a47ad5749875fd113a0d8 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 9 Jan 2023 22:45:11 +0000 Subject: [PATCH 09/21] Move file --- packages/nextjs/src/{config => server}/utils/nextLogger.ts | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/nextjs/src/{config => server}/utils/nextLogger.ts (100%) diff --git a/packages/nextjs/src/config/utils/nextLogger.ts b/packages/nextjs/src/server/utils/nextLogger.ts similarity index 100% rename from packages/nextjs/src/config/utils/nextLogger.ts rename to packages/nextjs/src/server/utils/nextLogger.ts From 22a20091a46ba086a0da39fed4afec6f3a3c0b80 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 9 Jan 2023 23:00:35 +0000 Subject: [PATCH 10/21] Fix up imports --- packages/nextjs/src/client/index.ts | 19 +++++---- packages/nextjs/src/client/tunnelRoute.ts | 5 +-- .../config/templates/apiWrapperTemplate.ts | 2 +- .../nextjs/src/config/withSentryConfig.ts | 5 ++- packages/nextjs/src/server/index.ts | 39 +++++++------------ packages/nextjs/src/server/utils/isBuild.ts | 4 +- .../nextjs/src/server/utils/wrapperUtils.ts | 4 +- packages/nextjs/src/server/withSentryAPI.ts | 4 +- .../server/withSentryGetServerSideProps.ts | 8 +++- .../src/server/withSentryGetStaticProps.ts | 4 +- .../withSentryServerSideAppGetInitialProps.ts | 8 +++- ...SentryServerSideDocumentGetInitialProps.ts | 4 +- ...ithSentryServerSideErrorGetInitialProps.ts | 8 +++- .../withSentryServerSideGetInitialProps.ts | 8 +++- 14 files changed, 63 insertions(+), 59 deletions(-) diff --git a/packages/nextjs/src/client/index.ts b/packages/nextjs/src/client/index.ts index 98ea9928b8ed..08f5b113ba74 100644 --- a/packages/nextjs/src/client/index.ts +++ b/packages/nextjs/src/client/index.ts @@ -1,17 +1,16 @@ import { RewriteFrames } from '@sentry/integrations'; -import { configureScope, init as reactInit, Integrations } from '@sentry/react'; +import { BrowserOptions, configureScope, init as reactInit, Integrations } from '@sentry/react'; import { BrowserTracing, defaultRequestInstrumentationOptions, hasTracingEnabled } from '@sentry/tracing'; import { EventProcessor } from '@sentry/types'; -import { nextRouterInstrumentation } from './performance/client'; -import { buildMetadata } from './utils/metadata'; -import { NextjsOptions } from './utils/nextjsOptions'; -import { applyTunnelRouteOption } from './utils/tunnelRoute'; -import { addOrUpdateIntegration } from './utils/userIntegrations'; +import { buildMetadata } from '../common/metadata'; +import { addOrUpdateIntegration } from '../common/userIntegrations'; +import { nextRouterInstrumentation } from './performance'; +import { applyTunnelRouteOption } from './tunnelRoute'; export * from '@sentry/react'; -export { nextRouterInstrumentation } from './performance/client'; -export { captureUnderscoreErrorException } from './utils/_error'; +export { nextRouterInstrumentation } from './performance'; +export { captureUnderscoreErrorException } from '../common/_error'; export { Integrations }; @@ -35,7 +34,7 @@ const globalWithInjectedValues = global as typeof global & { }; /** Inits the Sentry NextJS SDK on the browser with the React SDK. */ -export function init(options: NextjsOptions): void { +export function init(options: BrowserOptions): void { applyTunnelRouteOption(options); buildMetadata(options, ['nextjs', 'react']); options.environment = options.environment || process.env.NODE_ENV; @@ -52,7 +51,7 @@ export function init(options: NextjsOptions): void { }); } -function addClientIntegrations(options: NextjsOptions): void { +function addClientIntegrations(options: BrowserOptions): void { let integrations = options.integrations || []; // This value is injected at build time, based on the output directory specified in the build config. Though a default diff --git a/packages/nextjs/src/client/tunnelRoute.ts b/packages/nextjs/src/client/tunnelRoute.ts index 16d4ce9b71cc..29ad48325bb9 100644 --- a/packages/nextjs/src/client/tunnelRoute.ts +++ b/packages/nextjs/src/client/tunnelRoute.ts @@ -1,7 +1,6 @@ +import { BrowserOptions } from '@sentry/react'; import { dsnFromString, logger } from '@sentry/utils'; -import { NextjsOptions } from './nextjsOptions'; - const globalWithInjectedValues = global as typeof global & { __sentryRewritesTunnelPath__?: string; }; @@ -9,7 +8,7 @@ const globalWithInjectedValues = global as typeof global & { /** * Applies the `tunnel` option to the Next.js SDK options based on `withSentryConfig`'s `tunnelRoute` option. */ -export function applyTunnelRouteOption(options: NextjsOptions): void { +export function applyTunnelRouteOption(options: BrowserOptions): void { const tunnelRouteOption = globalWithInjectedValues.__sentryRewritesTunnelPath__; if (tunnelRouteOption && options.dsn) { const dsnComponents = dsnFromString(options.dsn); diff --git a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts index a16c3a0c7666..65fe3b240adc 100644 --- a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts @@ -14,7 +14,7 @@ import type { PageConfig } from 'next'; // We import this from `wrappers` rather than directly from `next` because our version can work simultaneously with // multiple versions of next. See note in `wrappers/types` for more. -import type { NextApiHandler } from '../wrappers'; +import type { NextApiHandler } from '../../server/types'; type NextApiModule = ( | { diff --git a/packages/nextjs/src/config/withSentryConfig.ts b/packages/nextjs/src/config/withSentryConfig.ts index 2b8d12792470..5c16b82286c7 100644 --- a/packages/nextjs/src/config/withSentryConfig.ts +++ b/packages/nextjs/src/config/withSentryConfig.ts @@ -1,4 +1,5 @@ -import { NEXT_PHASE_DEVELOPMENT_SERVER, NEXT_PHASE_PRODUCTION_BUILD } from '../utils/phases'; +import { PHASE_DEVELOPMENT_SERVER, PHASE_PRODUCTION_BUILD } from 'next/constants'; + import type { ExportedNextConfig, NextConfigFunction, @@ -50,7 +51,7 @@ function getFinalConfigObject( // In order to prevent all of our build-time code from being bundled in people's route-handling serverless functions, // we exclude `webpack.ts` and all of its dependencies from nextjs's `@vercel/nft` filetracing. We therefore need to // make sure that we only require it at build time or in development mode. - if (phase === NEXT_PHASE_PRODUCTION_BUILD || phase === NEXT_PHASE_DEVELOPMENT_SERVER) { + if (phase === PHASE_PRODUCTION_BUILD || phase === PHASE_DEVELOPMENT_SERVER) { // eslint-disable-next-line @typescript-eslint/no-var-requires const { constructWebpackConfigFunction } = require('./webpack'); return { diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 0500bd802300..0f1c50ce0e6b 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -1,24 +1,18 @@ import { Carrier, getHubFromCarrier, getMainCarrier } from '@sentry/core'; import { RewriteFrames } from '@sentry/integrations'; -import { configureScope, getCurrentHub, init as nodeInit, Integrations } from '@sentry/node'; +import { configureScope, getCurrentHub, init as nodeInit, Integrations, NodeOptions } from '@sentry/node'; import { hasTracingEnabled } from '@sentry/tracing'; import { EventProcessor } from '@sentry/types'; import { escapeStringForRegex, logger } from '@sentry/utils'; import * as domainModule from 'domain'; import * as path from 'path'; +import { buildMetadata } from '../common/metadata'; +import { addOrUpdateIntegration, IntegrationWithExclusionOption } from '../common/userIntegrations'; import { isBuild } from './utils/isBuild'; -import { buildMetadata } from './utils/metadata'; -import { NextjsOptions } from './utils/nextjsOptions'; -import { addOrUpdateIntegration, IntegrationWithExclusionOption } from './utils/userIntegrations'; export * from '@sentry/node'; -export { captureUnderscoreErrorException } from './utils/_error'; - -// Exporting the Replay integration also from index.server.ts because TS only recognizes types from index.server.ts -// If we didn't export this, TS would complain that it can't find `Sentry.Replay` in the package, -// causing a build failure, when users initialize Replay in their sentry.client.config.js/ts file. -export { Replay } from './index.client'; +export { captureUnderscoreErrorException } from '../common/_error'; // Here we want to make sure to only include what doesn't have browser specifics // because or SSR of next.js we can only use this. @@ -40,7 +34,7 @@ export const IS_BUILD = isBuild(); const IS_VERCEL = !!process.env.VERCEL; /** Inits the Sentry NextJS SDK on node. */ -export function init(options: NextjsOptions): void { +export function init(options: NodeOptions): void { if (__DEBUG_BUILD__ && options.debug) { logger.enable(); } @@ -107,7 +101,7 @@ function sdkAlreadyInitialized(): boolean { return !!hub.getClient(); } -function addServerIntegrations(options: NextjsOptions): void { +function addServerIntegrations(options: NodeOptions): void { let integrations = options.integrations || []; // This value is injected at build time, based on the output directory specified in the build config. Though a default @@ -146,21 +140,16 @@ function addServerIntegrations(options: NextjsOptions): void { // TODO (v8): Remove this /** - * @deprecated Use the constant `IS_BUILD` instead. + * @deprecated */ const deprecatedIsBuild = (): boolean => isBuild(); // eslint-disable-next-line deprecation/deprecation export { deprecatedIsBuild as isBuild }; -export type { SentryWebpackPluginOptions } from './config/types'; -export { withSentryConfig } from './config/withSentryConfig'; -export { - withSentryGetServerSideProps, - withSentryGetStaticProps, - withSentryServerSideGetInitialProps, - withSentryServerSideAppGetInitialProps, - withSentryServerSideDocumentGetInitialProps, - withSentryServerSideErrorGetInitialProps, - withSentryAPI, - withSentry, -} from './config/wrappers'; +export { withSentryGetStaticProps } from './withSentryGetStaticProps'; +export { withSentryServerSideGetInitialProps } from './withSentryServerSideGetInitialProps'; +export { withSentryServerSideAppGetInitialProps } from './withSentryServerSideAppGetInitialProps'; +export { withSentryServerSideDocumentGetInitialProps } from './withSentryServerSideDocumentGetInitialProps'; +export { withSentryServerSideErrorGetInitialProps } from './withSentryServerSideErrorGetInitialProps'; +export { withSentryGetServerSideProps } from './withSentryGetServerSideProps'; +export { withSentry, withSentryAPI } from './withSentryAPI'; diff --git a/packages/nextjs/src/server/utils/isBuild.ts b/packages/nextjs/src/server/utils/isBuild.ts index 0e6f6691bf44..92b9808f75b9 100644 --- a/packages/nextjs/src/server/utils/isBuild.ts +++ b/packages/nextjs/src/server/utils/isBuild.ts @@ -1,8 +1,8 @@ -import { NEXT_PHASE_PRODUCTION_BUILD } from './phases'; +import { PHASE_PRODUCTION_BUILD } from 'next/constants'; /** * Decide if the currently running process is part of the build phase or happening at runtime. */ export function isBuild(): boolean { - return process.env.NEXT_PHASE === NEXT_PHASE_PRODUCTION_BUILD; + return process.env.NEXT_PHASE === PHASE_PRODUCTION_BUILD; } diff --git a/packages/nextjs/src/server/utils/wrapperUtils.ts b/packages/nextjs/src/server/utils/wrapperUtils.ts index 18665040d45d..8883f1163f57 100644 --- a/packages/nextjs/src/server/utils/wrapperUtils.ts +++ b/packages/nextjs/src/server/utils/wrapperUtils.ts @@ -5,8 +5,8 @@ import { baggageHeaderToDynamicSamplingContext, extractTraceparentData } from '@ import * as domain from 'domain'; import { IncomingMessage, ServerResponse } from 'http'; -import { platformSupportsStreaming } from '../../utils/platformSupportsStreaming'; -import { autoEndTransactionOnResponseEnd, flushQueue } from './utils/responseEnd'; +import { platformSupportsStreaming } from './platformSupportsStreaming'; +import { autoEndTransactionOnResponseEnd, flushQueue } from './responseEnd'; declare module 'http' { interface IncomingMessage { diff --git a/packages/nextjs/src/server/withSentryAPI.ts b/packages/nextjs/src/server/withSentryAPI.ts index 561939685aa0..acb15a4c5514 100644 --- a/packages/nextjs/src/server/withSentryAPI.ts +++ b/packages/nextjs/src/server/withSentryAPI.ts @@ -11,9 +11,9 @@ import { } from '@sentry/utils'; import * as domain from 'domain'; -import { formatAsCode, nextLogger } from '../../utils/nextLogger'; -import { platformSupportsStreaming } from '../../utils/platformSupportsStreaming'; import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler, WrappedNextApiHandler } from './types'; +import { formatAsCode, nextLogger } from './utils/nextLogger'; +import { platformSupportsStreaming } from './utils/platformSupportsStreaming'; import { autoEndTransactionOnResponseEnd, finishTransaction, flushQueue } from './utils/responseEnd'; /** diff --git a/packages/nextjs/src/server/withSentryGetServerSideProps.ts b/packages/nextjs/src/server/withSentryGetServerSideProps.ts index 8aa2244ea90e..76c8b848712a 100644 --- a/packages/nextjs/src/server/withSentryGetServerSideProps.ts +++ b/packages/nextjs/src/server/withSentryGetServerSideProps.ts @@ -2,8 +2,12 @@ import { hasTracingEnabled } from '@sentry/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import { GetServerSideProps } from 'next'; -import { isBuild } from '../../utils/isBuild'; -import { getTransactionFromRequest, withErrorInstrumentation, withTracedServerSideDataFetcher } from './wrapperUtils'; +import { isBuild } from './utils/isBuild'; +import { + getTransactionFromRequest, + withErrorInstrumentation, + withTracedServerSideDataFetcher, +} from './utils/wrapperUtils'; /** * Create a wrapped version of the user's exported `getServerSideProps` function diff --git a/packages/nextjs/src/server/withSentryGetStaticProps.ts b/packages/nextjs/src/server/withSentryGetStaticProps.ts index 396c57c0652c..7220480b4117 100644 --- a/packages/nextjs/src/server/withSentryGetStaticProps.ts +++ b/packages/nextjs/src/server/withSentryGetStaticProps.ts @@ -1,7 +1,7 @@ import { GetStaticProps } from 'next'; -import { isBuild } from '../../utils/isBuild'; -import { callDataFetcherTraced, withErrorInstrumentation } from './wrapperUtils'; +import { isBuild } from './utils/isBuild'; +import { callDataFetcherTraced, withErrorInstrumentation } from './utils/wrapperUtils'; type Props = { [key: string]: unknown }; diff --git a/packages/nextjs/src/server/withSentryServerSideAppGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideAppGetInitialProps.ts index a136bbcdc96a..89ed3af12dc7 100644 --- a/packages/nextjs/src/server/withSentryServerSideAppGetInitialProps.ts +++ b/packages/nextjs/src/server/withSentryServerSideAppGetInitialProps.ts @@ -2,8 +2,12 @@ import { hasTracingEnabled } from '@sentry/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import App from 'next/app'; -import { isBuild } from '../../utils/isBuild'; -import { getTransactionFromRequest, withErrorInstrumentation, withTracedServerSideDataFetcher } from './wrapperUtils'; +import { isBuild } from './utils/isBuild'; +import { + getTransactionFromRequest, + withErrorInstrumentation, + withTracedServerSideDataFetcher, +} from './utils/wrapperUtils'; type AppGetInitialProps = typeof App['getInitialProps']; diff --git a/packages/nextjs/src/server/withSentryServerSideDocumentGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideDocumentGetInitialProps.ts index a04aa11a3398..d08222b6064c 100644 --- a/packages/nextjs/src/server/withSentryServerSideDocumentGetInitialProps.ts +++ b/packages/nextjs/src/server/withSentryServerSideDocumentGetInitialProps.ts @@ -1,8 +1,8 @@ import { hasTracingEnabled } from '@sentry/tracing'; import Document from 'next/document'; -import { isBuild } from '../../utils/isBuild'; -import { withErrorInstrumentation, withTracedServerSideDataFetcher } from './wrapperUtils'; +import { isBuild } from './utils/isBuild'; +import { withErrorInstrumentation, withTracedServerSideDataFetcher } from './utils/wrapperUtils'; type DocumentGetInitialProps = typeof Document.getInitialProps; diff --git a/packages/nextjs/src/server/withSentryServerSideErrorGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideErrorGetInitialProps.ts index 8ceae817e1bc..a0cadad23fa3 100644 --- a/packages/nextjs/src/server/withSentryServerSideErrorGetInitialProps.ts +++ b/packages/nextjs/src/server/withSentryServerSideErrorGetInitialProps.ts @@ -3,8 +3,12 @@ import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import { NextPageContext } from 'next'; import { ErrorProps } from 'next/error'; -import { isBuild } from '../../utils/isBuild'; -import { getTransactionFromRequest, withErrorInstrumentation, withTracedServerSideDataFetcher } from './wrapperUtils'; +import { isBuild } from './utils/isBuild'; +import { + getTransactionFromRequest, + withErrorInstrumentation, + withTracedServerSideDataFetcher, +} from './utils/wrapperUtils'; type ErrorGetInitialProps = (context: NextPageContext) => Promise; diff --git a/packages/nextjs/src/server/withSentryServerSideGetInitialProps.ts b/packages/nextjs/src/server/withSentryServerSideGetInitialProps.ts index 06d0f7766fec..d3760d792b9d 100644 --- a/packages/nextjs/src/server/withSentryServerSideGetInitialProps.ts +++ b/packages/nextjs/src/server/withSentryServerSideGetInitialProps.ts @@ -2,8 +2,12 @@ import { hasTracingEnabled } from '@sentry/tracing'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import { NextPage } from 'next'; -import { isBuild } from '../../utils/isBuild'; -import { getTransactionFromRequest, withErrorInstrumentation, withTracedServerSideDataFetcher } from './wrapperUtils'; +import { isBuild } from './utils/isBuild'; +import { + getTransactionFromRequest, + withErrorInstrumentation, + withTracedServerSideDataFetcher, +} from './utils/wrapperUtils'; type GetInitialProps = Required['getInitialProps']; From 7774fa50a8b9df250270bb5930f7577fa7a71fa2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 9 Jan 2023 23:48:02 +0000 Subject: [PATCH 11/21] Re-export stuff --- packages/nextjs/package.json | 7 +++--- packages/nextjs/rollup.npm.config.js | 4 +-- packages/nextjs/src/config/index.ts | 2 ++ .../nextjs/src/config/withSentryConfig.ts | 3 +-- packages/nextjs/src/index.ts | 1 + packages/nextjs/src/index.types.ts | 25 +++++++++++++++++++ 6 files changed, 34 insertions(+), 8 deletions(-) create mode 100644 packages/nextjs/src/config/index.ts create mode 100644 packages/nextjs/src/index.ts create mode 100644 packages/nextjs/src/index.types.ts diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 1e6ab034d851..27d2f4abece4 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -9,10 +9,9 @@ "engines": { "node": ">=8" }, - "main": "build/cjs/index.server.js", - "module": "build/esm/index.server.js", - "browser": "build/esm/index.client.js", - "types": "build/types/index.server.d.ts", + "main": "build/cjs/index.js", + "module": "build/esm/index.js", + "types": "build/types/index.types.d.ts", "publishConfig": { "access": "public" }, diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index d605dc65f3eb..d9d46c369da8 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -5,11 +5,11 @@ export default [ 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/config/webpack.ts'], + entrypoints: ['src/index.ts', 'src/config/index.ts', 'src/client/index.ts', 'src/server/index.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'] }, + packageSpecificConfig: { external: ['next/router', 'next/constants'] }, }), ), ...makeNPMConfigVariants( diff --git a/packages/nextjs/src/config/index.ts b/packages/nextjs/src/config/index.ts new file mode 100644 index 000000000000..f537125a75f3 --- /dev/null +++ b/packages/nextjs/src/config/index.ts @@ -0,0 +1,2 @@ +export type { SentryWebpackPluginOptions } from './types'; +export { withSentryConfig } from './withSentryConfig'; diff --git a/packages/nextjs/src/config/withSentryConfig.ts b/packages/nextjs/src/config/withSentryConfig.ts index 5c16b82286c7..c3e15a6e66e8 100644 --- a/packages/nextjs/src/config/withSentryConfig.ts +++ b/packages/nextjs/src/config/withSentryConfig.ts @@ -7,6 +7,7 @@ import type { NextConfigObjectWithSentry, SentryWebpackPluginOptions, } from './types'; +import { constructWebpackConfigFunction } from './webpack'; /** * Add Sentry options to the config to be exported from the user's `next.config.js` file. @@ -52,8 +53,6 @@ function getFinalConfigObject( // we exclude `webpack.ts` and all of its dependencies from nextjs's `@vercel/nft` filetracing. We therefore need to // make sure that we only require it at build time or in development mode. if (phase === PHASE_PRODUCTION_BUILD || phase === PHASE_DEVELOPMENT_SERVER) { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { constructWebpackConfigFunction } = require('./webpack'); return { ...userNextConfigObject, webpack: constructWebpackConfigFunction(userNextConfigObject, userSentryWebpackPluginOptions, userSentryOptions), diff --git a/packages/nextjs/src/index.ts b/packages/nextjs/src/index.ts new file mode 100644 index 000000000000..f03c2281a914 --- /dev/null +++ b/packages/nextjs/src/index.ts @@ -0,0 +1 @@ +export * from './config'; diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts new file mode 100644 index 000000000000..70912babcd63 --- /dev/null +++ b/packages/nextjs/src/index.types.ts @@ -0,0 +1,25 @@ +/* eslint-disable import/export */ + +export * from './config'; +export * from './client'; +export * from './server'; + +import type { Integration, Options, StackParser } from '@sentry/types'; + +import type { BrowserOptions } from './client'; +import * as clientSdk from './client'; +import type { NodeOptions } from './server'; +import * as serverSdk from './server'; + +export declare function init(options: Options | BrowserOptions | NodeOptions): void; + +export const Integrations = { ...clientSdk.Integrations, ...serverSdk.Integrations }; + +export declare const defaultIntegrations: Integration[]; +export declare const defaultStackParser: StackParser; + +declare const runtime: 'client' | 'server'; + +export const close = runtime === 'client' ? clientSdk.close : serverSdk.close; +export const flush = runtime === 'client' ? clientSdk.flush : serverSdk.flush; +export const lastEventId = runtime === 'client' ? clientSdk.lastEventId : serverSdk.lastEventId; From 339f4bcf38984e89b05702813044d45a07d84002 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 11:58:04 +0000 Subject: [PATCH 12/21] Export server from main export --- packages/nextjs/package.json | 1 + packages/nextjs/rollup.npm.config.js | 2 +- packages/nextjs/src/index.ts | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 27d2f4abece4..15a05ca0553e 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -11,6 +11,7 @@ }, "main": "build/cjs/index.js", "module": "build/esm/index.js", + "browser": "build/esm/client/index.js", "types": "build/types/index.types.d.ts", "publishConfig": { "access": "public" diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index d9d46c369da8..f7716e5956c1 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -5,7 +5,7 @@ export default [ 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.ts', 'src/config/index.ts', 'src/client/index.ts', 'src/server/index.ts'], + entrypoints: ['src/index.ts', 'src/client/index.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) diff --git a/packages/nextjs/src/index.ts b/packages/nextjs/src/index.ts index f03c2281a914..133b6ecf1da0 100644 --- a/packages/nextjs/src/index.ts +++ b/packages/nextjs/src/index.ts @@ -1 +1,2 @@ export * from './config'; +export * from './server'; From 74d845e863f9edc120723488da93c86a72bfe21c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 12:18:51 +0000 Subject: [PATCH 13/21] Fix madge --- packages/nextjs/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 15a05ca0553e..4faa96f252f0 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -56,7 +56,7 @@ "build:rollup:watch": "nodemon --ext ts --watch src scripts/buildRollup.ts", "build:types:watch": "tsc -p tsconfig.types.json --watch", "build:npm": "ts-node ../../scripts/prepack.ts && npm pack ./build", - "circularDepCheck": "madge --circular src/index.client.ts && madge --circular --exclude 'config/types\\.ts' src/index.server.ts # see https://github.com/pahen/madge/issues/306", + "circularDepCheck": "madge --circular src/index.ts && madge --circular src/client/index.ts && madge --circular src/index.types.ts", "clean": "rimraf build coverage sentry-nextjs-*.tgz", "fix": "run-s fix:eslint fix:prettier", "fix:eslint": "eslint . --format stylish --fix", From 6b033d461f019caffc739a54b89954df76fd7bf9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 13:52:58 +0000 Subject: [PATCH 14/21] Fix up some tests --- .../nextjs/test/config/withSentry.test.ts | 3 +- packages/nextjs/test/config/wrappers.test.ts | 11 +-- packages/nextjs/test/index.client.test.ts | 4 +- packages/nextjs/test/index.server.test.ts | 2 +- .../nextjs/test/performance/client.test.ts | 2 +- packages/nextjs/test/utils/isESM.test.ts | 69 ------------------- .../nextjs/test/utils/tunnelRoute.test.ts | 13 ++-- .../test/utils/userIntegrations.test.ts | 4 +- 8 files changed, 16 insertions(+), 92 deletions(-) delete mode 100644 packages/nextjs/test/utils/isESM.test.ts diff --git a/packages/nextjs/test/config/withSentry.test.ts b/packages/nextjs/test/config/withSentry.test.ts index fa468b32670b..0ff2dca6018f 100644 --- a/packages/nextjs/test/config/withSentry.test.ts +++ b/packages/nextjs/test/config/withSentry.test.ts @@ -3,7 +3,8 @@ import * as Sentry from '@sentry/node'; import { Client, ClientOptions } from '@sentry/types'; import { NextApiHandler, NextApiRequest, NextApiResponse } from 'next'; -import { AugmentedNextApiResponse, withSentry, WrappedNextApiHandler } from '../../src/config/wrappers'; +import { withSentry } from '../../src/server'; +import type { AugmentedNextApiResponse, WrappedNextApiHandler } from '../../src/server/types'; const FLUSH_DURATION = 200; diff --git a/packages/nextjs/test/config/wrappers.test.ts b/packages/nextjs/test/config/wrappers.test.ts index abbefd93c3e8..ba742869ac49 100644 --- a/packages/nextjs/test/config/wrappers.test.ts +++ b/packages/nextjs/test/config/wrappers.test.ts @@ -2,16 +2,7 @@ import * as SentryCore from '@sentry/core'; import * as SentryTracing from '@sentry/tracing'; import { IncomingMessage, ServerResponse } from 'http'; -import { - withSentryGetServerSideProps, - withSentryServerSideGetInitialProps, - // TODO: Leaving `withSentryGetStaticProps` out for now until we figure out what to do with it - // withSentryGetStaticProps, - // TODO: Leaving these out for now until we figure out pages with no data fetchers - // withSentryServerSideAppGetInitialProps, - // withSentryServerSideDocumentGetInitialProps, - // withSentryServerSideErrorGetInitialProps, -} from '../../src/config/wrappers'; +import { withSentryGetServerSideProps, withSentryServerSideGetInitialProps } from '../../src/server'; const startTransactionSpy = jest.spyOn(SentryCore, 'startTransaction'); diff --git a/packages/nextjs/test/index.client.test.ts b/packages/nextjs/test/index.client.test.ts index 9ed02519e722..4a7577c21a33 100644 --- a/packages/nextjs/test/index.client.test.ts +++ b/packages/nextjs/test/index.client.test.ts @@ -6,8 +6,8 @@ import { Integration } from '@sentry/types'; import { logger } from '@sentry/utils'; import { JSDOM } from 'jsdom'; -import { init, Integrations, nextRouterInstrumentation } from '../src/index.client'; -import { UserIntegrationsFunction } from '../src/utils/userIntegrations'; +import { init, Integrations, nextRouterInstrumentation } from '../src/client'; +import { UserIntegrationsFunction } from '../src/common/userIntegrations'; const { BrowserTracing } = TracingIntegrations; diff --git a/packages/nextjs/test/index.server.test.ts b/packages/nextjs/test/index.server.test.ts index 232d3b746f1e..56ef03377df6 100644 --- a/packages/nextjs/test/index.server.test.ts +++ b/packages/nextjs/test/index.server.test.ts @@ -4,7 +4,7 @@ import { Integration } from '@sentry/types'; import { GLOBAL_OBJ, logger } from '@sentry/utils'; import * as domain from 'domain'; -import { init } from '../src/index.server'; +import { init } from '../src/index'; const { Integrations } = SentryNode; diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index 590a8254e109..1cd7d0e787a7 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -4,7 +4,7 @@ import { JSDOM } from 'jsdom'; import { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils'; import { default as Router } from 'next/router'; -import { nextRouterInstrumentation } from '../../src/performance/client'; +import { nextRouterInstrumentation } from '../../src/client/performance'; const globalObject = WINDOW as typeof WINDOW & { __BUILD_MANIFEST?: { diff --git a/packages/nextjs/test/utils/isESM.test.ts b/packages/nextjs/test/utils/isESM.test.ts deleted file mode 100644 index 22169377e730..000000000000 --- a/packages/nextjs/test/utils/isESM.test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import { isESM } from '../../src/utils/isESM'; - -// Based on https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import -describe('import syntax', function () { - it('recognizes import syntax', function () { - expect(isESM("import dogs from 'dogs';")).toBe(true); - expect(isESM("import * as dogs from 'dogs';")).toBe(true); - expect(isESM("import { maisey } from 'dogs';")).toBe(true); - expect(isESM("import { charlie as goofball } from 'dogs';")).toBe(true); - expect(isESM("import { default as maisey } from 'dogs';")).toBe(true); - expect(isESM("import { charlie, masiey } from 'dogs';")).toBe(true); - expect(isESM("import { masiey, charlie as pickle } from 'dogs';")).toBe(true); - expect(isESM("import charlie, { maisey } from 'dogs';")).toBe(true); - expect(isESM("import maisey, * as dogs from 'dogs';")).toBe(true); - expect(isESM("import 'dogs';")).toBe(true); - }); -}); - -// Based on https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Statements/export -describe('export syntax', function () { - it('recognizes exported declarations', () => { - expect(isESM('export var maisey, charlie;')).toBe(true); - expect(isESM('export let charlie, maisey;')).toBe(true); - expect(isESM("export var maisey = 'silly', charlie = 'goofy';")).toBe(true); - expect(isESM("export let charlie = 'goofy', maisey = 'silly';")).toBe(true); - expect(isESM("export const maisey = 'silly', charlie = 'goofy';")).toBe(true); - expect(isESM('export function doDogStuff() { /* ... */ }')).toBe(true); - expect(isESM('export class Dog { /* ... */ }')).toBe(true); - expect(isESM('export function* generateWayTooManyPhotosOnMyPhone() { /* ... */ }')).toBe(true); - expect(isESM('export const { maisey, charlie } = dogObject;')).toBe(true); - expect(isESM('export const { charlie, masiey: maiseyTheDog } = dogObject;')).toBe(true); - expect(isESM('export const [ maisey, charlie ] = dogArray;')).toBe(true); - }); - - it('recognizes lists of exports', () => { - expect(isESM('export { maisey, charlie };')).toBe(true); - expect(isESM('export { charlie as charlieMcCharlerson, masiey as theMaiseyMaiseyDog };')).toBe(true); - expect(isESM('export { charlie as default };')).toBe(true); - }); - - it('recognizes default exports', () => { - expect(isESM("export default 'dogs are great';")).toBe(true); - expect(isESM('export default function doDogStuff() { /* ... */ }')).toBe(true); - expect(isESM('export default class Dog { /* ... */ }')).toBe(true); - expect(isESM('export default function* generateWayTooManyPhotosOnMyPhone() { /* ... */ }')).toBe(true); - expect(isESM('export default function () { /* ... */ }')).toBe(true); - expect(isESM('export default class { /* ... */ }')).toBe(true); - expect(isESM('export default function* () { /* ... */ }')).toBe(true); - }); - - it('recognizes exports directly from another module', () => { - expect(isESM("export * from 'dogs';")).toBe(true); - expect(isESM("export * as dogs from 'dogs';")).toBe(true); - expect(isESM("export { maisey, charlie } from 'dogs';")).toBe(true); - expect( - isESM("export { maisey as goodGirl, charlie as omgWouldYouJustPeeAlreadyIWantToGoToBed } from 'dogs';"), - ).toBe(true); - expect(isESM("export { default } from 'dogs';")).toBe(true); - expect(isESM("export { default, maisey } from 'dogs';")).toBe(true); - }); -}); - -describe('potential false positives', () => { - it("doesn't get fooled by look-alikes", () => { - expect(isESM("'this is an import statement'")).toBe(false); - expect(isESM("'this is an export statement'")).toBe(false); - expect(isESM('import(dogs)')).toBe(false); - }); -}); diff --git a/packages/nextjs/test/utils/tunnelRoute.test.ts b/packages/nextjs/test/utils/tunnelRoute.test.ts index 8a46f8b174ea..ae9b085da7ca 100644 --- a/packages/nextjs/test/utils/tunnelRoute.test.ts +++ b/packages/nextjs/test/utils/tunnelRoute.test.ts @@ -1,5 +1,6 @@ -import { NextjsOptions } from '../../src/utils/nextjsOptions'; -import { applyTunnelRouteOption } from '../../src/utils/tunnelRoute'; +import { BrowserOptions } from '@sentry/react'; + +import { applyTunnelRouteOption } from '../../src/client/tunnelRoute'; const globalWithInjectedValues = global as typeof global & { __sentryRewritesTunnelPath__?: string; @@ -14,7 +15,7 @@ describe('applyTunnelRouteOption()', () => { globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route'; const options: any = { dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333', - } as NextjsOptions; + } as BrowserOptions; applyTunnelRouteOption(options); @@ -25,7 +26,7 @@ describe('applyTunnelRouteOption()', () => { globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route'; const options: any = { // no dsn - } as NextjsOptions; + } as BrowserOptions; applyTunnelRouteOption(options); @@ -35,7 +36,7 @@ describe('applyTunnelRouteOption()', () => { it("should not apply `tunnelRoute` option when `tunnelRoute` option wasn't injected", () => { const options: any = { dsn: 'https://11111111111111111111111111111111@o2222222.ingest.sentry.io/3333333', - } as NextjsOptions; + } as BrowserOptions; applyTunnelRouteOption(options); @@ -46,7 +47,7 @@ describe('applyTunnelRouteOption()', () => { globalWithInjectedValues.__sentryRewritesTunnelPath__ = '/my-error-monitoring-route'; const options: any = { dsn: 'https://11111111111111111111111111111111@example.com/3333333', - } as NextjsOptions; + } as BrowserOptions; applyTunnelRouteOption(options); diff --git a/packages/nextjs/test/utils/userIntegrations.test.ts b/packages/nextjs/test/utils/userIntegrations.test.ts index 7a8693a8a524..7a5d8d879fa1 100644 --- a/packages/nextjs/test/utils/userIntegrations.test.ts +++ b/packages/nextjs/test/utils/userIntegrations.test.ts @@ -1,5 +1,5 @@ -import type { IntegrationWithExclusionOption as Integration } from '../../src/utils/userIntegrations'; -import { addOrUpdateIntegration, UserIntegrations } from '../../src/utils/userIntegrations'; +import type { IntegrationWithExclusionOption as Integration } from '../../src/common/userIntegrations'; +import { addOrUpdateIntegration, UserIntegrations } from '../../src/common/userIntegrations'; type MockIntegrationOptions = { name: string; From 7337595bfa163b5dc7caeb2cb1ebe0fecd71202d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 14:25:58 +0000 Subject: [PATCH 15/21] Rename some tests --- packages/nextjs/test/{index.client.test.ts => clientSdk.test.ts} | 0 packages/nextjs/test/{index.server.test.ts => serverSdk.test.ts} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename packages/nextjs/test/{index.client.test.ts => clientSdk.test.ts} (100%) rename packages/nextjs/test/{index.server.test.ts => serverSdk.test.ts} (100%) diff --git a/packages/nextjs/test/index.client.test.ts b/packages/nextjs/test/clientSdk.test.ts similarity index 100% rename from packages/nextjs/test/index.client.test.ts rename to packages/nextjs/test/clientSdk.test.ts diff --git a/packages/nextjs/test/index.server.test.ts b/packages/nextjs/test/serverSdk.test.ts similarity index 100% rename from packages/nextjs/test/index.server.test.ts rename to packages/nextjs/test/serverSdk.test.ts From 837fe928a9f0be3ce35ce8d33c75023ed0c4adf7 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 14:26:52 +0000 Subject: [PATCH 16/21] Undo removing of dynamic webpack import --- packages/nextjs/rollup.npm.config.js | 2 +- packages/nextjs/src/config/withSentryConfig.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index f7716e5956c1..2329f9eafed1 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -5,7 +5,7 @@ export default [ 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.ts', 'src/client/index.ts'], + entrypoints: ['src/index.ts', 'src/client/index.ts', 'src/config/webpack.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) diff --git a/packages/nextjs/src/config/withSentryConfig.ts b/packages/nextjs/src/config/withSentryConfig.ts index c3e15a6e66e8..5c16b82286c7 100644 --- a/packages/nextjs/src/config/withSentryConfig.ts +++ b/packages/nextjs/src/config/withSentryConfig.ts @@ -7,7 +7,6 @@ import type { NextConfigObjectWithSentry, SentryWebpackPluginOptions, } from './types'; -import { constructWebpackConfigFunction } from './webpack'; /** * Add Sentry options to the config to be exported from the user's `next.config.js` file. @@ -53,6 +52,8 @@ function getFinalConfigObject( // we exclude `webpack.ts` and all of its dependencies from nextjs's `@vercel/nft` filetracing. We therefore need to // make sure that we only require it at build time or in development mode. if (phase === PHASE_PRODUCTION_BUILD || phase === PHASE_DEVELOPMENT_SERVER) { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { constructWebpackConfigFunction } = require('./webpack'); return { ...userNextConfigObject, webpack: constructWebpackConfigFunction(userNextConfigObject, userSentryWebpackPluginOptions, userSentryOptions), From 8f837a4639100f0064e4e314c855e10ef1276325 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 14:47:33 +0000 Subject: [PATCH 17/21] Remove circ dep --- packages/nextjs/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 4faa96f252f0..ac30ea1eb5fe 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -31,7 +31,6 @@ "tslib": "^1.9.3" }, "devDependencies": { - "@sentry/nextjs": "7.29.0", "@types/webpack": "^4.41.31", "eslint-plugin-react": "^7.31.11", "next": "10.1.3" From f8e6d72e67eef6634ecbed8ee4cda11a88d79421 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 14:47:38 +0000 Subject: [PATCH 18/21] Fix size-limit path --- .size-limit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.size-limit.js b/.size-limit.js index 20cc71fb75f0..aee03e5a242c 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -46,7 +46,7 @@ module.exports = [ }, { name: '@sentry/nextjs Client - Webpack (gzipped + minified)', - path: 'packages/nextjs/build/esm/index.client.js', + path: 'packages/nextjs/build/esm/client/index.js', import: '{ init }', gzip: true, limit: '57 KB', From ce5df3080b37e0298daadc81ad0ae0560ad61b0f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 15:08:06 +0000 Subject: [PATCH 19/21] Add comments --- packages/nextjs/src/index.types.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index 70912babcd63..5df30dffd580 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -1,5 +1,7 @@ /* eslint-disable import/export */ +// We export everything from both the client part of the SDK and from the server part. Some of the exports collide, +// which is not allowed, unless we redifine the colliding exports in this file - which we do below. export * from './config'; export * from './client'; export * from './server'; @@ -11,13 +13,18 @@ import * as clientSdk from './client'; import type { NodeOptions } from './server'; import * as serverSdk from './server'; +/** Initializes Sentry Next.js SDK */ export declare function init(options: Options | BrowserOptions | NodeOptions): void; +// We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere. export const Integrations = { ...clientSdk.Integrations, ...serverSdk.Integrations }; export declare const defaultIntegrations: Integration[]; export declare const defaultStackParser: StackParser; +// This variable is not a runtime variable but just a type to tell typescript that the methods below can either come +// from the client SDK or from the server SDK. TypeScript is smart enough to understand that these resolve to the same +// methods from `@sentry/core`. declare const runtime: 'client' | 'server'; export const close = runtime === 'client' ? clientSdk.close : serverSdk.close; From c594c2fcbf01e358b3ff3a84e71e5c3007119fb1 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 15:20:14 +0000 Subject: [PATCH 20/21] Ignore linter --- packages/nextjs/src/config/templates/apiWrapperTemplate.ts | 1 + packages/nextjs/src/config/templates/pageWrapperTemplate.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts index 65fe3b240adc..2f8dd2184301 100644 --- a/packages/nextjs/src/config/templates/apiWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/apiWrapperTemplate.ts @@ -9,6 +9,7 @@ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved import * as origModule from '__SENTRY_WRAPPING_TARGET__'; +// eslint-disable-next-line import/no-extraneous-dependencies import * as Sentry from '@sentry/nextjs'; import type { PageConfig } from 'next'; diff --git a/packages/nextjs/src/config/templates/pageWrapperTemplate.ts b/packages/nextjs/src/config/templates/pageWrapperTemplate.ts index 116c0503fbef..e3b6b4e7e296 100644 --- a/packages/nextjs/src/config/templates/pageWrapperTemplate.ts +++ b/packages/nextjs/src/config/templates/pageWrapperTemplate.ts @@ -9,6 +9,7 @@ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved import * as wrapee from '__SENTRY_WRAPPING_TARGET__'; +// eslint-disable-next-line import/no-extraneous-dependencies import * as Sentry from '@sentry/nextjs'; import type { GetServerSideProps, GetStaticProps, NextPage as NextPageComponent } from 'next'; From 490ce16a8512e4d76db13bb33ba3033407394eca Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Jan 2023 16:24:00 +0000 Subject: [PATCH 21/21] Add back isBuild deprecation message --- packages/nextjs/src/server/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nextjs/src/server/index.ts b/packages/nextjs/src/server/index.ts index 0f1c50ce0e6b..2374b33ea519 100644 --- a/packages/nextjs/src/server/index.ts +++ b/packages/nextjs/src/server/index.ts @@ -140,7 +140,7 @@ function addServerIntegrations(options: NodeOptions): void { // TODO (v8): Remove this /** - * @deprecated + * @deprecated Use the constant `IS_BUILD` instead. */ const deprecatedIsBuild = (): boolean => isBuild(); // eslint-disable-next-line deprecation/deprecation