-
-
Notifications
You must be signed in to change notification settings - Fork 98
Pr/fix init error #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.
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
Pr/fix init error #127
Conversation
When findUp can't find an existing package-scripts.js file, it returns null, which is passed back to findUp and throws an error because path.resolve can only accept strings as arguments sezna#124
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 10 10
Lines 286 286
=====================================
Hits 286 286
Continue to review full report at Codecov.
|
kentcdodds
left a comment
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.
Just a small fix to the contributors table :)
.all-contributorsrc
Outdated
| "name": "Nate Cavanaugh", | ||
| "avatar_url": "https://avatars1.githubusercontent.com/u/116871?v=3", | ||
| "profile": "http://alterform.com", | ||
| "contributions": [] |
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.
Could you add "code" to this array and then run npm start generateContributors?
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.
Done, thanks for the heads up :)
|
This is great, thank you! |
The contributions array in .all-contributorsrc was missing the "code" value
|
Thank you so much! |
When findUp can't find an existing package-scripts.js file, it returns null, which is passed back to
findUp and throws an error because path.resolve can only accept strings as arguments
#124
What: Fixes error thrown when a package-scripts.js file doesn't exist while running
initWhy: Because if you're running
nps initwithout an existing package-scripts.js file, it won't work, forcing developers to have to create the file manually.How: Checking that the
pathreturned fromgetPSConfigFilepathis actually a string before passing it tofindUp.sync.I tried figuring out why the tests never fail for this, but I believe it's because of jest's mocking of findUp means that it never even hits the
path.resolvemethod, so I'm not sure how you'd want to test for this particular case.