protobufjs / protobuf.js

Protocol Buffers for JavaScript & TypeScript.
Other
9.91k stars 1.41k forks source link

global is not defined - minimal.js assumes node environment? #874

Open ghost opened 7 years ago

ghost commented 7 years ago

protobuf.js version: 6.8.0

We run this:

$ pbjs -t static-module -w es6 -o proto.js entity.proto

(Same thing with -w commongjs, by the way)

Then import it like so in a JSX:

import { Entity } from 'proto.js'

Using babel, webpack emits the client code, using this babel configuration:

{
    "presets":[
        "es2015", "react"
    ]
}

But when running in the browser, we get:

Uncaught ReferenceError: global is not defined
    at Object.<anonymous> (minimal.js:49)
    ...

Which is that line in minimal.js:

util.isNode = Boolean(global.process && global.process.versions && global.process.versions.node);

Now that makes sense, since we are in browser environment and not node.

I just wonder if we're doing something wrong, or should minimal.js not assume node environment?

dcodeIO commented 7 years ago

The sources expect a node.js environment, while the distribution files wrap the sources through browserify and uglifyjs, where the latter defines an outer iife including the global var (see).

If you are building your own distribution files form the sources using something else than browserify, respective configuration might be required. In this case, though, there should probably be an additional check added testing for the existence of global for compatibility with other bundlers than browserify (see also).

util.isNode = Boolean(typeof global !== "undefined" && global && global.process && global.process.versions && global.process.versions.node);
ghost commented 7 years ago

typeof global !== "undefined" is exactly what needed.

To overcome this issue, right now we add this to index.html:

  <script>
      global = {}
  </script>
ghost commented 7 years ago

Right! This is a bigger scandal than Trump's RussiaGate, but it turns out our Webpack config had

target: 'node',    

rather than

target: 'web',

(a copy-paste mistake)

The latter just works out of the box and there's no need for the global = {} hack above.

So it works with Webpack.

Not sure whether this ticket should be closed or not. Adding typeof global !== "undefined" serves right an isNode predicate.

But with nothing breaking, why fix? Also, not having typeof global !== "undefined" could hint that some outer configuration is wrong.

I'm going to leave this for you guys to decide.

Close at will.