-
Notifications
You must be signed in to change notification settings - Fork 924
Fix browser detection #1773
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix browser detection #1773
Conversation
BTW, should we merge these |
Web Workers are browser too.
33b08f9
to
4789457
Compare
@@ -68,13 +67,13 @@ export function isNode(): boolean { | |||
* Detect Browser Environment | |||
*/ | |||
export function isBrowser(): boolean { | |||
return typeof window !== 'undefined'; | |||
return typeof self === 'object' && self.self === self; |
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.
Just curious why checking self.self? Does some none browser env have self, but not self.self, or just to handle the case where someone defined a variable named self in none browse env?
The intention of this function was just to check if it's a browser window env.
My intuition is that webworker probably needs a dedicated env check logic, but we don't need it anywhere. Do you see any implication of treating webworker as a browser env?
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 make sure that self is actually the global self, and not some random global that a developer defined.
I thought that in the place you use this function, you'd want the result to be true for web workers as well. It's a useful check in both window and web worker contexts.
I was thinking something along that line too. We could just export the result of the function. Then you can do something like import { Environment, ENVIRONMENT } from `@firebase/util`
if (Environment === ENVIRONMENT.Browser) .... We can also extend the function to return an object if there is other useful information. I think it's a good idea. |
Web Workers are browser too.