Skip to content
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

Have global exports be compatible with Web Workers #2053

Merged
merged 1 commit into from Jan 26, 2016

Conversation

mislav
Copy link
Contributor

@mislav mislav commented Jan 15, 2016

Assigning to window only works in a normal browser environment; however, Web Workers don't have access to window. Instead, the global object inside Web Workers is self.

Browserify already ensures that global is present and dereferenced accordingly. It will equal to window unless self is defined.

This fixes Web Workers compatibility lost in a81e555 (v2.3.0).

/cc @jbnicolai @ndhoule

Review on Reviewable

Assigning to `window` only works in a normal browser environment;
however, Web Workers don't have access to `window`. Instead, the global
object inside Web Workers is `self`.

Browserify ensures that `global` is present to all scripts by creating a
shim reference to it using this logic:

    typeof global !== "undefined" ? global :
    typeof self !== "undefined" ? self :
    typeof window !== "undefined" ? window : {}

Thus, if neither `global` nor `self` were originally present,
`global === window` will be true.

Web Workers compatibility was broken in a81e555.
@dasilvacontin
Copy link
Contributor

test/browser/index.html is failing now. I can't really look into it right now.

@mislav
Copy link
Contributor Author

mislav commented Jan 23, 2016

@dasilvacontin If you're talking about this failure, the same error is present on master:

screen shot 2016-01-23 at 4 27 36 pm

It's not clear to me whether this error is expected or not. The browser test suite might have fallen out of date since it doesn't seem exercised under CI.

@danielstjules
Copy link
Contributor

test/browser/index.html was fixed in the latest patch release. Confirmed that this PR fixes web worker compatibility. Thanks! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants