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): Use Next.js internal AsyncStorage #7630

Merged
merged 6 commits into from Mar 29, 2023
Merged

Conversation

lforst
Copy link
Member

@lforst lforst commented Mar 28, 2023

Fixes #7562

Somewhat building on 9ab0772, we cannot use Next.js' headers() function in our server components wrapping because it will trigger an error stating that "dynamic functions" cannot be used in static components.

We fix this by directly using the RequestAsyncStorage that Next.js exports. This will circumvent the error-generating code. Unfortunately, this export is not part of the public API so we had to add some additional handling in case the export should ever disappear.

This change is tested implicitly by our E2E tests that test for connected traces in server components.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 28, 2023

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 20.61 KB (0%)
@sentry/browser - ES5 CDN Bundle (minified) 64.39 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 19.16 KB (-0.01% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 56.77 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 21.62 KB (0%)
@sentry/browser - Webpack (minified) 72.03 KB (0%)
@sentry/react - Webpack (gzipped + minified) 21.64 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 51.99 KB (+0.02% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 28.15 KB (+0.02% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 26.35 KB (+0.03% 🔺)
@sentry/replay ES6 CDN Bundle (gzipped + minified) 44.47 KB (-0.01% 🔽)
@sentry/replay - Webpack (gzipped + minified) 38.54 KB (0%)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 63.13 KB (+0.01% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified) 56.22 KB (-0.01% 🔽)

@lforst lforst marked this pull request as ready for review March 28, 2023 09:46
@lforst lforst requested review from mydea and AbhiPrasad March 28, 2023 09:46
let sentryTraceHeader: string | undefined | null = undefined;
let baggageHeader: string | undefined | null = undefined;

// TODO: Explain this try catch
Copy link
Member

Choose a reason for hiding this comment

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

still a todo?

NEXTJS_REQUEST_ASYNC_STORAGE_MODULE_PATH,
);
} else {
templateCode = templateCode.replace(/__SENTRY_NEXTJS_REQUEST_ASYNC_STORAGE_SHIM__/g, requestAsyncStorageShimPath);
Copy link
Member

Choose a reason for hiding this comment

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

l: Instead of warning at runtime, can we add a warning here instead?

// Needs to end in .cjs in order for the `commonjs` plugin to pick it up
const WRAPPING_TARGET_MODULE_NAME = '__SENTRY_WRAPPING_TARGET_FILE__.cjs';

const NEXTJS_REQUEST_ASYNC_STORAGE_MODULE_PATH = 'next/dist/client/components/request-async-storage';
Copy link
Member

Choose a reason for hiding this comment

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

let's link to GH sha w/ with filename?

@lforst lforst enabled auto-merge (squash) March 29, 2023 06:52
@lforst lforst merged commit 2974ff1 into develop Mar 29, 2023
30 of 31 checks passed
@lforst lforst deleted the lforst-fix-revalidation branch March 29, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants