Skip to content

fix(bin): write into streams before exit #127

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 19, 2016
Merged

fix(bin): write into streams before exit #127

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
28ff697
fix(bin): write into streams before exit
mmeineke Sep 27, 2016
acfd6a7
fix(bin): removed process.exit
mmeineke Sep 30, 2016
c3702cc
refactor(bin): moved resolver exit
mmeineke Oct 5, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 64 additions & 65 deletions bin/react-docgen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
*
*/

/*eslint no-process-exit: 0*/

var argv = require('nomnom')
.script('react-docgen')
.help(
Expand Down Expand Up @@ -90,10 +88,11 @@ if (argv.resolver) {
if (e.code !== 'MODULE_NOT_FOUND') {
throw e;
}
exitWithError(
writeError(
`Unknown resolver: "${argv.resolver}" is neither a built-in resolver ` +
`nor can it be found locally ("${resolverPath}")`
);
process.exitCode = 1;
Copy link
Member

@fkling fkling Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we just call process.exit(1) here?

Copy link
Contributor Author

@mmeineke mmeineke Oct 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually yes, but I wanted to get rid of the eslint no-process-exit: 0 line :D
I would say you should only disable a linting rule if there is no other way.

Copy link
Member

@fkling fkling Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that rule is more targeted towards arbitrary modules. From the docs:

This is a dangerous operation because it can occur in any method at any point in time, potentially stopping a Node.js application completely when an error occurs.

So, using it in the executable file itself is completely valid :)

Copy link
Contributor Author

@mmeineke mmeineke Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems legit. But disabling this rule could lead to the same problem in the future.
So I would prefer to have this rule enabled also here. Are the new changes ok for you?

Copy link
Member

@fkling fkling Oct 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can always disable the rule for just that one line ;)

But yeah, I guess this is OK.

}
}
}
Expand All @@ -112,12 +111,7 @@ function writeError(msg, filePath) {
}
}

function exitWithError(error) {
writeError(error);
process.exit(1);
}

function exitWithResult(result) {
function writeResult(result) {
result = argv.pretty ?
JSON.stringify(result, null, 2) :
JSON.stringify(result);
Expand All @@ -126,7 +120,6 @@ function exitWithResult(result) {
} else {
process.stdout.write(result + '\n');
}
process.exit(0);
}

function traverseDir(filePath, result, done) {
Expand All @@ -139,7 +132,7 @@ function traverseDir(filePath, result, done) {
},
function(error, content, filename, next) {
if (error) {
exitWithError(error);
throw error;
}
try {
result[filename] = parse(content);
Expand All @@ -150,72 +143,78 @@ function traverseDir(filePath, result, done) {
},
function(error) {
if (error) {
writeError(error);
throw error;
}
done();
}
);
}

/**
* 1. No files passed, consume input stream
*/
if (paths.length === 0) {
var source = '';
process.stdin.setEncoding('utf8');
process.stdin.resume();
var timer = setTimeout(function() {
process.stderr.write('Still waiting for std input...');
}, 5000);
process.stdin.on('data', function (chunk) {
clearTimeout(timer);
source += chunk;
});
process.stdin.on('end', function () {
try {
exitWithResult(parse(source));
} catch(error) {
writeError(error);
}
});
} else {
if (process.exitCode !== 1) {
/**
* 2. Paths are passed.
* 1. No files passed, consume input stream
*/
var result = Object.create(null);
async.eachSeries(paths, function(filePath, done) {
fs.stat(filePath, function(error, stats) {
if (error) {
writeError(error, filePath);
done();
return;
}
if (stats.isDirectory()) {
traverseDir(filePath, result, done);
if (paths.length === 0) {
var source = '';
process.stdin.setEncoding('utf8');
process.stdin.resume();
var timer = setTimeout(function() {
process.stderr.write('Still waiting for std input...');
}, 5000);
process.stdin.on('data', function (chunk) {
clearTimeout(timer);
source += chunk;
});
process.stdin.on('end', function () {
try {
writeResult(parse(source));
} catch(error) {
writeError(error);
}
else {
try {
result[filePath] = parse(fs.readFileSync(filePath));
} catch(error) {
});
} else {
/**
* 2. Paths are passed.
*/
var result = Object.create(null);
async.eachSeries(paths, function(filePath, done) {
fs.stat(filePath, function(error, stats) {
if (error) {
writeError(error, filePath);
}
finally {
done();
return;
}
if (stats.isDirectory()) {
try {
traverseDir(filePath, result, done);
} catch(error) {
writeError(error);
done();
}
}
else {
try {
result[filePath] = parse(fs.readFileSync(filePath));
} catch(error) {
writeError(error, filePath);
}
finally {
done();
}
}
});
}, function() {
var resultsPaths = Object.keys(result);
if (resultsPaths.length === 0) {
// we must have gotten an error
process.exitCode = 1;
} else if (paths.length === 1) { // a single path?
fs.stat(paths[0], function(error, stats) {
writeResult(stats.isDirectory() ? result : result[resultsPaths[0]]);
});
} else {
writeResult(result);
}
});
}, function() {
var resultsPaths = Object.keys(result);
if (resultsPaths.length === 0) {
// we must have gotten an error
process.exit(1);
}
if (paths.length === 1) { // a single path?
fs.stat(paths[0], function(error, stats) {
exitWithResult(stats.isDirectory() ? result : result[resultsPaths[0]]);
});
} else {
exitWithResult(result);
}
});
}
}