ref(gatsby): Move SDK initialization logic to SDK #4040
Conversation
Instead of placing the logic in the initialization script of the SDK, move it to the SDK itself.
size-limit report
|
| */ | ||
| export function init(options: GatsbyOptions): void { | ||
| new MetadataBuilder(options, ['gatsby']).addSdkMetadata(); | ||
| const integrations = getIntegrationsFromOptions(options); |
There was a problem hiding this comment.
nit: This function can just be in the same file for now, it's just added overhead to keep it in a separate file
There was a problem hiding this comment.
If you mean gatsby-browser, no. If we want to provide users the option to initialize the SDK from a place without gatsby-browser, this code must live in the SDK itself. Of course, this isn't required at the moment, but without that goal in mind, the whole PR isn't required at all. Moving it here is a refactoring, included it in this PR (and not in the feature PR that will follow-up) to make it easier to review and identify bugs.
There was a problem hiding this comment.
No meaning getIntegrationsFromOptions from packages/gatsby/src/utils/integrations.ts can just be a part of packages/gatsby/src/sdk.ts. Agree with the change from gatsby-browser
| * @param options The options users have defined. | ||
| */ | ||
| export function getIntegrationsFromOptions(options: GatsbyOptions): Integration[] { | ||
| const integrations = [...(options.integrations || [])]; |
There was a problem hiding this comment.
integrations can be a function as well
There was a problem hiding this comment.
Functions are non-serializable. The end goal is to provide a way to define these options. This refactoring is the previous step to that feature, which only supports non-serializable options. So no, at the moment they can't be functions.
| import { GatsbyOptions } from './utils/types'; | ||
|
|
||
| export const defaultOptions = { | ||
| autoSessionTracking: true, |
There was a problem hiding this comment.
This should be on by default now (since v6), we can rm -rf this
There was a problem hiding this comment.
I want the behavior to be as close as possible to the state prior to this refactoring. I'll make another PR to remove these options.
|
|
||
| export const defaultOptions = { | ||
| autoSessionTracking: true, | ||
| environment: process.env.NODE_ENV || 'development', |
There was a problem hiding this comment.
Tbh, should just be undefined by default
There was a problem hiding this comment.
Same answer as previous.
| "@testing-library/react": "^10.4.9", | ||
| "@types/node": "^16.10.3", |
There was a problem hiding this comment.
Linter was complaining about process, used in defaultOptions. Since they'll be removed, I can remove this dev dep too.
| */ | ||
| export function init(options: GatsbyOptions): void { | ||
| new MetadataBuilder(options, ['gatsby']).addSdkMetadata(); | ||
| const integrations = getIntegrationsFromOptions(options); |
There was a problem hiding this comment.
No meaning getIntegrationsFromOptions from packages/gatsby/src/utils/integrations.ts can just be a part of packages/gatsby/src/sdk.ts. Agree with the change from gatsby-browser
Follow-up to #4033.
This PR refactors how the Gatsby plugin initializes the SDK. The initialization was done in the
gatsby-browserscript, and the Gatsby SDK was initializing the React SDK. This approach forces to use that script to initialize it, without providing any alternative.The refactor provides another way to initialize the SDK - calling
Sentry.init, as you do with the other SDKs.gatsby-browserwill call thisSentry.initand initialize the Gatsby SDK, which initializes the React SDK under the hood; just as it was happening previously. For now, even ifSentry.initis exposed, the only way to do the initialization is throughgatsby-browser.Why this refactor?
The options defined in the user's
gatsby-configare serialized. This means that only serializable options can be defined, excluding important SDK features, such asbeforeSend. After this refactoring, additional features to support SDK initialization with non-serializable options will come.