Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(nextjs): Support automatic instrumentation for app directory with custom page extensions #12858

Merged
merged 2 commits into from
Jul 11, 2024
Merged

fix(nextjs): Support automatic instrumentation for app directory with custom page extensions #12858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb20466
fix(nextjs): Support automatic instrumentation for app directory with…
ziyadkhalil Jul 10, 2024
8ad089a
fix
lforst Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/nextjs/src/config/loaders/wrappingLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down
22 changes: 15 additions & 7 deletions packages/nextjs/src/config/webpack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export function constructWebpackConfigFunction(
);
};

const possibleMiddlewareLocations = ['js', 'jsx', 'ts', 'tsx'].map(middlewareFileEnding => {
const possibleMiddlewareLocations = pageExtensions.map(middlewareFileEnding => {
return path.join(middlewareLocationFolder, `middleware.${middlewareFileEnding}`);
});
const isMiddlewareResource = (resourcePath: string): boolean => {
Expand All @@ -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})$`),
)
);
};

Expand All @@ -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})$`),
)
);
};

Expand Down Expand Up @@ -285,10 +291,12 @@ 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 = 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 &&
Expand Down
1 change: 1 addition & 0 deletions packages/nextjs/test/config/fixtures.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
30 changes: 27 additions & 3 deletions packages/nextjs/test/config/loaders.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,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',
Expand Down Expand Up @@ -133,6 +137,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',
Expand Down Expand Up @@ -162,25 +170,41 @@ 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',
},
{
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,
Comment on lines -174 to -175
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked and this is actually not true. It's just very uncommon because usually you have jsx in your pages.

resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/page.ts',
expectedWrappingTargetKind: 'server-component',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/(group)/nested/page.tsx',
expectedWrappingTargetKind: 'server-component',
},
{
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',
Expand Down
Loading