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.

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): Correctly apply auto-instrumentation to pages in src folder #5984

Merged
merged 2 commits into from Oct 18, 2022

Conversation

lforst
Copy link
Member

@lforst lforst commented Oct 18, 2022

Fixes #5978

Next.js allows users to have their pages directory inside a src directory. So far, the regex that tests if we wanna apply our auto instrumentation loader to a file, didn't consider that if a pages directory exists we don't want to wrap anything inside src/pages, because these files don't count as "pages" anymore.

This PR introduces a check if there is a root-level pages directory - if so, we don't wrap anything inside src/pages.

https://nextjs.org/docs/advanced-features/src-directory

test: new RegExp(`${escapeStringForRegex(projectDir)}(/src)?/pages/.*\\.(${pageExtensionRegex})`),
test: new RegExp(
`${escapeStringForRegex(projectDir)}${
shouldIncludeSrcDirectory ? '(/src)?' : ''
Copy link
Member

Choose a reason for hiding this comment

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

l: Do we need the ? on (/src)? now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, don't need it anymore. Thanks! 652e137

@lforst lforst merged commit 80f586e into master Oct 18, 2022
@lforst lforst deleted the lforst-nextjs-src-folder branch October 18, 2022 11:26
Comment on lines +3 to +4
This page exists to test the compatibility of our auto-wrapper with the option of having the `pages` directory
inside a `src` directory (https://nextjs.org/docs/advanced-features/src-directory)
Copy link
Member

Choose a reason for hiding this comment

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

So the idea is that this page should not get wrapped, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collecting page data ...TypeError: Cannot read property 'getInitialProps' of undefined
3 participants