-
-
Notifications
You must be signed in to change notification settings - Fork 799
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
[legacy-framework] Change db/index.ts
in new app template to use globalThis
instead of global
#813
[legacy-framework] Change db/index.ts
in new app template to use globalThis
instead of global
#813
Conversation
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.
Thanks again, @kripod! Could you have a look at the lint errors?
Sure! It turns out that only ESLint >=7 supports {
"globals": {
"globalThis": false // Read-only
}
} |
Hey thanks for the PR @kripod!! I just did some digging and I opened an issue for upgrade eslint. You're welcome to take that or let someone else. Once that issue is finished, then we can merge this. However, for this PR you also need to change |
Thanks for getting back to me! Unfortunately, I’m not quite sure whether I’ll have time to do all of this. |
@kripod No worries, I’ve finished up the rest for you. All that remains is for blitz-js/legacy-framework#661 to be closed, and then this can be merged. |
@merelinguist Wonderful, thank you! 🙌 |
Hey @kripod! We finally upgrade to eslint v7! So now we have one final issue it seems. Apparently we also have to add |
Wonderful, thank you! Unfortunately, I’m currently busy with a company projects, so I may not have time to complete the task… |
Ok no problem! |
This facilitates the use of `globalThis`.
445a99f
to
d376c1a
Compare
Not sure what the implications are (if any) of enabling |
Perfect, thanks @kripod and @wKovacs64!! @all-contributors add @kripod for code |
I've put up a pull request to add @kripod! 🎉 |
db/index.ts
in new app template to use globalThis
instead of global
db/index.ts
in new app template to use globalThis
instead of global
db/index.ts
in new app template to use globalThis
instead of global
What are the changes and their implications?
While Node.js has a non-standard
global
namespace object, version 12.4 introduced support for the standardglobalThis
object. Blitz requires Node.js >=12, so the change could be made without losing backwards compatibility.Checklist