SAFE-Stack / SAFE-template

dotnet CLI template for SAFE project
MIT License
280 stars 87 forks source link

Use `npm ci` rather than `npm install` in Build.fs? #618

Open mattgallagher92 opened 2 weeks ago

mattgallagher92 commented 2 weeks ago

npm ci would restore dependencies specified in package-lock.json (if consistent with package.json), but not alter it. See https://docs.npmjs.com/cli/v10/commands/npm-ci. It might be faster too.

If developers want to alter what's in the lockfile, it feels like they should be deliberately running an npm install, rather than it being a side effect of build targets.

Any reason not to do this?

mattgallagher92 commented 2 weeks ago

It seems that it's much slower! Is there something else we can do to get this working?

jwthomson commented 1 week ago

I am quite confident that npm ci is the preferred command for build pipelines.

It seems that it's much slower!

Where have you observed this?

I think it is better to be slow (hopefully this can be rectified) and safe than fast and potentially "dangerous".

For now I have approved this in #619 and it has been merged but we can always undo this if the slowdown is very significant and canot be resolved.

mattgallagher92 commented 1 week ago

Prash tested it and said that it was of the order of 10 seconds (may even have been 20) to run npm ci as compared to less than 1 second for npm install

Larocceau commented 1 week ago

Sorry for merging this! I did not realize @jwthomson was not aware of the discussion we had on this last week

jwthomson commented 1 week ago

I've run each a few times locally and it is slower, at least on my machine. It was a lot slower the first time. It would be useful to test this in a real dev scenario where npm ci is being run fairly often but not lots of times in immediate succession, to get a more accurate feel for how this would work in practice.

npm ci npm install
Cold, first run 14.9s 1.2s
Successive runs ~4s ~1.2s
jwthomson commented 2 days ago

Following some discussion around caching, I have re-run these tests making sure that they are both using fresh, empty cache directories.

I'm measuring using the following:

Measure-Command { npm install --cache C:\tmp\install-cache }
Measure-Command { npm ci --cache C:\tmp\ci-cache }

I think it is important to look at the relative times rather than the raw times. Overall I don't there's a significant difference vs last week's tests.

npm ci npm install
Cold, first run (empty cache) 7.1s 0.8s
Successive runs, without clearing cache (mean of 3) 2.6s 0.8s