diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 0bd38d0a3dbb..f1e1ffae40fd 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -18,6 +18,7 @@ }, "dependencies": { "@rollup/plugin-sucrase": "4.0.4", + "@rollup/plugin-virtual": "3.0.0", "@sentry/core": "7.16.0", "@sentry/integrations": "7.16.0", "@sentry/node": "7.16.0", diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js index dcda038ba311..04d4675eda63 100644 --- a/packages/nextjs/rollup.npm.config.js +++ b/packages/nextjs/rollup.npm.config.js @@ -33,7 +33,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', /__RESOURCE_PATH__.*/], }, }), ), diff --git a/packages/nextjs/src/config/loaders/proxyLoader.ts b/packages/nextjs/src/config/loaders/proxyLoader.ts index 2c499211800a..e7eb30085f4f 100644 --- a/packages/nextjs/src/config/loaders/proxyLoader.ts +++ b/packages/nextjs/src/config/loaders/proxyLoader.ts @@ -38,7 +38,7 @@ export default async function proxyLoader(this: LoaderThis, userC // 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__')) { + if (this.resourceQuery.includes('__sentry_wrapped__') || this.resourceQuery.includes('__sentry_external__')) { return userCode; } @@ -55,34 +55,29 @@ export default async function proxyLoader(this: LoaderThis, userC // Fill in the path to the file we're wrapping and save the result as a temporary file in the same folder (so that // relative imports and exports are calculated correctly). - // - // TODO: We're saving the filled-in template to disk, however temporarily, because Rollup expects a path to a code - // file, not code itself. There is a rollup plugin which can fake this (`@rollup/plugin-virtual`) but the virtual file - // seems to be inside of a virtual directory (in other words, one level down from where you'd expect it) and that - // messes up relative imports and exports. Presumably there's a way to make it work, though, and if we can, it would - // be cleaner than having to first write and then delete a temporary file each time we run this loader. templateCode = templateCode.replace(/__RESOURCE_PATH__/g, this.resourcePath); - const tempFilePath = path.resolve(path.dirname(this.resourcePath), `temp${Math.random()}.js`); - fs.writeFileSync(tempFilePath, templateCode); // Run the proxy module code through Rollup, in order to split the `export * from ''` out into // individual exports (which nextjs seems to require), then delete the tempoary file. - let proxyCode = await rollupize(tempFilePath, this.resourcePath); - fs.unlinkSync(tempFilePath); + + let proxyCode = await rollupize(this.resourcePath, templateCode); if (!proxyCode) { // We will already have thrown a warning in `rollupize`, so no need to do it again here return userCode; } - // Add a query string onto all references to the wrapped file, so that webpack will consider it different from the - // non-query-stringged version (which we're already in the middle of loading as we speak), and load it separately from - // this. When the second load happens this loader will run again, but we'll be able to see the query string and will - // know to immediately return without processing. This avoids an infinite loop. const resourceFilename = path.basename(this.resourcePath); + + // For some reason when using virtual files (via the @rollup/plugin-virtual), rollup will always resolve imports with + // absolute imports to relative imports with `..`.In our case we need`.`, which is why we're replacing for that here. + // Also, we're adding 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. proxyCode = proxyCode.replace( - new RegExp(`/${escapeStringForRegex(resourceFilename)}'`, 'g'), - `/${resourceFilename}?__sentry_wrapped__'`, + 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 67daf63b0eca..eefa3d5e2307 100644 --- a/packages/nextjs/src/config/loaders/rollup.ts +++ b/packages/nextjs/src/config/loaders/rollup.ts @@ -1,23 +1,22 @@ -import type { RollupSucraseOptions } from '@rollup/plugin-sucrase'; import sucrase from '@rollup/plugin-sucrase'; +import virtual from '@rollup/plugin-virtual'; import { logger } from '@sentry/utils'; import * as path from 'path'; import type { InputOptions as RollupInputOptions, OutputOptions as RollupOutputOptions } from 'rollup'; import { rollup } from 'rollup'; -const getRollupInputOptions: (proxyPath: string, resourcePath: string) => RollupInputOptions = ( - proxyPath, - resourcePath, -) => ({ - input: proxyPath, +const SENTRY_PROXY_MODULE_NAME = 'sentry-proxy-module'; + +const getRollupInputOptions = (userModulePath: string, proxyTemplateCode: string): RollupInputOptions => ({ + input: SENTRY_PROXY_MODULE_NAME, + plugins: [ - // For some reason, even though everything in `RollupSucraseOptions` besides `transforms` is supposed to be - // optional, TS complains that there are a bunch of missing properties (hence the typecast). Similar to - // https://github.com/microsoft/TypeScript/issues/20722, though that's been fixed. (In this case it's an interface - // exporting a `Pick` picking optional properties which is turning them required somehow.)' + virtual({ + [SENTRY_PROXY_MODULE_NAME]: proxyTemplateCode, + }), sucrase({ transforms: ['jsx', 'typescript'], - } as unknown as RollupSucraseOptions), + }), ], // We want to process as few files as possible, so as not to slow down the build any more than we have to. We need the @@ -25,7 +24,7 @@ const getRollupInputOptions: (proxyPath: string, resourcePath: string) => Rollup // otherwise they won't be processed. (We need Rollup to process the former so that we can use the code, and we need // it to process the latter so it knows what exports to re-export from the proxy module.) Past that, we don't care, so // don't bother to process anything else. - external: importPath => importPath !== proxyPath && importPath !== resourcePath, + 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 @@ -66,19 +65,19 @@ const rollupOutputOptions: RollupOutputOptions = { * ''` call into individual exports (which nextjs seems to need). * * @param tempProxyFilePath The path to the temporary file containing the proxy module code - * @param resourcePath The path to the file being wrapped + * @param userModulePath The path to the file being wrapped * @returns The processed proxy module code, or undefined if an error occurs */ -export async function rollupize(tempProxyFilePath: string, resourcePath: string): Promise { +export async function rollupize(userModulePath: string, templateCode: string): Promise { let finalBundle; try { - const intermediateBundle = await rollup(getRollupInputOptions(tempProxyFilePath, resourcePath)); + const intermediateBundle = await rollup(getRollupInputOptions(userModulePath, templateCode)); finalBundle = await intermediateBundle.generate(rollupOutputOptions); } catch (err) { __DEBUG_BUILD__ && logger.warn( - `Could not wrap ${resourcePath}. An error occurred while processing the proxy module template:\n${err}`, + `Could not wrap ${userModulePath}. An error occurred while processing the proxy module template:\n${err}`, ); return undefined; } @@ -92,7 +91,7 @@ export async function rollupize(tempProxyFilePath: string, resourcePath: string) // square brackets into underscores. Further, Rollup adds file extensions to bare-path-type import and export sources. // Because it assumes that everything will have already been processed, it always uses `.js` as the added extension. // We need to restore the original name and extension so that Webpack will be able to find the wrapped file. - const resourceFilename = path.basename(resourcePath); + const resourceFilename = path.basename(userModulePath); const mutatedResourceFilename = resourceFilename // `[\\[\\]]` is the character class containing `[` and `]` .replace(new RegExp('[\\[\\]]', 'g'), '_') diff --git a/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts index 1cd7c40181eb..3429d3c28511 100644 --- a/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/apiProxyLoaderTemplate.ts @@ -4,11 +4,15 @@ * * We use `__RESOURCE_PATH__` as a placeholder for the path to the file being wrapped. Because it's not a real package, * this causes both TS and ESLint to complain, hence the pragma comments below. + * + * The `?__sentry_external__` is used to + * 1) tell rollup to treat the import as external (i.e. not process it) + * 2) tell webpack not to proxy this file again (avoiding an infinite loop) */ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved -import * as origModule from '__RESOURCE_PATH__'; +import * as origModule from '__RESOURCE_PATH__?__sentry_external__'; import * as Sentry from '@sentry/nextjs'; import type { PageConfig } from 'next'; diff --git a/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts b/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts index 9979ae814c49..991f2f7bd6a8 100644 --- a/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts +++ b/packages/nextjs/src/config/templates/pageProxyLoaderTemplate.ts @@ -4,11 +4,15 @@ * * We use `__RESOURCE_PATH__` as a placeholder for the path to the file being wrapped. Because it's not a real package, * this causes both TS and ESLint to complain, hence the pragma comments below. + * + * The `?__sentry_external__` is used to + * 1) tell rollup to treat the import as external (i.e. not process it) + * 2) tell webpack not to proxy this file again (avoiding an infinite loop) */ // @ts-ignore See above // eslint-disable-next-line import/no-unresolved -import * as wrapee from '__RESOURCE_PATH__'; +import * as wrapee from '__RESOURCE_PATH__?__sentry_external__'; import * as Sentry from '@sentry/nextjs'; import type { GetServerSideProps, GetStaticProps, NextPage as NextPageComponent } from 'next'; diff --git a/yarn.lock b/yarn.lock index 56cefe2024c9..2f3d6c5b0b7b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3317,6 +3317,11 @@ "@rollup/pluginutils" "^4.1.1" sucrase "^3.20.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"