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

Suggestion - suggest npm ci instead of npm install #37

Open
jackgeek opened this issue Sep 11, 2019 · 9 comments
Open

Suggestion - suggest npm ci instead of npm install #37

jackgeek opened this issue Sep 11, 2019 · 9 comments

Comments

@jackgeek
Copy link

For teams using package.lock.json advising the user to use npm install is not good advice. WOuld it be possible to add configuration to allow it do advise running npm ci instead?

@mgol
Copy link
Owner

mgol commented Sep 11, 2019

As far as I understand, npm ci is meant for CI systems, not for running on a local machine, therefore npm install is still applicable. npm ci is also mainly meant to be run on a state with no node_modules present (if the folder is present, it gets deleted first) so a dependency check before running npm ci doesn't make much sense to me - on CI it should be run always, locally - it shouldn't.

@mgol mgol closed this as completed Sep 11, 2019
@jackgeek
Copy link
Author

@mgol Thanks for your quick response.

This is not quite true. We use npm ci to ensure that the packages we install comply with package.lock.json. And we do this locally. The impact of running npm install is that it will change your package.lock.json and therefore you do not get the benefit of its protection.

We advise all our developers to run npm ci instead of npm install for this reason.

@jackgeek
Copy link
Author

BTW happy to submit a PR for this if you agree with it

@mgol
Copy link
Owner

mgol commented Sep 11, 2019

I'm fine with this being an option, I think npm install should still be the default. Perhaps a new packageInstallCommand being "install" by default would solve the issue? You'd be able to define it to be "ci" for your purposes; it'd also make it easier to support package managers that use a different command than install (if there are any).

PRs are welcome!

@mgol mgol reopened this Sep 11, 2019
@jackgeek
Copy link
Author

@mgol great, please expect a PR forthwith.

May I suggest that the packageInstallCommand default to "npm install" then you could have the option of changing it to "yarn install" or "pnmp install" or one of the many other npm alternatives.

@mgol
Copy link
Owner

mgol commented Sep 11, 2019

@jackgeek There's already the packageManager option that controls which package manager to use. This is needed because some of them use a different folder for their dependencies (e.g. Bower uses bower_components).

BTW, currently only npm & bower are officially supported (see the README). I think adding support for more should be doable, in many places there's logic that checks whether packageManager === 'npm' and if not, assumes it's Bower. If that logic was reversed, more package managers could be supported as most behave like npm. PRs for that are also welcome if you're interested (please remember about tests!).

@jackgeek
Copy link
Author

jackgeek commented Sep 12, 2019

Hi @mgol, This is taking me longer than I anticipated due to eslint and formatting rules. Do you use an auto formatter? Is there one I can use to get the style for your project? If not have you considered using prettier? This is what I use for my projects and it is excellent.

@mgol
Copy link
Owner

mgol commented Sep 12, 2019

Yeah, I’d use Prettier if was setting up the repo now. I’m fine with a PR that introduces it but I think you’d first need a PR to my eslint-config-mgol to remove all formatting rules from the config. I have a check there that all defined rules are either explicitly enabled or disabled so those rules need to be set to disabled.

@mgol
Copy link
Owner

mgol commented Sep 30, 2019

@jackgeek I went ahead & updated eslint-config-mgol & introduced Prettier to check-dependencies. Hopefully the setup is now more to your liking. :)

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

No branches or pull requests

2 participants