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

ref(nextjs): Use loader to set RewriteFrames helper value #5445

Merged
merged 6 commits into from
Jul 25, 2022

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Jul 21, 2022

Ref: #5505

In order to work, in the nextjs SDK our server-side RewriteFrames integration needs to know the name of the directory into which the app is built. While that information is readily available at build time, it isn't at runtime, so we adjust the webpack config to inject a global variable with the correct value in before the the injected Sentry.init() call from sentry.server.config.js wherever we inject that `Sentry.init().

Currently, this is done by creating a temporary file with the relevant code and adding it to the relevant pages' entry value along with sentry.server.config.js. This works, but has its downsides, and is a fair amount of machinery just to add a single line of code.

This injects the same line of code using a loader, which is really just a transformation function for the code from matching files. In this case, we're modifying sentry.server.config.js itself, so that by the time it's added to various pages' entry points, it already contains the relevant code. Since we don't know what the value will be ahead of time, there's a template, which the loader then populates before injecting the new code.

But how is that any less machinery?, you might ask. All of that, still for just one line of code? Honestly, it's not. But in the quest to improve parameterization of transactions names, we're going to be adding a loader in any case. Adding the RewriteFrames value thus provides the perfect proof of concept, precisely because it's so simple, letting us get the loader working separate from all of the other changes that work will entail.

Notes:

  • I moved setting the default value for distDir from retrieval-of-the-value time to storage-of-the-value time, because using a loader necessarily stringifies everything, meaning undefined was turning into the string 'undefined', preventing the default from getting picked up.
  • There are a few scattered lines of unrelated changes, artifacts of the fact that I ended up unpacking a few values at the top of the new webpack function.

Fixes #4684

@lobsterkatie lobsterkatie force-pushed the kmclb-nextjs-add-prefix-loader branch from f6a6180 to 92fcf90 Compare July 22, 2022 01:05
type LoaderOptions = {
distDir: string;
};
// TODO Use real webpack types
Copy link
Member

Choose a reason for hiding this comment

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

Is this gonna be done in a follow-up PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably? Maybe? It's low priority, and slightly complicated by the need to be compatible with both webpack 4 and webpack 5. Not a blocker at the moment.

@lobsterkatie lobsterkatie merged commit 57c964a into master Jul 25, 2022
@lobsterkatie lobsterkatie deleted the kmclb-nextjs-add-prefix-loader branch July 25, 2022 21:27
lobsterkatie added a commit that referenced this pull request Jul 28, 2022
…5479)

In #5445, a change was made to the way the global `RewriteFrames` helper value is handled. Specifically, setting the default value (using the `||` operator) was moved to the place where the value is set rather than where it's retrieved. But if something goes wrong and for whatever reason the value never gets set globally, it now causes errors when the value is later used, because it has nothing to default to.

This fixes that by restoring the default value to the old location, so that when it's used, it will never be undefined.

Fixes #5478.
@smeubank smeubank mentioned this pull request Aug 1, 2022
43 tasks
@vladanpaunovic vladanpaunovic linked an issue Aug 12, 2022 that may be closed by this pull request
43 tasks
@vladanpaunovic vladanpaunovic removed a link to an issue Aug 12, 2022
43 tasks
@vladanpaunovic
Copy link
Contributor

ref: #5505

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 creates rewriteFramesHelper.js during Nextjs "standalone" static generation
3 participants