-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: Expose prio scope functions #2092
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
Conversation
/** | ||
* @inheritDoc | ||
*/ | ||
public setExtras(extras: { [key: string]: any }): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isnt this setExtra? and extra?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, before we only had setExtra
on the scope which allowed only key:value
setExtras
allowed to set an object.
I will change that it accepts both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 i think its fine though i would probably want to override the entire extra per old functionality (for other things folks should probably just use setContext)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, we should even consider not even exposing setExtra
at all anymore.
Could be too early tho, I mean it's still there in configureScope
but not on Sentry.set...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You cannot have multiple input types in Go, so I'd still need to have 2 methods for it
This reverts commit 74f44f9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. The only change that I'd like to make in this PR as well is remove client
guard from configureScope
, as it's not client dependent. Once it's added, feel free to merge.
Don't mean to hijack, but:
When would you not have an active client? I'm guessing maybe if you don't provide a DSN value? (which is the official way to handle enabling & disabling Sentry between environments, such as for local development, iirc). Might be useful to note that in the documentation somewhere, if it's not already (not that I think people should be encouraged to make expensive/complex operations for Sentry data if they can avoid it 😂) (Also, I think you might have forgotten about exposing |
Counterpart to #2091
This exposes a new "global" functions from the Scope to provide a simpler API:
This enables users to do something like this:
The motivation for this change is:
Sentry.setTags({a: 'b'});
setXXX
vs.configureScope
configureScope
works differently in a way that in case there is no active client on the hub, theconfigureScope
callback will not be invoked at all, meaning if there are expensive operations this is better thansetXXX
.