-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
fix(nextjs): Support automatic instrumentation for app directory with custom page extensions #12858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ziyadkhalil thanks for the PR! Could you update an e2e test to validate this? https://github.com/getsentry/sentry-javascript/tree/develop/dev-packages/e2e-tests
… middleware with page extensions (getsentry#12858)
317caa7
to
295e85b
Compare
@AbhiPrasad Sure! I'm not sure which test to update tho as to test this change we have to set the Thank you for the quick feedback! |
Good question. I'll let the other sdk maintainers chime in here for this as I have no strong opinions |
295e85b
to
432ecb0
Compare
… middleware with page extensions (getsentry#12858)
432ecb0
to
bb20466
Compare
@AbhiPrasad Great! I created another PR #12864 with the suggested solution and it was a great idea that made me notice that page extensions feature is not working for server components as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
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, |
There was a problem hiding this comment.
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.
I am gonna merge this so that this is off your plate @ziyadkhalil. I will add tests as a follow-up. |
This PR updates the regex and logic for detecting middleware, route handler, and server component files to support custom page extensions, such as
middleware.custom.ts
androute.custom.ts
.Changes Made
isRouteHandler
function to use a regex that matches custom page extensions for route handler files.isMiddlewareResource
function to use a regex that matches custom page extensions for middleware files.isServerComponent
function to use a regex that matches custom page extensions for server component files.hasGlobalErrorFile
flag to account for custom page extensionsTesting
Checklist
isRouteHandler
function.isMiddlewareResource
function.isServerComponent
function.hasGlobalErrorFile
flag.Please review and provide feedback. Thank you!