-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(nextjs): Use compiler hook for uploading turbopack sourcemaps #17352
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
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com>
Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com>
if (isTurbopackSupported && isTurbopack && !userSentryOptions.sourcemaps?.disable) { | ||
// Only set if not already configured by user | ||
if (incomingUserNextConfigObject.productionBrowserSourceMaps === undefined) { | ||
// eslint-disable-next-line no-console | ||
console.log('[@sentry/nextjs] Automatically enabling browser source map generation for turbopack build.'); | ||
incomingUserNextConfigObject.productionBrowserSourceMaps = true; | ||
} | ||
|
||
// Enable source map deletion if not explicitly disabled | ||
if (userSentryOptions.sourcemaps?.deleteSourcemapsAfterUpload === undefined) { | ||
// eslint-disable-next-line no-console | ||
console.warn( | ||
'[@sentry/nextjs] Source maps will be automatically deleted after being uploaded to Sentry. If you want to keep the source maps, set the `sourcemaps.deleteSourcemapsAfterUpload` option to false in `withSentryConfig()`. If you do not want to generate and upload sourcemaps at all, set the `sourcemaps.disable` option to true.', | ||
); | ||
userSentryOptions.sourcemaps = { | ||
...userSentryOptions.sourcemaps, | ||
deleteSourcemapsAfterUpload: 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.
two questions:
- is there a way to set
productionBrowserSourceMaps = 'hidden'
(or something similar) rather thantrue
- So far, our strategy was to delete source maps only if we enabled them. Can we do this here as well? Or would this diverge from current behaviour in NextJS?
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.
The only way to enable/disable client-side sourcemaps for turbopack is productionBrowserSourceMaps (boolean)
The only way to disable all sourcemaps for turbopack is nextConfig.experimental.turbopackSourceMaps: false
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.
Updated the logic to only delete the sourcemaps automatically if we enabled them ✅
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.
LGTM from my side
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.
Thanks for explaining and making the deletion change!
Makes use of the runAfterProductionCompile hook to handle sourcemap upload logic.
When activated by setting
_experimental.useRunAfterProductionCompileHook true
inwithSentryConfig
we apply the following logic:Webpack: we disable sourcemap uploads and release creation in the webpack plugin and instead handle these in `runAfterProductionCompile. We do keep the debugId injection in the plugin though.We'll do that in a follow up, webpack stays as is in this PR.Additionally we handle automatic enabling of source maps for turbopack in this PR.
productionBrowserSourceMaps: true
in the Next options. We do that if not explicitly disabled.nextConfig.experimental.turbopackSourceMaps: false
can theoretically be set by the user to disable all sourcemaps, but we don't really want to do anything different in this case.