Throw a more user-friendly exception #107
3 participants
| @@ -28,6 +29,12 @@ const cli = meow(` | ||
| updateNotifier({pkg: cli.pkg}).notify(); | ||
| +const pkg = readPkgUp.sync().pkg; |
Actually, cli.pkg already has the package data. It uses read-pkg-up as well.
When I inspected the contents of cli.pkg it seemed to contain the package info for np, not the package you were trying to publish. I'll double-check that though :)
Oh yes, you're right. It off course needs the np information for update-notifier. Seems I need some more coffee :).
| @@ -28,6 +29,12 @@ const cli = meow(` | ||
| updateNotifier({pkg: cli.pkg}).notify(); | ||
| +const pkg = readPkgUp.sync().pkg; | ||
| +if (!pkg) { | ||
| + console.error('\nNo package.json found, are you in the correct project?'); |
Why the newline? Just wondering. Maybe add [log-symbols](https://github.com/sindresorhus/log-symbols).error in front of the error message. I'm also not sure about the message itself, can't come up with something better though :).
I'm just going with how other errors or exceptions are printed, ie. https://github.com/sindresorhus/np/blob/master/cli.js#L55
That's because we don't want the error/message being printed directly after the execution list.
Drop the newline.
Message:
${logSymbols.error} No package.json found. Make sure you're in the correct project.
| @@ -38,7 +45,7 @@ Promise | ||
| }); | ||
| } | ||
| - return ui(cli.flags); | ||
| + return ui(cli.flags, pkg); |
I think I'd rather have it added to the options object Object.assign(cli.flags, {pkg}).
But wait before doing that untill @sindresorhus replies. Most of the time, he tends to have a strong opinion on parameters :).
| @@ -47,7 +54,7 @@ Promise | ||
| return options; | ||
| }) | ||
| - .then(options => np(options.version, options)) | ||
| + .then(options => np(options.version, options, pkg)) |
|
I don't really see much benefit in passing the |
|
I've addressed your concerns, PTAL when you have some time |
| @@ -27,6 +28,9 @@ module.exports = (input, opts) => { | ||
| const runTests = !opts.yolo; | ||
| const runCleanup = !opts.skipCleanup && !opts.yolo; | ||
| const pkg = readPkgUp.sync().pkg; | ||
| + if (!pkg) { | ||
| + throw new Error(`${logSymbols.error} No package.json found. Make sure you're in the correct project.`); | ||
| + } |
I think it would be cleaner if we extracted this in something like lib/util.js
const pkg = util.readPkg();@sindresorhus Any reason why we didn't added the error symbol yet in cli.js?
Any reason why we didn't added the error symbol yet in cli.js?
No reason. Can be done.
| @@ -0,0 +1,11 @@ | ||
| +const readPkgUp = require('read-pkg-up'); |
|
I've made the following changes:
Sadly, it still shows the newline when the |
| @@ -0,0 +1,11 @@ | ||
| +const readPkgUp = require('read-pkg-up'); |
|
Done |
1 check passed
|
Very good. Thanks @gillesdemey :) |
|
Thanks @gillesdemey |
Additionally reads the
package.jsonfile once and passes it ontoui.jsandindex.jsFixes #106