-
Notifications
You must be signed in to change notification settings - Fork 940
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
Check for undefined on browser globals. #462
Conversation
Not all environments include these globals. For example, web workers do not have global window objects.
👍 I mentioned this in the PR, but I think it got merged before it was read. |
Ah yeah I see it there now. Yeah this little issue just bit us in the ass, because all sorts of dependencies use debug, and we're using some of them in web workers, which means they're throwing exceptions now :/. |
@marbemac could you remove the second check for the tests that you've updated? We don't need a truthiness test for |
I think part of the idea in that last commit was that the undefined check does not cover null. Without that second check, window.process will throw an exception if window is null. |
In my experience those values will ever be Doesn't hurt to be defensive I guess, but the point of cae07b7 was to clean up these checks a bit. |
I'm happy to take it out! Shall I? Or leave it? |
Let's take them out please.
…On Wed, May 17, 2017 at 19:05 Marc MacLeod ***@***.***> wrote:
I'm happy to take it out! Shall I? Or leave it?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#462 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEWWPX0LNu3tPxfIGaZsritOMTIdOyPks5r66dVgaJpZM4NemH_>
.
|
All set! |
👍 |
When will this be released? |
2.6.8 published |
Not all environments include these globals. For example, web workers do not have global window objects.
This seems to be a regression from this commit: cae07b7