Fix npm cache compatibility

[0-vortex]

Description

This PR makes the cache property work for all package managers.

Motivation

The motivation of this pull request is to be able to use ALL package managers while not compromising security - in order to do that we have to restore npm functionality and align that usage to the docs.

The blog post for npm v7 will include support for yarn.lock files

One common question we’ve gotten a few times now, once we announce that npm v7 will include support for yarn.lock files, is “Why keep package-lock.json at all, then? Why not just use yarn.lock only?”
The simple answer is: because yarn.lock doesn’t fully address npm’s needs, and relying on it exclusively would limit our ability to produce optimal package installs or add features in the future.

^ Read to the bottom if you want the full reasoning of the npm team/inventor.

Testing

Have added some POCs based real-life use cases proving it is not reliable to cross-use the yarn.lock file between npm and yarn for node-setup v2.2.0 and v2.1.5:

• https://github.com/0-vortex/node-setup-npm-production-not-compatible-yarn
• https://github.com/0-vortex/node-setup-npm-production-not-compatible-yarn-pre-2.2.0

[maxim-lobanov]

Why do you drop yarn.lock option? It is valid option for NPM 7.

[0-vortex]

The blog post I mentioned describes this issue very well - https://blog.npmjs.org/post/621733939456933888/npm-v7-series-why-keep-package-lockjson

Have also linked practical explanations of why "supporting" does not mean it's re-usable - npm ci command does not work with yarn.lock only npm install does. Furthermore, they do not share the same formatting, yarn does not use hyphens while npm does, attempting to interop them will lead to changes that cannot be easily differentiated in bash or other simple solutions.

• https://github.com/0-vortex/node-setup-npm-production-not-compatible-yarn/actions
• https://github.com/0-vortex/node-setup-npm-production-not-compatible-yarn-pre-2.2.0/actions

Have made 2 repositories so to prove the pushed changes never actually worked in previous versions either, supporting a file format does not infer compatibility. Furthermore, it is not debatable that lockFilePatterns: ['package-lock.json', 'npm-shrinkwrap.json'], are the only practical valid formats for npm versions 3-6 and 7 because of existing practices and standards and if we envision working interoperability the code changes to this repository are much more complex, requiring additional documentation on how to properly deal with the quirks.

Simply put, the amount of effort required to achieve lockFilePatterns: ['package-lock.json', 'npm-shrinkwrap.json', 'yarn.lock'], in a backwards compatible way with the current practices (not just actions/setup-node) is out of my individual reach - would need some discussions and coordination from the team beside code changes.

At the moment unsupported npm ci usage in conjunction with the actions/setup-node cache feature quirks not being documented will lead to error messages that will be irrelevant to amateur practitioners; in the case of seasoned developers there are multiple things that come to mind to resolve them, but all involve some form of technical debt like switching from npm ci to npm install will compromise most security practices since running install on top of a yarn.lock file will at least correct the JSON formatting, thus making the file diffable only with yarn, inviting users to switch to yarn in the best case scenario.

Am happy to contribute the compatibility layer of the cache feature with some guidance, currently this fix enables npm to work with the actions/setup-node cache feature in all scenarios in a backwards compatible way and without any technical debt.

[dmitry-shibanov]

Hello @0-vortex. We do not mind to add support for npm-shrinkwrap.json. I think it will even be better to place npm-shrinkwrap.json the first due to the fact that npm-shrinkwrap.json has higher priority than package-lock.json.

But we would prefer to keep yarn.lock, because it's still a valid case to use it with npm 7 and npm install.
Could you please update your pull request according to this.

[0-vortex]

it really feels like you are just trying to convince me to switch to yarn at this point.

have already explained how adding that does not work. have 4 repositories outlining all the issues, what more can I do here ?

just close the pull request if you were never going to pass it anyway.

[dmitry-shibanov]

Hello @0-vortex . The pull request with adding support for monorepos and projects with complex structure was merged. You can specify npm-shrinkwrap.json with this code snippet.

- uses: actions/setup-node@v2
  with:
    node-version: '14'
    cache: 'npm'
    cache-dependency-path: npm-shrinkwrap.json

For now I'm closing the pull request.

[0-vortex]

Thank you for answering and using SemVer, we are still pinned to 2.1.5 and using environment cache keys on top of that. As npm was already doing the per project configuration right we experienced a bigger need for extending node configuration and performance on the runner rather than unifying package management usage through this action. Nonetheless we will give the new version a try!