Loading node-semver via browserify throws errors in closure compiler verbose mode

[matthew-andrews]

Firstly I recognise this is a slightly obscure use case and probably doesn't affect many (any?) other people at all. Also I have a 'good enough' workaround (instead of require('semver') simply use require('semver/semver')).

What we're doing:

• Loading node-semver via browserify.
• Running our browserified javascript through closure compiler with warning_level VERBOSE.

The errors we see:

semver.browser.js:4: ERROR - variable module is undeclared
if (typeof module === 'object' && module.exports === exports)
           ^

semver.browser.js:912: ERROR - variable define is undeclared
if (typeof define === 'function' && define.amd)
           ^

semver.browser.js:916: ERROR - variable exports is undeclared
  typeof exports === 'object' ? exports :
         ^

semver.browser.js:918: ERROR - variable semver is undeclared
  semver = {}
  ^

I'm not sure what the best fix for this would be but since it's possible to workaround it by just pointing browserify at the raw semver file (rather the semver.browser.js, which has the head.js and foot.js files sandwiching it) I was wondering why the browser property (which - and I may be wrong here - is only used by browserify?) wasn't just pointing to the semver.js file.

Alternatively the code to compile the library 'for browsers' could be switched for browserify with the standalone option, which I know to be 'closure compiler safe' - but that will increase the size of the library unnecessarily...

What do you think?

Thanks!

Matt

[isaacs]

Why is clojure compiler complaining about these things?

Using the typeof operator on a undefined reference is safe. Setting a variable without var defines it as a global, which is also valid JavaScript.

This is a bug in clojure.

[isaacs]

Also, if you're using browserify, then it's wrong -- exports and module WILL be defined when this script is run.

[matthew-andrews]

Thanks for your quick reply @isaacs.

I'm sorry I took a shortcut and the output above was inaccurate:

Also, if you're using browserify, then it's wrong -- exports and module WILL be defined when this script is run.

You're right - those two will be defined - but semver is not.

The error thrown during closure compiling the browserified script was actually just ERROR - variable semver is undeclared semver = {}.

The previous output was got using Google Closure Compiler on the whole browser bundle (which in retrospect was not a fair test). Apologies for the confusion.

[isaacs]

Well... look at the code. It's only going to assign to the semver global if module and exports are undefined. And if they are undefined, then it's not "leaking" a global, it's intentionally setting a very specific global.

So, this is still a case of clojure being either overly zealous or misconfigured.

[isaacs]

Sorry, I misspoke, it'll set semver = {} if both exports and define are unset. It sniffs module.exports to determine whether it's in a Node-like system or simply a CommonJS-like system.

[matthew-andrews]

Ok I think this is just a case of different opinions of coding style. Thanks for your time in going through this with me and explaining it in detail. I have a workaround - and I will discuss with my colleagues about whether we should reduce the enthusiasm of our current Google Closure Compiler settings. Thank you again.

[isaacs]

Oh, I just realized, if it's getting tripped up by the code style of putting semver = {} on a new line, I'd be happy to accept a patch to shuffle things around so that clojure will be happy. But it should be posted as a bug upstream.

[matthew-andrews]

I will investigate this and get back to you.

Thanks for your advice.

[tomByrer]

@matthew-andrews Did you ever find a work-around please?