feat: withScope returns from wrapped function #3753
Conversation
rhcarvalho
requested review from
vladanpaunovic,
HazAT,
bruno-garcia and
mitsuhiko
size-limit report
|
packages/hub/test/hub.test.ts
Outdated
| return 'ok'; | ||
| }; | ||
| const hub = new Hub(); | ||
| const value = hub.withScope(someFn); // wraps someFn to run it in a new scope |
There was a problem hiding this comment.
I wanted to write...
| const value = hub.withScope(someFn); // wraps someFn to run it in a new scope | |
| // wrapped wraps someFn to run it in a new scope | |
| const wrapped = () => hub.withScope(someFn); | |
| const value = wrapped(); |
... because we wanted withScope (and, eventually, Sentry.trace) to be easier to write wrapped versions of existing functions.
However, by delaying evaluation with () => ..., we're also delaying the evaluation of what is the current scope, which is not what one would typically want. Thus, the current form is only good for wrap-and-call existing functions.
rhcarvalho
force-pushed
the
rhcarvalho/withscope
branch
from
52a83e3
to
f9c71ff
Compare
This is part of a larger work towards better scope management. This change makes Sentry.withScope and Hub.withScope easier to use with existing functions that return a non-void type. The motivation is that withScope could be used to wrap existing functions and run them in a new scope without requiring a closure or mutating state outside of the wrapped function (since it had to be void). While this does change the withScope signature, we're considering it relatively low risk of breaking downstream code as we're making it more general.
rhcarvalho
force-pushed
the
rhcarvalho/withscope
branch
from
f9c71ff
to
a498172
Compare
|
@mitsuhiko || @HazAT || @kamilogorek could you please review? |
|
Probably not going to merge this as the current direction to introduce a |
|
Not going to merge this. It became unclear whether this change is necessary, in face of larger conceptual changes required to handle scope management different than what we have today. More details on lessons learned shall go into getsentry/develop#356. |
This is part of a larger work towards better scope management. See #3751.
This change makes
Sentry.withScopeandHub.withScopeeasier to use with existing functions that return a non-void type.The motivation is that withScope could be used to wrap existing functions and run them in a new scope without requiring a closure or mutating state outside of the wrapped function (since it had to be void).
While this does change the withScope signature, we're considering it relatively low risk of breaking downstream code as we're making it more general.