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

Truncated output of --help with Node v6.0.0 #1055

Closed
mgol opened this issue Apr 24, 2016 · 11 comments
Closed

Truncated output of --help with Node v6.0.0 #1055

mgol opened this issue Apr 24, 2016 · 11 comments

Comments

@mgol
Copy link
Contributor

mgol commented Apr 24, 2016

uglify-js --help output is truncated in Node v6.0.0-rc.3. I'd guess the cause is similar to nodejs/node#6297 (reported also as alanshaw/david#106) i.e. process.exit() doesn't guarantee flushing stdout & stderr.

Output:

$ npm -g i uglify-js
$ uglify-js --version
uglify-js 2.6.2
$ node --version
v6.0.0-rc.3
$ uglify-js --help
uglifyjs input1.js [input2.js ...] [options]
Use a single dash to read input from the standard input.

NOTE: by default there is no mangling/compression.
Without [options] it will simply parse input files and dump the AST
with whitespace and comments discarded.  To achieve compression and
mangling you need to use `-c` and `-m`.

Options:
  --source-map                  Specify an output file where to generate source
                                map.                                    [string]
  --source-map-root             The path to the original source to be included
                                in the source map.                      [string]
  --source-map-url              The path to the source map to be added in //#
                                sourceMappingURL.  Defaults to the value passed
                                with --source-map.                      [string]
  --source-map-include-sources  Pass this flag if you want to include the
    %
@kzc
Copy link
Contributor

kzc commented Apr 24, 2016

The node issue outlines various potential workarounds, both in user applications and Node itself: nodejs/node#6297. None of which are great. Waiting for direction from the NodeJS project.

@kzc
Copy link
Contributor

kzc commented Apr 24, 2016

One workaround involves setting process.exitCode instead of calling process.exit() but it does not work on node v0.10.41 (a version I had on hand) or jxcore. process.exitCode is supported on node 0.12.x and later versions.

The npm module "exit" does work on node 0.10.x however. The fix would be to require("exit") in bin/uglifyjs and replace all occurrences of process.exit with exit. I'll make a pull request.

@kzc
Copy link
Contributor

kzc commented Apr 24, 2016

Fix in PR #1056

Edit: PR withdrawn.

@mgol mgol changed the title Truncated output of --help with Node v6.0.0-rc.3 Truncated output of --help with Node v6.0.0 Apr 26, 2016
@mgol
Copy link
Contributor Author

mgol commented Apr 26, 2016

Node 6.0.0 final has been released.

@kzc
Copy link
Contributor

kzc commented Apr 26, 2016

@mishoo In light of this Node 6.0.0 release and the truncated output of --help on Mac, it's probably worth cutting a new Uglify release with PR #1056 included. I tested it on Mac and Linux and it works on node versions 0.10.x through to 6.0.0.

I also have a CR-line-ending parsing fix in the PR queue pending review that is pretty low risk.

@mishoo
Copy link
Owner

mishoo commented Apr 27, 2016

If there's a solution that works after Node 0.12 and doesn't involve a dependency, I'd prefer to go with that. I don't think too many people use older Node…

@kzc
Copy link
Contributor

kzc commented Apr 27, 2016

If you look at the code for the exit module it's a couple of dozen lines. You could copy it right into the uglify source tree if you like.

As for using process.exitCode which works with node 0.12.x and later, it would involve a changing the logic in bin/uglifyjs.

I think the exit module is preferable as it involves the smallest code change and continues to work on node 0.10.x.

@mgol
Copy link
Contributor Author

mgol commented Apr 27, 2016

Unfortunately, Node.js 0.10 is still downloaded a lot, its download count is half of that of Node 5 even though it's Node 5 that's been available directly to download from the main https://nodejs.org/ site.

@kzc
Copy link
Contributor

kzc commented Apr 27, 2016

bin/uglifyjs already relies on the external modules yargs and async. I don't see an issue with adding a third dependency for the small self-contained module exit.

@kzc
Copy link
Contributor

kzc commented Apr 30, 2016

exit module not suitable to replace process.exit(): nodejs/node#6410 (comment)

@kzc
Copy link
Contributor

kzc commented May 1, 2016

A better fix: #1061

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

No branches or pull requests

3 participants