-
Notifications
You must be signed in to change notification settings - Fork 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
fix: only access Node.js globals if available #3501
Conversation
The latest changes of this PR are available on NPM as Also you can depend on latest version built from this PR: |
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
To better telegraph the intent in the changelog, you could make this 2 PRs, one updating the eslint with these settings, and a following one with the then-allowed addition of globalThis.
Or, you could rebase these changes into 2 commits that do exactly just that, and then the changes could not be squashed.
But in my meager opinion, that would be gravy and I also approve as is. :)
@n1ru4l please rebase : ) |
@saihaj done 🎉 |
Was added as part of graphql#3501
The
process
object is only available in Node.js and Node.js-like environments (e.g. webpack).By introducing this simple change, the code can run in a browser environment without requiring polyfilling Node.js APIs such as the following:
globalThis
is part of the ECMA 2020 specification and is supported on any relevant platform.The eslint config specifies ecma version 2020 but it seems like the
globalThis
is not automatically provided. I added thees2020
env. (eslint/eslint#15199 (comment))Closes #2409