-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Preserve global context with clock
methods (setTimeout
/clearTimeout
)
#1704
Conversation
🦋 Changeset detectedLatest commit: 6f7ba59 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
16368b3
to
bfee861
Compare
e7059ff
to
8a8cfa3
Compare
By the way, I just confirmed this fix by using diff --git a/node_modules/xstate/es/interpreter.js b/node_modules/xstate/es/interpreter.js
index a35ec06..8c7fedf 100644
--- a/node_modules/xstate/es/interpreter.js
+++ b/node_modules/xstate/es/interpreter.js
@@ -1237,10 +1237,10 @@ function () {
deferEvents: true,
clock: {
setTimeout: function (fn, ms) {
- return global.setTimeout.call(null, fn, ms);
+ return global.setTimeout.call(global, fn, ms);
},
clearTimeout: function (id) {
- return global.clearTimeout.call(null, id);
+ return global.clearTimeout.call(global, id);
}
},
logger: global.console.log.bind(console),
diff --git a/node_modules/xstate/lib/interpreter.js b/node_modules/xstate/lib/interpreter.js
index 8216933..64213c7 100644
--- a/node_modules/xstate/lib/interpreter.js
+++ b/node_modules/xstate/lib/interpreter.js
@@ -1039,10 +1039,10 @@ var Interpreter = /** @class */ (function () {
deferEvents: true,
clock: {
setTimeout: function (fn, ms) {
- return global.setTimeout.call(null, fn, ms);
+ return global.setTimeout.call(global, fn, ms);
},
clearTimeout: function (id) {
- return global.clearTimeout.call(null, id);
+ return global.clearTimeout.call(global, id);
}
},
logger: global.console.log.bind(console),
Firefox now works and Chrome still works with the change. |
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.
The problem fixed by using global.
etc was probably very similar to what you have experienced - before the fix the this
has been changed because setTimeout
and clearTimeout
were attached to another object and they were used from there (oh, our beautiful JS). See the previous version here: 66c8cf2#diff-5b8880ef88d507d13e4d97365bcfbafe3a9d36a16655ef655f51c1c38ab01bb7L108
We don't do any hoops though with global, call etc if we just invoke them using the global context (like in my suggested changes). This way they will be always looked up on a global object and the invocation context will be correct.
If RxJS is using those timers as is (without hoops) I bet that we can do that as well 😉 :
https://github.com/ReactiveX/rxjs/blob/cfa267bc0916ede09c8b14aedcdb69a791055fb6/src/internal/scheduler/timeoutProvider.ts#L21
I'm happy to do this whichever way is most preferable. @davidkpiano, what do you think? |
Could you try the proposed changes with |
Let's do the changes and try them out. If your use-case still passes, it's probably fine 👍 |
I tested with @Andarist 's patch as well (below), and both work. I'll update now. diff --git a/node_modules/xstate/es/interpreter.js b/node_modules/xstate/es/interpreter.js
index a35ec06..d2e2abb 100644
--- a/node_modules/xstate/es/interpreter.js
+++ b/node_modules/xstate/es/interpreter.js
@@ -1237,10 +1237,10 @@ function () {
deferEvents: true,
clock: {
setTimeout: function (fn, ms) {
- return global.setTimeout.call(null, fn, ms);
+ return setTimeout(fn, ms);
},
clearTimeout: function (id) {
- return global.clearTimeout.call(null, id);
+ return clearTimeout(id);
}
},
logger: global.console.log.bind(console),
diff --git a/node_modules/xstate/lib/interpreter.js b/node_modules/xstate/lib/interpreter.js
index 8216933..5f8be29 100644
--- a/node_modules/xstate/lib/interpreter.js
+++ b/node_modules/xstate/lib/interpreter.js
@@ -1039,10 +1039,10 @@ var Interpreter = /** @class */ (function () {
deferEvents: true,
clock: {
setTimeout: function (fn, ms) {
- return global.setTimeout.call(null, fn, ms);
+ return setTimeout(fn, ms);
},
clearTimeout: function (id) {
- return global.clearTimeout.call(null, id);
+ return clearTimeout(id);
}
},
logger: global.console.log.bind(console),
|
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
clock
methods to global
thisArg
clock
methods (setTimeout
/clearTimeout
)
Fixes #1703