From bb20466a7b0ce7c555fad6a9ac28186838eff2c0 Mon Sep 17 00:00:00 2001 From: Ziyad Date: Thu, 11 Jul 2024 01:48:29 +0300 Subject: [PATCH 1/2] fix(nextjs): Support automatic instrumentation for app directory with page extensions (#12858) --- .../src/config/loaders/wrappingLoader.ts | 2 +- packages/nextjs/src/config/webpack.ts | 19 +++++++------- packages/nextjs/test/config/loaders.test.ts | 25 +++++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index a0d953d8315b..955268fb304b 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -182,7 +182,7 @@ export default function wrappingLoader( const componentTypeMatch = path.posix .normalize(path.relative(appDir, this.resourcePath)) - .match(/\/?([^/]+)\.(?:js|ts|jsx|tsx)$/); + .match(/\/?([^/.]+)(?:\..*)?\.(?:js|ts|jsx|tsx)$/); if (componentTypeMatch && componentTypeMatch[1]) { let componentType: ServerComponentContext['componentType']; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 4002db18f295..2e1a180e6319 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -147,12 +147,12 @@ export function constructWebpackConfigFunction( ); }; - const possibleMiddlewareLocations = ['js', 'jsx', 'ts', 'tsx'].map(middlewareFileEnding => { - return path.join(middlewareLocationFolder, `middleware.${middlewareFileEnding}`); - }); const isMiddlewareResource = (resourcePath: string): boolean => { const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - return possibleMiddlewareLocations.includes(normalizedAbsoluteResourcePath); + return ( + normalizedAbsoluteResourcePath.startsWith(middlewareLocationFolder) && + !!normalizedAbsoluteResourcePath.match(/[\\/]middleware(\..*)?\.(js|jsx|ts|tsx)$/) + ); }; const isServerComponentResource = (resourcePath: string): boolean => { @@ -163,7 +163,7 @@ export function constructWebpackConfigFunction( return ( appDirPath !== undefined && normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && - !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/) + !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)(?:\..*)?\.(?:js|jsx|tsx)$/) ); }; @@ -172,7 +172,7 @@ export function constructWebpackConfigFunction( return ( appDirPath !== undefined && normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && - !!normalizedAbsoluteResourcePath.match(/[\\/]route\.(js|jsx|ts|tsx)$/) + !!normalizedAbsoluteResourcePath.match(/[\\/]route(?:\..*)?\.(?:js|jsx|ts|tsx)$/) ); }; @@ -285,10 +285,9 @@ export function constructWebpackConfigFunction( } if (appDirPath) { - const hasGlobalErrorFile = ['global-error.js', 'global-error.jsx', 'global-error.ts', 'global-error.tsx'].some( - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - globalErrorFile => fs.existsSync(path.join(appDirPath!, globalErrorFile)), - ); + const hasGlobalErrorFile = fs + .readdirSync(appDirPath) + .some(file => file.match(/^global-error(?:\..*)?\.(?:js|ts|jsx|tsx)$/)); if ( !hasGlobalErrorFile && diff --git a/packages/nextjs/test/config/loaders.test.ts b/packages/nextjs/test/config/loaders.test.ts index c2aaf0c9a707..6ccc701949f1 100644 --- a/packages/nextjs/test/config/loaders.test.ts +++ b/packages/nextjs/test/config/loaders.test.ts @@ -14,6 +14,7 @@ import { import { materializeFinalWebpackConfig } from './testUtils'; const existsSyncSpy = jest.spyOn(fs, 'existsSync'); +jest.spyOn(fs, 'readdirSync').mockReturnValue([]); const lstatSyncSpy = jest.spyOn(fs, 'lstatSync'); type MatcherResult = { pass: boolean; message: () => string }; @@ -96,6 +97,10 @@ describe('webpack loaders', () => { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/testPage.tsx', expectedWrappingTargetKind: 'page', }, + { + resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/testPage.custom.tsx', + expectedWrappingTargetKind: 'page', + }, { resourcePath: './src/pages/testPage.tsx', expectedWrappingTargetKind: 'page', @@ -133,6 +138,10 @@ describe('webpack loaders', () => { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/middleware.js', expectedWrappingTargetKind: 'middleware', }, + { + resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/middleware.custom.js', + expectedWrappingTargetKind: 'middleware', + }, { resourcePath: './src/middleware.js', expectedWrappingTargetKind: 'middleware', @@ -162,10 +171,26 @@ describe('webpack loaders', () => { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/api/nested/testApiRoute.js', expectedWrappingTargetKind: 'api-route', }, + { + resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/api/nested/testApiRoute.custom.js', + expectedWrappingTargetKind: 'api-route', + }, + { + resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/route.ts', + expectedWrappingTargetKind: 'route-handler', + }, + { + resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/route.custom.ts', + expectedWrappingTargetKind: 'route-handler', + }, { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/page.js', expectedWrappingTargetKind: 'server-component', }, + { + resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/page.custom.js', + expectedWrappingTargetKind: 'server-component', + }, { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/page.js', expectedWrappingTargetKind: 'server-component', From 8ad089abebb74f0580503d1a264550c9dcf2ecca Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 11 Jul 2024 08:15:16 +0000 Subject: [PATCH 2/2] fix --- .../src/config/loaders/wrappingLoader.ts | 3 ++- packages/nextjs/src/config/webpack.ts | 27 ++++++++++++------- packages/nextjs/test/config/fixtures.ts | 1 + packages/nextjs/test/config/loaders.test.ts | 7 +++-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 955268fb304b..ea7828497f95 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -182,7 +182,8 @@ export default function wrappingLoader( const componentTypeMatch = path.posix .normalize(path.relative(appDir, this.resourcePath)) - .match(/\/?([^/.]+)(?:\..*)?\.(?:js|ts|jsx|tsx)$/); + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor + .match(new RegExp(`/\\/?([^/]+)\\.(?:${pageExtensionRegex})$`)); if (componentTypeMatch && componentTypeMatch[1]) { let componentType: ServerComponentContext['componentType']; diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 2e1a180e6319..ecc39f7372dd 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -147,12 +147,12 @@ export function constructWebpackConfigFunction( ); }; + const possibleMiddlewareLocations = pageExtensions.map(middlewareFileEnding => { + return path.join(middlewareLocationFolder, `middleware.${middlewareFileEnding}`); + }); const isMiddlewareResource = (resourcePath: string): boolean => { const normalizedAbsoluteResourcePath = normalizeLoaderResourcePath(resourcePath); - return ( - normalizedAbsoluteResourcePath.startsWith(middlewareLocationFolder) && - !!normalizedAbsoluteResourcePath.match(/[\\/]middleware(\..*)?\.(js|jsx|ts|tsx)$/) - ); + return possibleMiddlewareLocations.includes(normalizedAbsoluteResourcePath); }; const isServerComponentResource = (resourcePath: string): boolean => { @@ -163,7 +163,10 @@ export function constructWebpackConfigFunction( return ( appDirPath !== undefined && normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && - !!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)(?:\..*)?\.(?:js|jsx|tsx)$/) + !!normalizedAbsoluteResourcePath.match( + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor + new RegExp(`[\\\\/](page|layout|loading|head|not-found)\\.(${pageExtensionRegex})$`), + ) ); }; @@ -172,7 +175,10 @@ export function constructWebpackConfigFunction( return ( appDirPath !== undefined && normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) && - !!normalizedAbsoluteResourcePath.match(/[\\/]route(?:\..*)?\.(?:js|jsx|ts|tsx)$/) + !!normalizedAbsoluteResourcePath.match( + // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor + new RegExp(`[\\\\/]route\\.(${pageExtensionRegex})$`), + ) ); }; @@ -285,9 +291,12 @@ export function constructWebpackConfigFunction( } if (appDirPath) { - const hasGlobalErrorFile = fs - .readdirSync(appDirPath) - .some(file => file.match(/^global-error(?:\..*)?\.(?:js|ts|jsx|tsx)$/)); + const hasGlobalErrorFile = pageExtensions + .map(extension => `global-error.${extension}`) + .some( + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + globalErrorFile => fs.existsSync(path.join(appDirPath!, globalErrorFile)), + ); if ( !hasGlobalErrorFile && diff --git a/packages/nextjs/test/config/fixtures.ts b/packages/nextjs/test/config/fixtures.ts index 7da47e37be33..a3c4feb0123b 100644 --- a/packages/nextjs/test/config/fixtures.ts +++ b/packages/nextjs/test/config/fixtures.ts @@ -13,6 +13,7 @@ export const EDGE_SDK_CONFIG_FILE = 'sentry.edge.config.js'; /** Mock next config object */ export const userNextConfig: NextConfigObject = { publicRuntimeConfig: { location: 'dogpark', activities: ['fetch', 'chasing', 'digging'] }, + pageExtensions: ['jsx', 'js', 'tsx', 'ts', 'custom.jsx', 'custom.js', 'custom.tsx', 'custom.ts'], webpack: (incomingWebpackConfig: WebpackConfigObject, _options: BuildContext) => ({ ...incomingWebpackConfig, mode: 'universal-sniffing', diff --git a/packages/nextjs/test/config/loaders.test.ts b/packages/nextjs/test/config/loaders.test.ts index 6ccc701949f1..c559ee643baf 100644 --- a/packages/nextjs/test/config/loaders.test.ts +++ b/packages/nextjs/test/config/loaders.test.ts @@ -14,7 +14,6 @@ import { import { materializeFinalWebpackConfig } from './testUtils'; const existsSyncSpy = jest.spyOn(fs, 'existsSync'); -jest.spyOn(fs, 'readdirSync').mockReturnValue([]); const lstatSyncSpy = jest.spyOn(fs, 'lstatSync'); type MatcherResult = { pass: boolean; message: () => string }; @@ -196,8 +195,8 @@ describe('webpack loaders', () => { expectedWrappingTargetKind: 'server-component', }, { - resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/page.ts', // ts is not a valid file ending for pages in the app dir - expectedWrappingTargetKind: undefined, + resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/page.ts', + expectedWrappingTargetKind: 'server-component', }, { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/(group)/nested/page.tsx', @@ -205,7 +204,7 @@ describe('webpack loaders', () => { }, { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/(group)/nested/loading.ts', - expectedWrappingTargetKind: undefined, + expectedWrappingTargetKind: 'server-component', }, { resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/layout.js',