enable CI cache for NPM

[cometkim]

No description provided.

[cometkim]

This doesn't make any difference on npm install speed right now. However the GitHub cache is slightly faster than the NPM registry, and the effect accumulates as dependencies grow.

Also we have some unspecified dev dependencies; the Puppeteer. It will be useful if we explicitly specify the version of the Puppeteer we use (we should)

[cknitt]

Certainly can't hurt to turn on the npm caching.
However, IMHO the goal should not be that the dependencies grow (unless there is a very good reason).

Regarding puppeteer, it comes with a lot of transitive dependencies, it is not needed for the CI build and it is usually not needed when working on the compiler either.

It is only needed in the single case that you want to update the reserved keywords list, which is documented here: https://github.com/rescript-lang/rescript-compiler/blob/8100772b31bc4e616a23630dce14b1813ef16a8e/CONTRIBUTING.md?plain=1#L304

So I would prefer not to add it as a dev dependency. Maybe we could instead move the "npm install puppeteer" into scripts/build_reserved.js and explicitly specify the version there?

[cometkim]

So I would prefer not to add it as a dev dependency. Maybe we could instead move the "npm install puppeteer" into scripts/build_reserved.js and explicitly specify the version there?

that makes sense, wouldn't have any integrity check anyway

However, IMHO the goal should not be that the dependencies grow (unless there is a very good reason).

But I agreed with this, and have a plan to remove the keyword table as follows #6658, then we don't need puppeteer at all

[cknitt]

This doesn't make any difference on npm install speed right now.

@cometkim Hmm, actually it seems to make things slower in total, e.g.:

Before:

macos-13:

• Use Node.js: 1s
• Install npm packages: 4s

windows-latest:

• Use Node.js: 8s
• Install npm packages: 9s

After:

macos-13:

• Use Node.js: 4s
• Install npm packages: 3s

windows-latest:

• Use Node.js: 16s
• Install npm packages: 8s

[cometkim]

Hmmmm... 🤔 Ok we're not going to use cache as we don't need it (yet?) then

[cknitt]

But I agreed with this, and have a plan to remove the keyword table as follows #6658, then we don't need puppeteer at all

Would be great if we could get rid of puppeteer altogether! 👍