Fix node-version-file interprets entire package.json as a version (#865)

* Fix URL of locally-cached Node.js version in README

* Change example of Major versions in Supported version syntax

setup-node v4 supports node v20, but these example may
cause misunderstanding like a v20 is not supported.

---------

Co-authored-by: fusagiko/takayamaki <takayamaki@users.noreply.github.com>

.github/workflows/basic-validation.yml

@@ -15,3 +15,5 @@ jobs:
   call-basic-validation:
     name: Basic validation
     uses: actions/reusable-workflows/.github/workflows/basic-validation.yml@main
+    with:
+      node-version: '20.x'

.github/workflows/check-dist.yml

@@ -15,3 +15,5 @@ jobs:
   call-check-dist:
     name: Check dist/
     uses: actions/reusable-workflows/.github/workflows/check-dist.yml@main
+    with:
+      node-version: '20.x'

.github/workflows/e2e-cache.yml

@@ -93,7 +93,7 @@ jobs:
           node-version: ${{ matrix.node-version }}
           cache: 'yarn'
       - name: Install dependencies
-        run: yarn install
+        run: yarn install --ignore-engines
       - name: Verify node and yarn
         run: __tests__/verify-node.sh "${{ matrix.node-version }}"
         shell: bash

README.md

@@ -18,14 +18,14 @@ See [action.yml](action.yml)
 
 <!-- start usage -->
 ```yaml
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     # Version Spec of the version to use in SemVer notation.
     # It also emits such aliases as lts, latest, nightly and canary builds
     # Examples: 12.x, 10.15.1, >=10.15.0, lts/Hydrogen, 16-nightly, latest, node
     node-version: ''
 
-    # File containing the version Spec of the version to use.  Examples: .nvmrc, .node-version, .tool-versions.
+    # File containing the version Spec of the version to use.  Examples: package.json, .nvmrc, .node-version, .tool-versions.
     # If node-version and node-version-file are both provided the action will use version from node-version. 
     node-version-file: ''
 
@@ -84,7 +84,7 @@ See [action.yml](action.yml)
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: 18
 - run: npm ci
@@ -103,12 +103,12 @@ The `node-version` input supports the Semantic Versioning Specification, for mor
 
 Examples:
 
- - Major versions: `14`, `16`, `18`
+ - Major versions: `16`, `18`, `20`
  - More specific versions: `10.15`, `16.15.1` , `18.4.0`
  - NVM LTS syntax: `lts/erbium`, `lts/fermium`, `lts/*`, `lts/-n`
  - Latest release: `*` or `latest`/`current`/`node`
 
-**Note:** Like the other values, `*` will get the latest [locally-cached Node.js version](https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md#nodejs), or the latest version from [actions/node-versions](https://github.com/actions/node-versions/blob/main/versions-manifest.json), depending on the [`check-latest`](docs/advanced-usage.md#check-latest-version) input.
+**Note:** Like the other values, `*` will get the latest [locally-cached Node.js version](https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#nodejs), or the latest version from [actions/node-versions](https://github.com/actions/node-versions/blob/main/versions-manifest.json), depending on the [`check-latest`](docs/advanced-usage.md#check-latest-version) input.
 
 `current`/`latest`/`node` always resolve to the latest [dist version](https://nodejs.org/dist/index.json).
 That version is then downloaded from actions/node-versions if possible, or directly from Node.js if not.
@@ -133,7 +133,7 @@ See the examples of using cache for `yarn`/`pnpm` and `cache-dependency-path` in
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: 16
     cache: 'npm'
@@ -146,7 +146,7 @@ steps:
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: 16
     cache: 'npm'
@@ -168,7 +168,7 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Setup node
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node }}
       - run: npm ci
@@ -182,7 +182,7 @@ jobs:
 To get a higher rate limit, you can [generate a personal access token on github.com](https://github.com/settings/tokens/new) and pass it as the `token` input for the action:
 
 ```yaml
-uses: actions/setup-node@v3
+uses: actions/setup-node@v4
 with:
   token: ${{ secrets.GH_DOTCOM_TOKEN }}
   node-version: 16

__tests__/main.test.ts

@@ -101,6 +101,7 @@ describe('main tests', () => {
       ${'  14.1.0  '}                              | ${'14.1.0'}
       ${'{"volta": {"node": ">=14.0.0 <=17.0.0"}}'}| ${'>=14.0.0 <=17.0.0'}
       ${'{"engines": {"node": "17.0.0"}}'}         | ${'17.0.0'}
+      ${'{}'}                                      | ${null}
     `.it('parses "$contents"', ({contents, expected}) => {
       expect(util.parseNodeVersionFile(contents)).toBe(expected);
     });

action.yml

@@ -8,7 +8,7 @@ inputs:
   node-version:
     description: 'Version Spec of the version to use. Examples: 12.x, 10.15.1, >=10.15.0.'
   node-version-file:
-    description: 'File containing the version Spec of the version to use.  Examples: .nvmrc, .node-version, .tool-versions.'
+    description: 'File containing the version Spec of the version to use.  Examples: package.json, .nvmrc, .node-version, .tool-versions.'
   architecture:
     description: 'Target architecture for Node to use. Examples: x86, x64. Will use system architecture by default.'
   check-latest:

dist/cache-save/index.js

@@ -82953,11 +82953,20 @@ process.on('uncaughtException', e => {
     const warningPrefix = '[warning]';
     core.info(`${warningPrefix}${e.message}`);
 });
-function run() {
+// Added early exit to resolve issue with slow post action step:
+function run(earlyExit) {
     return __awaiter(this, void 0, void 0, function* () {
         try {
             const cacheLock = core.getState(constants_1.State.CachePackageManager);
+            if (cacheLock) {
                 yield cachePackages(cacheLock);
+                if (earlyExit) {
+                    process.exit(0);
+                }
+            }
+            else {
+                core.debug(`Caching for '${cacheLock}' is not supported`);
+            }
         }
         catch (error) {
             core.setFailed(error.message);
@@ -82990,7 +82999,7 @@ const cachePackages = (packageManager) => __awaiter(void 0, void 0, void 0, func
     }
     core.info(`Cache saved with the key: ${primaryKey}`);
 });
-run();
+run(true);
 
 
 /***/ }),
