feat(gatsby): Support non-serializable SDK options #4064
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iker-barriocanal
requested review from
lobsterkatie and
vladanpaunovic
and removed request for
a team
size-limit report
|
AbhiPrasad
reviewed
Oct 14, 2021
packages/gatsby/gatsby-browser.js
Outdated
| @@ -1,17 +1,63 @@ | |||
| const Sentry = require('@sentry/gatsby'); | |||
|
|
|||
| // To avoid confusion, you must set the Sentry configuration in one | |||
There was a problem hiding this comment.
I think this behaviour is still too complicated. Let's just make it so that if sentry.config.{js, ts} is defined, that will always take priority over the plugin params (and then we can just log a warning if plugin params are also used).
There was a problem hiding this comment.
We can then also rm -rf all this enabled logic.
There was a problem hiding this comment.
Simplified the logic. We can't get rid of the isSentryInitialized because the gatsby-browser script's scope is in the browser, so we need to still check for the Sentry instance. But, we are no longer playing around with the enabled.
If... - The SDK has been initialized in a config file, use that instance. If options are also defined, log a warning to merge the options. - The SDK has not been initialized, initialize it. Unless options have not been provided, and log an error if that happens.
rhcarvalho
reviewed
Oct 18, 2021
AbhiPrasad
requested changes
Oct 20, 2021
AbhiPrasad
approved these changes
Oct 21, 2021
This was referenced Nov 2, 2021
iker-barriocanal
added a commit
to getsentry/sentry-docs
that referenced
this pull request
Nov 4, 2021
Adds the docs supporting the Gatsby SDK initialization in a config file, see getsentry/sentry-javascript#4064.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current Gatsby SDK requires to set the SDK options in
gatsby-config.js, which doesn't allow to set non-serializable options. This PR introducessentry.config.js, a file where you can define yourSentry.initwith non-serializable options. The current approach of defining the SDK options is still supported. Only one of the two approaches can be used; if a user defines both, theinitinsentry.config.jsis prioritized and other options are ignored.This PR is a follow-up to #4040.
How does this work?
That file is injected into the webpack config's entry properties, which makes the SDK to initialize before
gatsby-browser.jsis run, the only place where the SDK was being initialized before. That script has the following behavior:sentry.config.jsand there are options defined ingatsby-config.js, a warning message telling the user to merge both configs is logged. The SDK is initialized with theinitin the config file.sentry.config.jsand there aren't any options defined ingatsby-config.js, an error message telling the user init options aren't defined is logged, linking to the docs.