ygoe / msgpack.js

A minimal yet complete MessagePack implementation for JavaScript. msgpack.org[JavaScript]
MIT License
151 stars 34 forks source link

Detect Web Workers environment #12

Open grz0zrg opened 4 years ago

grz0zrg commented 4 years ago

When the library is used in a web worker there is an issue with undefined window so i suggest adding Web Workers detection such as :

// Environment detection
if (typeof module === "object" && module && typeof module.exports === "object") {
    // Node.js
    module.exports = msgpack;
}
else if (typeof WorkerGlobalScope !== 'undefined' && self instanceof WorkerGlobalScope) {
    // Global object
    self[self.msgpackJsName || "msgpack"] = msgpack;
} else {
    // Global object
    window[window.msgpackJsName || "msgpack"] = msgpack;
}
ygoe commented 4 years ago

Unfortunately I have no idea about web workers. This answer suggests that your suggested code change won't work very well. And I couldn't find out what that self is. I'll leave this open until somebody provides a reliable alternative.

ShenTengTu commented 4 years ago

In the script of Web Workers, the self property returns WorkerGlobalScope object (the Worker global scope).

WorkerGlobalScope - Web APIs | MDN

I reference the solution from the other library:

  // Environment detection
  if (
    typeof module === "object" && module && typeof module.exports === "object"
  ) {
    // Node.js
    module.exports = msgpack;
  } else {
    // Global object
    var g;
    if (typeof window !== "undefined") {
      g = window;
    } else if (typeof global !== "undefined") {
      g = global;
    } else if (typeof self !== "undefined") {
      g = self;
    } else {
      g = this;
    }
    g[g.msgpackJsName || "msgpack"] = msgpack;
  }
darcyparker commented 3 years ago

https://caniuse.com/?search=globalThis https://mathiasbynens.be/notes/globalthis has discussion on polyfills

Based on the above, pass the globalThis into your IIFE like this:

(function (_global) { //bind globalThis to _global
...
    // Environment detection
    if (typeof module === "object" && module && typeof module.exports === "object") {
        // Node.js
        module.exports = msgpack;
    }
    else if (_global) {
        // Global object
        _global[_global.msgpackJsName || "msgpack"] = msgpack;
    }
)(globalThis || this || self || window);

Notes:

There won't be any perfect solution to polyfill globalThis... but this is decent I think.

Another option is to not define global.mspack. and instead just support commonJS and expect browsers to use an applicable module loader that supports commonjs.