[ci] cache downloaded npm dependencies

[jerome-benoit]

No description provided.

[jerome-benoit]

Seems the action needs package-lock.json to make the caching works. If there's a good reason for not including it in the repo, that trivial PR is useless :D

[lpinca]

It seems this requires package-lock.json but we disabled it.

ws/.npmrc

Line 1 in 511aefe

package-lock=false

[lpinca]

The reasons are

1. It is not needed. There are only optional peer and dev dependencies. I prefer to always install the latest available versions in the specified range without constantly updating the lock file.
2. rm -rf node_modules && rm -rf package-lock.json && npm i generates changes in the lock file that must be committed.

[jerome-benoit]

It seems this requires package-lock.json but we disabled it.

511aefe/.npmrc#L1C7-L1C7

Given the strategy matrix, caching will speed up the CI.
Reintroducing package-lock.json will need to choose a minimal package-lock.json format version to support and add a step in CI installing the minimum npm version for old node version not supporting.
It's pretty easy to do if using up2date npm has no runtime side-effect on old node version. That change will have no effect on the shipped package (no dependencies), only developers will need to use up2date npm version.
Up to the maintainer, it's not really an important improvement. Re-adding package-lock.json gives a more reproducible 'dev env'.

[lpinca]

Yes I like this PR but I don't like the diff noise generated by the lock file.

[lpinca]

I wonder if the pnpm value still depends on lock file. Can you try to use it?

[jerome-benoit]

Yes I like this PR but I don't like the diff noise generated by the lock file.

The diff noise is almost reduced to zero since npm version 8.

I do not think I can use the cache action directly to cache for example node_modules if there's no way to create a unique hash representing its content (package-lock.json is usually used). I need to look at it, that will allow caching without re-adding package-lock.json.

Totally unrelated to the PR, I've created some examples for multithreading ws server: https://github.com/poolifier/poolifier/tree/master/examples/typescript/websocket-server-pool
But I'm having hard time at finding a WS load testing tool as simple as autocannon for HTTP to assess the connection scheduling because I think RR might cause issue with WS and I would like to assess it before implementing least-connection algo in node.js cluster module. Do you know a simple WS load testing tool as good as autocannon?

[jerome-benoit]

I wonder if the pnpm value still depends on lock file. Can you try to use it?

That's even a better solution as long as long devs are OK with the switch. pnpm has a lock file with versioning but if you state that pnpm version X is required for development, the changes in ci.yml are easier and the pnpm lock file is not subject to the 'diff noise' mentioned. I'm using pnpm on several repos and I'm very happy with it.

[lpinca]

I was thinking about only using it in CI but it seems it is not preinstalled. Anyway the caching behavior is documented here https://github.com/actions/setup-node#caching-global-packages-data.

[lpinca]

Do you know a simple WS load testing tool as good as autocannon?

I'm using https://github.com/uNetworking/uWebSockets/blob/master/benchmarks/load_test.c, but I'm not sure if it is ok for your needs.

[jerome-benoit]

I was thinking about only using it in CI but it seems it is not preinstalled. Anyway the caching behavior is documented here actions/setup-node#caching-global-packages-data.

I see, the issue is the same: the pnpm lock file needs to be in the repo.
I think using https://github.com/actions/cache is possible if a cache key identifying a unique directory content can be built. I can dig it later that week, or next week.

[lpinca]

Ok, thank you.

[lpinca]

The last commit is based on this discussion actions/setup-node#782. It is also used in other repositories, see https://github.com/search?q=cache-dependency-path%3A+.%2Fpackage.json&type=code

There is also this https://github.com/bahmutov/npm-install#use-lock-file but I don't like to use another action if it works as is.

[jerome-benoit]

The last commit is based on this discussion actions/setup-node#782. It is also used in other repositories, see github.com/search?q=cache-dependency-path%3A+.%2Fpackage.json&type=code

There is also this bahmutov/npm-install#use-lock-file but I don't like to use another action if it works as is.

The cache will not always be updated and reused if the dependencies is not updated in package.json. I think that can work that way for a repo without runtime dependencies at validating code changes. The dev deps can sometimes be not up2date in the CI, but I guess it's not a big deal. I would not recommend it on a repo with runtime deps.

[lpinca]

Yes, I understand and agree. It depends on how long the cache is valid.

[lpinca]

It seems 3 different caches are used, one per OS. It makes sense but in our case only one is needed. Anyway I think it is still an improvement.

[lpinca]

Thank you.