feat(nextjs): Sourcemaps for custom build directories #4003
Conversation
size-limit report
|
| * @param distDir The distribution directory. | ||
| */ | ||
| function setProjectBasepath(filePath: string, varPrefix: string, distDir: string): void { | ||
| const fileContents = fs.readFileSync(filePath).toString(); |
There was a problem hiding this comment.
has to be sync? we can't async await?
There was a problem hiding this comment.
That was my initial approach and afaik we can definitely do it, but since we're running the code before webpack runs, the number of files is small (2 currently, and this is not something that scales), and files themselves are small, I believe the sync approach is faster (even if we block the event loop). That said, if you think the async approach is more convenient, I'm happy to change it. What do you think?
There was a problem hiding this comment.
I prefer defaulting to async, but I'll leave it up to you to make the final call.
| * This variable gets overridden at build time to set the correct path -- what users | ||
| * defined in the `distDir` option in their Next.js configuration. | ||
| */ | ||
| export const PROJECT_BASEPATH = '.next'; |
There was a problem hiding this comment.
I wonder if this should just go in it's own file - and then we can even snapshot test (https://jestjs.io/docs/snapshot-testing) against potential regressions.
There was a problem hiding this comment.
IMO, yes. There's more code related to integrations that should be moved to another file, like the RewriteFrames integration itself and the addServerIntegrations below, and also the new code in the webpack file. However, that refactor isn't part of this PR and should go in another one.
There was a problem hiding this comment.
I'm comfortable to ship, but @lobsterkatie should prob make the final call
|
Closing in favor of #4017 |
This PR makes the Next.js SDK rewrite at build time the
RewriteFramesintegration's base path RegExp to thedistDiroption the user has defined in the Next.js configuration.Why this approach?
The file names of the source maps must be formatted to display correct stack traces on server errors, defining the file names in the
RewriteFramesintegration. One use case is defining a custom distribution directory (thedistDiroption), and that's the option explored below.There are two times to define the path: run time and build time. In any case, Next.js requires the user to set
distDirin the options to compile the project. To get this path at run time and the SDK build it, users must also define the option in the SDK's server's configuration. However, to get the path at build time, users only need to set it once since thewithSentryConfigwrapper can read the user's Next.js configuration. The SDK generates theRewriteFramesintegration at initialization, meaning it's impossible to dynamically set the distribution directory path at build time if the option is only available at run time. To solve this issue, the SDK at build time rewrites the file where the default path of the integration is, so it's successfully generated at run time without additional configuration.Since the second option improves the user experience, it's the approach taken in this PR.