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): Filter RequestAsyncStorage locations by locations that webpack will resolve #9114

Conversation

lforst
Copy link
Member

@lforst lforst commented Sep 26, 2023

Fixes #9092

In our previous attempt of fixing we used require.resolves' paths option which we wanted to limit the locations with to check for the availability of Next.js RequestAsyncStorage module.

Unfortunately, require.resolve will always check "global" node_modules for when resolving, which means that (old) global installations of Next.js would have still been picked up.

With this PR we additionally check for whether the resolved path would be within the locations webpack would check for resolving the module. This way we shouldn't pick up any global installations of Next.js.

@lforst lforst requested review from mydea and AbhiPrasad September 26, 2023 09:00
@github-actions
Copy link
Contributor

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 75.58 KB (+0.01% 🔺)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 31.46 KB (0%)
@sentry/browser - Webpack (gzipped) 22.06 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 70.28 KB (+0.02% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 28.57 KB (+0.02% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped) 20.65 KB (+0.01% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 222.21 KB (+0.03% 🔺)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 86.57 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 61.42 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 31.43 KB (+0.01% 🔺)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 75.61 KB (+0.01% 🔺)
@sentry/react - Webpack (gzipped) 22.09 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 93.48 KB (+0.01% 🔺)
@sentry/nextjs Client - Webpack (gzipped) 51.04 KB (0%)

@lforst lforst changed the title some clarifications fix(nextjs): Filter RequestAsyncStorage locations by locations that webpack will resolve Sep 26, 2023
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

this is a bit painful but if it works, let's :shipit:

@lforst
Copy link
Member Author

lforst commented Sep 26, 2023

this is a bit painful

@mydea Story of my life

@lforst lforst merged commit a57b66d into develop Sep 26, 2023
49 checks passed
@lforst lforst deleted the lforst-filter-request-async-storage-location-by-webpack-resolvable-locations branch September 26, 2023 11:21
billyvg pushed a commit that referenced this pull request Sep 26, 2023
c298lee pushed a commit that referenced this pull request Sep 27, 2023
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.

Sentry doesn't work on Next 13.5
2 participants