Ref(V3): Refactor init & drop deprecated omit. #1046
Conversation
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| }; | ||
| finalOptions.siblingOptions && delete finalOptions.siblingOptions; | ||
|
|
||
| if (finalOptions.enabled === false || NATIVE.platform === 'web') { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
we don't want
dsn,
siblingOptions: {
vueOptions:{
app: app
}
}
to be present, we actually want it to be together with the other options when setting the browser options, like
dsn,
app: app
| * When set to `false`, Sentry will suppress reporting of all props data | ||
| * from your Vue components for privacy concerns. | ||
| */ | ||
| attachProps: boolean; | ||
|
|
||
| /** | ||
| * By default, Sentry attaches an error handler to capture exceptions and report them to Sentry. | ||
| * When `attachErrorHandler` is set to `false`, automatic error reporting is disabled. | ||
| * | ||
| * Usually, this option should stay enabled, unless you want to set up Sentry error reporting yourself. | ||
| * For example, the Sentry Nuxt SDK does not attach an error handler as it's using the error hooks provided by Nuxt. | ||
| * | ||
| * @default true |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
I would like to keep it as is since it's an exact copy of Vue options.
| } | ||
|
|
||
| const browserOptions = { | ||
| ...passedOptions.siblingOptions?.vueOptions, | ||
| ...passedOptions.siblingOptions?.nuxtClientOptions, | ||
| ...finalOptions, | ||
| autoSessionTracking: | ||
| NATIVE.platform === 'web' && finalOptions.enableAutoSessionTracking, | ||
| } as BrowserOptions & T; | ||
| } as BrowserOptions; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
BrowserOptions is a common type, and it's used for Angular/React. It's also the same type for Vue/Nuxt with additional parameters.
It indeed extends what is defined by BrowserOptions but it allows the original options to be preserved for the following SDK init.
antonis
left a comment
antonis
left a comment
There was a problem hiding this comment.
LGTM 🚀
Thank you for your work on this Lucas 🙇
This change introduces a break change thus is going to a major version.
The main change was done here:
to
The previous change was required since a sibling Option may have additional parameters that are required in order to initialize it, adding
Thelped with it, but also brought two problems:Twill have no JSDoc due to the nature of how TypeScirpt deals with it.The PR fixes both issues on the following way:
Tand enforce all options to beCapacitorOptions. This way it's easier to remove items that aren't supposed to be used with Capacitor.siblingOptions: This sadly wasn't a nice change, since it will require additional changes from users during migration, BUT, it also brings JSDocs back to their respective sibling options, so required fields from Vue/Nuxts are again required to be defined, instead of being optional.With that we were able to eliminate the _experimental field, removing all the items that were deprecated and also removed the
autoSessionTrackingthat was found out to be removed anyway from the newer versions of Sentry JavaScript.Screenshots of a SDK init with siblngOptions:
Required field missing that wasn't warned before: