fix: replace with getGlobal #3133
Conversation
🦋 Changeset detectedLatest commit: f47d149 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
CodeSee Review Map:Review in an interactive map View more CodeSee Maps Legend |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f47d149:
|
|
This seems to be missing the |
🚧 the Mini Program development framework don't provide 🚑️ It should be better to replace that line of code with function
|
packages/core/src/interpreter.ts
Outdated
| @@ -142,9 +142,9 @@ export class Interpreter< | |||
| return clearTimeout(id); | |||
| } | |||
| } as Clock, | |||
| logger: global.console.log.bind(console), | |||
| logger: global?.console.log.bind(console), | |||
There was a problem hiding this comment.
Is this change required though? Are all standard~ global objects not available in this environment? Even self and globalThis?
There was a problem hiding this comment.
my fault😢 . this change is not required.
Only self and globalThis is undefined. window is available.
There was a problem hiding this comment.
the return value of getGlobal may be undefined
There was a problem hiding this comment.
window is available.
This is somewhat confusing as on the screenshot shared here it states that window shouldn't be available in this environment 🤔
There was a problem hiding this comment.
replacing global with window or globalThis may be the right way, but why not use getGlobal() directly.
There was a problem hiding this comment.
😅 global don't have the console object
There was a problem hiding this comment.
That is... some bizarre env there 😅 The especially surprising bit is this:
window "[object Object]"
globalThis "[object Window]"
I'm still surprised that window is available here as that contradicts the documentation - but, of course, I believe the screenshot that you have shared just now. Is globalThis.console available there?
There was a problem hiding this comment.
Is
globalThis.consoleavailable there?
Yep.
There was a problem hiding this comment.
I've tweaked the fix and pushed it out to your branch. Could you check if this variant works for you?
There was a problem hiding this comment.
Everything works perfectly. Thank you so much 🎉 🚀
| @@ -142,9 +142,9 @@ export class Interpreter< | |||
| return clearTimeout(id); | |||
| } | |||
| } as Clock, | |||
| logger: global.console.log.bind(console), | |||
There was a problem hiding this comment.
my rationale here is that the argument passed to this bind call wasn't looked up on the global variable anyway. This means that all our users were already accessing this in their respective global scopes automatically and it worked. it had to work as otherwise they wouldn't be able to run XState at all as it would throw Uncaught ReferenceError: console is not defined at the initialization time.
So I think that we don't have to resolve the global object here manually at all and we can just rely on the standard JS scope lookup.
In some runtime environments(the Mini Program unique to China), self or global may be empty.