Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix early error #145

Closed
kentcdodds opened this issue Jun 15, 2017 · 4 comments
Closed

Fix early error #145

kentcdodds opened this issue Jun 15, 2017 · 4 comments

Comments

@kentcdodds
Copy link
Collaborator

We used to have a nice error message when you entered a script name that didn't exist. We seem to have broken that and now we see:

~/Developer/glamorous (pr/with-theme)
馃惪  $ nps blah
TypeError: Cannot read property 'scriptName' of undefined
    at runPackageScript (/Users/kdodds/Developer/glamorous/node_modules/nps/dist/index.js:95:35)
    at /Users/kdodds/Developer/glamorous/node_modules/nps/dist/index.js:77:14
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:169:7)
    at Function.Module.runMain (module.js:607:11)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:575:3

This seems to be coming from this. We should probably just add || {} to the end of that line. We should also have a unit test for this.

Should be pretty straightforward. Any takers?

@imbhargav5
Copy link
Contributor

imbhargav5 commented Jun 15, 2017

I faced this issue when I was trying to commit the first time as well. I wanted to add myself as a contributor and then make my first PR, but that din't happen. So had to do it without adding myself. I was debugging it, thought maybe I was doing something wrong. I will have a look at this one.

Does the "lint-staged" task for contributors work now ? I seem to be getting this error all the time.

Edit Never mind. The error on my end was because I was using "fish" shell and it doesn't support && in commands.

I will take up this issue nevertheless.

@kentcdodds
Copy link
Collaborator Author

Oh thanks! I didn't realize that fish doesn't support &&... Hmmm... Maybe that's something we can handle in the nps-utils series util... 馃

@imbhargav5
Copy link
Contributor

Yes! I was surprised that it doesn't. And the issue is in "will-not-implement" milestone. 馃槄 So perhaps we should find a way to not use "&&" and use "series" as you mentioned. Here you go.
Issue in fish shell

@kentcdodds
Copy link
Collaborator Author

Yeah, if there's a way for us to detect that you're using fish (I'll bet there's an npm package that'll tell you the current shell) then we could use that in series to determine whether to use && or something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants