-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(nextjs): Add SDK to serverside app
directory
#6927
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
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.
feels... brittle idk. But I guess the next webpack entry code has always been like that.
|
||
newConfig.module.rules.push({ | ||
test: /node_modules[/\\]@sentry[/\\]nextjs/, | ||
use: [ | ||
{ | ||
loader: path.resolve(__dirname, 'loaders', 'sdkMultiplexerLoader.js'), | ||
options: { | ||
importTarget: buildContext.nextRuntime === 'edge' ? './edge' : './client', | ||
importTarget: { browser: './client', node: './server', edge: './edge' }[runtime], |
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.
l: Let's move this object into it's own const variable?
// User-specified pages to skip. (Note: For ease of use, `excludeServerRoutes` is specified in terms of routes, | ||
// which don't have the `pages` prefix.) | ||
const entryPointRoute = entryPointName.replace(/^pages/, ''); | ||
if (stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true)) { |
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.
l: can we just do
if (stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true)) { | |
return !stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true) |
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.
lol yeah - the classic
): void { | ||
const assetPrefix = userNextConfig.assetPrefix || userNextConfig.basePath || ''; | ||
|
||
const isomorphicValues = { | ||
// `rewritesTunnel` set by the user in Next.js config | ||
__sentryRewritesTunnelPath__: userSentryOptions.tunnelRoute, | ||
SENTRY_RELEASE: { id: getSentryRelease(buildContext.buildId) }, |
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.
l: can we move the The webpack plugin's release injection breaks the
app directory
comment down here?
@@ -138,7 +139,7 @@ describe('constructWebpackConfigFunction()', () => { | |||
); | |||
}); | |||
|
|||
it('injects user config file into `_app` in client bundle but not in server bundle', async () => { | |||
it('injects user config file into `_app` in server bundle but not in client bundle', async () => { |
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.
why did this condition flip?
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.
It basically doesn't matter where we inject what. To clean things up a bit I decided we should inject the server SDK in the pages/_app
entrypoint and the client SDK to the main
and app-main
entrypoints.
I assume this test just verified that we do not accidentally mess this up.
Absolutely. It is horrible but out of the 100 things I've tried, this was the only one that worked reliably... |
Ref: #6726
This PR injects the Next.js SDK into serverside
app
directory bundles, allowing users to call the Sentry SDK in server components.