-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Support private packages #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've commented about error message.
const viewPackage = packageName => { | ||
return new Promise((resolve, reject) => { | ||
const view = spawn("npm", ["view", packageName, "versions", "--json"]); | ||
let result = ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add errorResult
like
let errorResult = "";
lib/can-npm-publish.js
Outdated
}); | ||
|
||
view.stderr.on("data", err => { | ||
console.error(err.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errorResult += err.toString()
lib/can-npm-publish.js
Outdated
|
||
view.on("close", code => { | ||
if (code > 0) { | ||
reject(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reject(new Error(errorResult));
We should reject with errorResult
, becausereject
should have Error arguments.
This error message will be output to
Line 39 in e8ea937
console.log(error.message); |
@azu Thanks for your feedback, I updated the PR accordingly |
In case your packages require you to login to npm using their cli,
can-npm-publish
won't see them (instead getting a 404 withnode-fetch
). I changed the check to usenpm view
as suggested in #1