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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not throw on missing package.json #15

Closed

Conversation

ruyadorno
Copy link
Contributor

@ruyadorno ruyadorno commented Oct 29, 2020

This fixes running npm exec in folders that does not have a package.json since runScript throws a ENOENT error.

NOTE: Not sure if throwing a ENOENT error on missing package.json is the intended behavior, if that's the case we can also simply patch its usage in npm exec by providing the empty pkg: {scripts:{}} arg 🤔

This fixes running `npm exec` in folders that does not have a
`package.json` since `runScript` throws a `ENOENT` error.
@ruyadorno ruyadorno requested review from isaacs and nlf October 29, 2020 15:39
Copy link
Contributor

@nlf nlf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it makes sense to me to patch this here, though we should maybe only catch and try again if the error is ENOENT? as is, if you have a package.json but it's invalid we would lose that signal

@ruyadorno
Copy link
Contributor Author

tbh the more I think about it the more I think it makes more sense to patch it in lib/exec.js instead. The empty {scripts: {}} dance is already there, the problem is just that the usage added to support already installed pkgs gets triggered before that dance is set up.

I'll try to float a PR in the cli instead (the more annoying part is the unit tests there I guess) and close this one if that turns out to work well.

@ruyadorno
Copy link
Contributor Author

closing in favor of: npm/cli#2081

@ruyadorno ruyadorno closed this Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants