search

mixu / gluejs

Build CommonJS modules for the browser via a chainable API
http://mixu.net/gluejs/
165 stars 13 forks source link

Results don't pass JSHint #22

Open jamesarosen opened 11 years ago

jamesarosen commented 11 years ago

Running JSHint v2.0.1 results in the following violations in the definition of require:

tmp/out.js: line 1, col 44, Expected an assignment or function call and instead saw an expression.
tmp/out.js: line 1, col 248, Missing semicolon.
tmp/out.js: line 1, col 343, Missing semicolon.
tmp/out.js: line 1, col 456, Missing semicolon.
tmp/out.js: line 1, col 663, Expected an assignment or function call and instead saw an expression.
tmp/out.js: line 1, col 664, Missing semicolon.
tmp/out.js: line 1, col 696, Missing semicolon.
tmp/out.js: line 1, col 697, Missing semicolon.

Additionally, if the unused flag is set and any module doesn't require another module (is a leaf node in the dependency tree), the follow violations also occur:

tmp/out.js: line ..., col 48, 'require' is defined but never used.
tmp/out.js: line ..., col 39, 'exports' is defined but never used.

The alternative is to run JSHint on the inputs to gluejs and just assume that gluejs doesn't cause any problems. I like the extra assurance of being able to run JSHint on the file I'm actually delivering to the client, though.

mixu commented 11 years ago

I investigated this a bit more. The issue is that the require shim is minified with uglifyjs, which produces compact output but that output does not comply with the coding standards that jshint wants. I'm putting in a commit later on that provides more control over the require implementation to use; if you want the shim to pass jshint, you can use the unminified version once that option is in.

The latter issue would require gluejs to run code analysis on the files to detect that they are not using require / exports; that's a bit too much work for a fairly low-value improvement.