@@ -83329,9 +83338,25 @@ function parseNodeVersionFile(contents) {
     let nodeVersion;
     // Try parsing the file as an NPM `package.json` file.
     try {
-        nodeVersion = (_a = JSON.parse(contents).volta) === null || _a === void 0 ? void 0 : _a.node;
+        const manifest = JSON.parse(contents);
+        // JSON can parse numbers, but that's handled later
+        if (typeof manifest === 'object') {
+            nodeVersion = (_a = manifest.volta) === null || _a === void 0 ? void 0 : _a.node;
             if (!nodeVersion)
-            nodeVersion = (_b = JSON.parse(contents).engines) === null || _b === void 0 ? void 0 : _b.node;
+                nodeVersion = (_b = manifest.engines) === null || _b === void 0 ? void 0 : _b.node;
+            // if contents are an object, we parsed JSON
+            // this can happen if node-version-file is a package.json
+            // yet contains no volta.node or engines.node
+            //
+            // if node-version file is _not_ json, control flow
+            // will not have reached these lines.
+            //
+            // And because we've reached here, we know the contents
+            // *are* JSON, so no further string parsing makes sense.
+            if (!nodeVersion) {
+                return null;
+            }
+        }
     }
     catch (_d) {
         core.info('Node version file is not JSON file');

dist/setup/index.js

@@ -93728,7 +93728,13 @@ function resolveVersionInput() {
         if (!fs_1.default.existsSync(versionFilePath)) {
             throw new Error(`The specified node version file at: ${versionFilePath} does not exist`);
         }
-        version = (0, util_1.parseNodeVersionFile)(fs_1.default.readFileSync(versionFilePath, 'utf8'));
+        const parsedVersion = (0, util_1.parseNodeVersionFile)(fs_1.default.readFileSync(versionFilePath, 'utf8'));
+        if (parsedVersion) {
+            version = parsedVersion;
+        }
+        else {
+            core.warning(`Could not determine node version from ${versionFilePath}. Falling back`);
+        }
         core.info(`Resolved ${versionFileInput} as ${version}`);
     }
     return version;
@@ -93783,9 +93789,25 @@ function parseNodeVersionFile(contents) {
     let nodeVersion;
     // Try parsing the file as an NPM `package.json` file.
     try {
-        nodeVersion = (_a = JSON.parse(contents).volta) === null || _a === void 0 ? void 0 : _a.node;
+        const manifest = JSON.parse(contents);
+        // JSON can parse numbers, but that's handled later
+        if (typeof manifest === 'object') {
+            nodeVersion = (_a = manifest.volta) === null || _a === void 0 ? void 0 : _a.node;
             if (!nodeVersion)
-            nodeVersion = (_b = JSON.parse(contents).engines) === null || _b === void 0 ? void 0 : _b.node;
+                nodeVersion = (_b = manifest.engines) === null || _b === void 0 ? void 0 : _b.node;
+            // if contents are an object, we parsed JSON
+            // this can happen if node-version-file is a package.json
+            // yet contains no volta.node or engines.node
+            //
+            // if node-version file is _not_ json, control flow
+            // will not have reached these lines.
+            //
+            // And because we've reached here, we know the contents
+            // *are* JSON, so no further string parsing makes sense.
+            if (!nodeVersion) {
+                return null;
+            }
+        }
     }
     catch (_d) {
         core.info('Node version file is not JSON file');

docs/advanced-usage.md

@@ -46,7 +46,7 @@ If `check-latest` is set to `true`, the action first checks if the cached versio
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '16'
     check-latest: true
@@ -64,7 +64,7 @@ See [supported version syntax](https://github.com/actions/setup-node#supported-v
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version-file: '.nvmrc'
 - run: npm ci
@@ -96,7 +96,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: '14'
           architecture: 'x64' # optional, x64 or x86. If not specified, x64 will be used by default
@@ -117,7 +117,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: '20.0.0-v8-canary' # it will install the latest v8 canary release for node 20.0.0
       - run: npm ci
@@ -132,7 +132,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: '20-v8-canary' # it will install the latest v8 canary release for node 20
       - run: npm ci
@@ -148,7 +148,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: 'v20.1.1-v8-canary20221103f7e2421e91'
       - run: npm ci
@@ -168,7 +168,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: '16-nightly' # it will install the latest nightly release for node 16
       - run: npm ci
@@ -184,7 +184,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: '16.0.0-nightly' # it will install the latest nightly release for node 16.0.0
       - run: npm ci
@@ -200,7 +200,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: '16.0.0-nightly20210420a0261d231c'
       - run: npm ci
@@ -218,7 +218,7 @@ jobs:
     name: Node sample
     steps:
       - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
+      - uses: actions/setup-node@v4
         with:
           node-version: '16.0.0-rc.1'
       - run: npm ci
@@ -235,7 +235,7 @@ Yarn caching handles both yarn versions: 1 or 2.
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14'
     cache: 'yarn'
@@ -257,7 +257,7 @@ steps:
 - uses: pnpm/action-setup@v2
   with:
     version: 6.32.9
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14'
     cache: 'pnpm'
@@ -273,7 +273,7 @@ steps:
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14'
     cache: 'npm'
@@ -286,7 +286,7 @@ steps:
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14'
     cache: 'npm'
@@ -324,7 +324,7 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Setup node
-        uses: actions/setup-node@v3
+        uses: actions/setup-node@v4
         with:
           node-version: ${{ matrix.node_version }}
           architecture: ${{ matrix.architecture }}
@@ -336,7 +336,7 @@ jobs:
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14.x'
     registry-url: 'https://registry.npmjs.org'
@@ -344,7 +344,7 @@ steps:
 - run: npm publish
   env:
     NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     registry-url: 'https://npm.pkg.github.com'
 - run: npm publish
@@ -356,7 +356,7 @@ steps:
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14.x'
     registry-url: <registry url>
@@ -364,7 +364,7 @@ steps:
 - run: yarn publish
   env:
     NODE_AUTH_TOKEN: ${{ secrets.YARN_TOKEN }}
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     registry-url: 'https://npm.pkg.github.com'
 - run: yarn publish
@@ -376,7 +376,7 @@ steps:
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14.x'
     registry-url: 'https://registry.npmjs.org'
@@ -396,7 +396,7 @@ Below you can find a sample "Setup .yarnrc.yml" step, that is going to allow you
 ```yaml
 steps:
 - uses: actions/checkout@v4
-- uses: actions/setup-node@v3
+- uses: actions/setup-node@v4
   with:
     node-version: '14.x'
 - name: Setup .yarnrc.yml

src/cache-save.ts

@@ -12,10 +12,20 @@ process.on('uncaughtException', e => {
   core.info(`${warningPrefix}${e.message}`);
 });
 
-export async function run() {
+// Added early exit to resolve issue with slow post action step:
+export async function run(earlyExit?: boolean) {
   try {
     const cacheLock = core.getState(State.CachePackageManager);
+
+    if (cacheLock) {
       await cachePackages(cacheLock);
+
+      if (earlyExit) {
+        process.exit(0);
+      }
+    } else {
+      core.debug(`Caching for '${cacheLock}' is not supported`);
+    }
   } catch (error) {
     core.setFailed((error as Error).message);
   }
@@ -58,4 +68,4 @@ const cachePackages = async (packageManager: string) => {
   core.info(`Cache saved with the key: ${primaryKey}`);
 };
 
-run();
+run(true);

src/main.ts

@@ -105,7 +105,17 @@ function resolveVersionInput(): string {
       );
     }
 
-    version = parseNodeVersionFile(fs.readFileSync(versionFilePath, 'utf8'));
+    const parsedVersion = parseNodeVersionFile(
+      fs.readFileSync(versionFilePath, 'utf8')
+    );
+
+    if (parsedVersion) {
+      version = parsedVersion;
+    } else {
+      core.warning(
+        `Could not determine node version from ${versionFilePath}. Falling back`
+      );
+    }
 
     core.info(`Resolved ${versionFileInput} as ${version}`);
   }

src/util.ts

@@ -1,13 +1,31 @@
 import * as core from '@actions/core';
 import * as exec from '@actions/exec';
 
-export function parseNodeVersionFile(contents: string): string {
+export function parseNodeVersionFile(contents: string): string | null {
   let nodeVersion: string | undefined;
 
   // Try parsing the file as an NPM `package.json` file.
   try {
-    nodeVersion = JSON.parse(contents).volta?.node;
-    if (!nodeVersion) nodeVersion = JSON.parse(contents).engines?.node;
+    const manifest = JSON.parse(contents);
+
+    // JSON can parse numbers, but that's handled later
+    if (typeof manifest === 'object') {
+      nodeVersion = manifest.volta?.node;
+      if (!nodeVersion) nodeVersion = manifest.engines?.node;
+
+      // if contents are an object, we parsed JSON
+      // this can happen if node-version-file is a package.json
+      // yet contains no volta.node or engines.node
+      //
+      // if node-version file is _not_ json, control flow
+      // will not have reached these lines.
+      //
+      // And because we've reached here, we know the contents
+      // *are* JSON, so no further string parsing makes sense.
+      if (!nodeVersion) {
+        return null;
+      }
+    }
   } catch {
     core.info('Node version file is not JSON file');
   }