Breaks if no package.json is present #6
Conversation
| opts = Object.assign({projectName: require(pkgPath).name}, opts); | ||
| } | ||
|
|
||
| if (!opts || !opts.projectName) { | ||
| throw new Error('Project name could not be inferred. Please specify the `projectName` option.'); | ||
| } |
There was a problem hiding this comment.
I think I would rather do:
const pkgPath = pkgUp.sync(parentDir);
opts = Object.assign({
// if the package.json was not found, avoid breaking with `require(null)`.
projectName: pkgPath && require(pkgPath).name
}, opts);
if (!opts.projectName) {
throw new Error('Project name could not be inferred. Please specify the `projectName` option.');
}There was a problem hiding this comment.
Tons of ways to handle this null value. I will update it to match yours, nice and clean.
There was a problem hiding this comment.
Yup, totally subjective. Both work equally fine :)
|
Looks good. Just curious, why is there no package.json available? |
|
Can you fix the merge conflict? Sorry, I had a local fix that I pushed by accident now. |
|
And no worries about the merge conflict. I'll fix that once I get home (done at the office for today). |
|
I just noticed the test you added still has |
|
Ugh, not again. Fixed in master. Thanks for noticing. |
|
No problem, happens to the best of us! |
# Conflicts: # index.js # test.js
If there is no `package.json` available then the path is `null` and you can't stop it from breaking. Even if you manually provide the `projectName` in options it will still break. This adds a few simple checks to prevent it from blowing up when you do provide the name. I've also added an appropriate test, which properly fails when run without the changes to `index.js` and correctly passes once added. Closes #6
|
Thank you :) |
|
My pleasure! Quick (potentially silly) question: This does not say "merged", but instead closed with a commit that "I committed with you". Is that the result of Github squashing the commits? Or something you did to keep things clean? Next time should I have squashed my commits first? |
|
GitHub now has a button to squash and merge a PR so no need in doing that yourself, although you can off course :). Not sure if Sindre uses the button though. Before there was a button to squash, he did everything "manually". |
|
Hey @SamVerschueren, I've seen that but have yet to use it. I'm guessing he did things manually, hence the "closed" status as opposed to Github's "merged" status. Thanks for the insight! |
|
Oh, and @sindresorhus, may I recommend a bump to Thank you! |
Yeah. I did it manually as I wanted to tweak something on merge. I could probably have used squash and then force pushed, but old habits die hard.
Done :) |
|
:] |
If there is no
package.jsonavailable then the path isnulland you can't stop it from breaking.Even if you manually provide the
projectNamein options it will still break. This adds a few simple checks to prevent it from blowing up when you do provide the name.I've also added an appropriate test, which properly fails when run without the changes to
index.jsand correctly passes once added.