search

es-shims / globalThis

ECMAScript spec-compliant polyfill/shim for `globalThis`.
https://github.com/tc39/proposal-global
MIT License
155 stars 14 forks source link

Implementation question #2

Closed matthewp closed 4 years ago

matthewp commented 8 years ago

Hey, great repo, and thanks a million for championing this proposal. I have written environment checks so many times and am thrilled I'll be able to stop doing it in the future!

With that being said, I think the implementation takes the wrong approach by searching for browser globals first. These globals are easily mocked (and often are) in Node.

Checking for Node first is a better approach, imo, because it can't as easily be faked. I always use:

typeof process === 'object' && {}.toString.call(process) === '[object process]'

This approach can't easily (at all?) be mocked so you should always get the correct global.

Happy to submit a pull request but wanted to run it by you first. Thanks again for your hard work!

ljharb commented 8 years ago

To rephrase, you're suggesting that the first check be a process + global check, rather than checking for global last? That seems reasonable to me. (With a cached Object.prototype.toString, ofc)

matthewp commented 8 years ago

Yes, I'm not sure if a global check is even necessary, but it couldn't hurt. I'll submit a PR, thanks!