Remove unnecessary this overwrite in stack context manager
#873
Conversation
dyladan
requested review from
bg451,
markwolff,
mayurkale22,
obecny,
OlivierAlbertini,
rochdev and
vmarchaud
as code owners
| @@ -112,7 +112,7 @@ export class StackContextManager implements ContextManager { | |||
| this._currentContext = context || Context.ROOT_CONTEXT; | |||
|
|
|||
| try { | |||
| return fn.apply(context); | |||
| return fn(); | |||
There was a problem hiding this comment.
@obecny would like to get your reviews on this.
There was a problem hiding this comment.
hmm it was a bit tricky from what I remember, but if the all tests are working fine, then I guess it should be fine too ?. Can you check the the examples replacing the zone scope manager with the stack if nothing is missing ?.
dyladan
changed the title
this overwrite in stack context manager
| @@ -112,7 +112,7 @@ export class StackContextManager implements ContextManager { | |||
| this._currentContext = context || Context.ROOT_CONTEXT; | |||
|
|
|||
| try { | |||
| return fn.apply(context); | |||
| return fn(); | |||
There was a problem hiding this comment.
hmm it was a bit tricky from what I remember, but if the all tests are working fine, then I guess it should be fine too ?. Can you check the the examples replacing the zone scope manager with the stack if nothing is missing ?.
|
@obecny the tracer web example is failing for unrelated reasons. The window.addEventListener fails. It fails in the same way with and without the changes. Because the tests are passing and it appears to be working with my local testing, i'm going to merge this. |
Stack scope manager was calling
fn.apply(context, args)which causes thethisinfnto be set tocontextrather than what it would have been normally. It appears not to affect the working of the Context Manager