-
Notifications
You must be signed in to change notification settings - Fork 132
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: global-scope reference error in nodejs #474
Conversation
Hi there @kevinpagtakhan! 👋 This PR should resolve the issue we were experiencing with using |
@mattlubner Thanks for working on this! We appreciate your contributions and thoughts on this matter. I intentionally created an abstraction for the global object through the global-scope module, and would prefer to keep it. We also want to keep the solution lightweight and the SDK to support IE. That said, what are your thoughts on updating GlobalScope to: const GlobalScope = typeof window !== 'undefined' ? window : typeof self !== 'undefined' ? self : undefined; I believe that should still cover the environments we want: browser, web worker, and node specifically for server side rendering. |
Hey @kevinpagtakhan, no problem, I'm always happy to contribute. 🙂 I think your approach is reasonable. Although I do wonder if assigning What do you think of a hybrid approach like this? /* global globalThis */
const GlobalScope = (() => {
if (typeof globalThis !== 'undefined') {
return globalThis;
}
if (typeof window !== 'undefined') {
return window;
}
if (typeof self !== 'undefined') {
return self;
}
if (typeof global !== 'undefined') {
return global;
}
})();
export default GlobalScope; It would be nice if there were a single library for both node and browsers, but I digress… |
@mattlubner Awesome, I like your suggestion above. I feel good about the changes but I'll do some quick testing from my end, the ones I used for the web worker PR. Once that is green, I'll get this merged. Thanks again! |
## [8.14.1](v8.14.0...v8.14.1) (2021-12-23) ### Bug Fixes * global-scope reference error in nodejs ([#474](#474)) ([bdce39d](bdce39d))
🎉 This PR is included in version 8.14.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Addresses #473 by augmenting
GlobalScope
with a preference to usingglobalThis
, when available.Checklist