Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node 20: process.chdir can break via typescript-eslint v7 (works in Node 18) #53730

Closed
ajvincent opened this issue Jul 4, 2024 · 7 comments
Closed
Labels
invalid Issues and PRs that are invalid.

Comments

@ajvincent
Copy link

Version

v20.15.0

Platform

Linux fedora 6.9.7-200.fc40.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Jun 27 18:11:45 UTC 2024 x86_64 GNU/Linux

Subsystem

process

What steps will reproduce the bug?

  1. Create the following files:

package.json

{
  "type": "module",
  "engines": {
    "node": ">=18.19"
  },
  "devDependencies": {
    "@tsconfig/node18": "^18.2.4",
    "@types/eslint": "^8.56.10",
    "@types/node": "^20.14.8",
    "@typescript-eslint/eslint-plugin": "^7.13.1",
    "@typescript-eslint/parser": "^7.13.1",
    "eslint": "^8.57.0",
    "eslint-import-resolver-typescript": "^3.6.1",
    "eslint-plugin-import": "^2.29.1",
    "ts-node": "^10.9.2",
    "tslib": "^2.6.3",
    "typescript": "^5.5.2"
  }
}

tsconfig.json

{
  "compilerOptions": {
    "lib": ["ESNext"],
    "module": "es2022",
    "target": "esnext",
    "moduleResolution": "node",

    "baseUrl": "."
  },

  "extends": "@tsconfig/node18/tsconfig.json"
}

build.js (this is where the bug manifests)

import path from "path";
import { chdir, cwd } from 'node:process';
import assert from "node:assert/strict";

async function recursiveBuild(dirName, relativePathToModule) {
    const popDir = cwd();
    try {
        const pushDir = path.resolve(popDir, dirName);
        const scriptToLoad = path.resolve(pushDir, relativePathToModule);
        console.log(`push from popDir ${popDir} to ${pushDir}, loading ${scriptToLoad}`);
        chdir(pushDir);
        await import(scriptToLoad);
    }
    finally {
        console.log(`pop to ${popDir}`);
        chdir(popDir);
        assert.equal(process.cwd(), popDir, "should have popped the directory");
    }
}

await recursiveBuild("utilities", "stage-build.js");

utilities/stage-build.js

import path from "node:path";
import { fileURLToPath } from "node:url";
import assert from "node:assert/strict";

import { ESLint } from "eslint";

console.log("starting utilities, cwd = " + process.cwd());
const expectedDir = path.dirname(fileURLToPath(import.meta.url));
assert.equal(process.cwd(), expectedDir, "where are we?");

async function runESLint(cwd, files) {
    console.log("files to lint:", JSON.stringify(files));
    const eslintRunner = new ESLint({
        cwd,
        overrideConfigFile: path.join(cwd, "typescript-eslintrc.cjs"),
    });

    await eslintRunner.lintFiles(files);
}

console.log("starting utilities:eslint");
await runESLint(expectedDir, [
    "./toBeLinted.ts",
]);

utilities/toBeLinted.ts

import path from "node:path";
void(path);

utilities/typescript-eslintrc.cjs

const configuration = {
  "env": {
    "es2021": true,
    "node": true
  },
  "extends": [
    "eslint:recommended",
    "plugin:import/recommended",
    "plugin:import/typescript",
    "plugin:@typescript-eslint/eslint-recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:@typescript-eslint/recommended-type-checked",
    "plugin:@typescript-eslint/stylistic-type-checked"
  ],

  "overrides": [
    {
      "files": [
        "**/*.ts"
      ]
    }
  ],
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "ecmaVersion": 2022,
    "sourceType": "module",
    "tsconfigRootDir": ".",
    "project": true
  },
  "plugins": [
    "@typescript-eslint",
    "import"
  ],
  "root": true,
  "rules": {
    "@typescript-eslint/ban-types": [
      "error",
      {
        "types": {
          "Function": false
        }
      }
    ],
    "@typescript-eslint/consistent-type-definitions": "off",
    "@typescript-eslint/explicit-function-return-type": ["error"]
  },
  "settings": {
    "import/parsers": {
      "@typescript-eslint/parser": [".ts"]
    },
    "import/resolver": {
      "typescript": {
        "alwaysTryTypes": true, // always try to resolve types under `<root>@types` directory even it doesn't contain any source code, like `@types/unist`

        // Choose from one of the "project" configs below or omit to use <root>/tsconfig.json by default

        "project": "_*/tsconfig.json"
      }
    }
  }
};

module.exports = configuration;
  1. npm install

(I'm sorry. I tried to make this as minimal a testcase as I could.)

  1. Install nvm, so you can switch between NodeJS 18 and NodeJS 20.
  2. Run the following commands from your directory:
nvm use 18
node ./build.js

This completes cleanly: pop to /home/ajvincent/test/nodejs-cwd-regression

  1. Run the following commands from your directory:
nvm use 20
node ./build.js

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior? Why is that the expected behavior?

push from popDir /home/ajvincent/test/nodejs-cwd-regression to /home/ajvincent/test/nodejs-cwd-regression/utilities, loading /home/ajvincent/test/nodejs-cwd-regression/utilities/stage-build.js
starting utilities, cwd = /home/ajvincent/test/nodejs-cwd-regression/utilities
starting utilities:eslint
files to lint: ["./toBeLinted.ts"]
pop to /home/ajvincent/test/nodejs-cwd-regression

What do you see instead?

push from popDir /home/ajvincent/test/nodejs-cwd-regression to /home/ajvincent/test/nodejs-cwd-regression/utilities, loading /home/ajvincent/test/nodejs-cwd-regression/utilities/stage-build.js
starting utilities, cwd = /home/ajvincent/test/nodejs-cwd-regression/utilities
starting utilities:eslint
files to lint: ["./toBeLinted.ts"]
pop to /home/ajvincent/test/nodejs-cwd-regression
node:internal/modules/run_main:129
    triggerUncaughtException(
    ^

AssertionError [ERR_ASSERTION]: should have popped the directory
    at recursiveBuild (file:///home/ajvincent/test/nodejs-cwd-regression/build.js:17:16)
    at async file:///home/ajvincent/test/nodejs-cwd-regression/build.js:21:1 {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: '/home/ajvincent/test/nodejs-cwd-regression/utilities',
  expected: '/home/ajvincent/test/nodejs-cwd-regression',
  operator: 'strictEqual'
}

Node.js v20.15.0

Additional information

Standard eslint version 8 (which typescript-eslint v7 is tied to) linting against JavaScript files doesn't seem to trigger this.

I also suspected this might be related to #53681, another "Node 18 versus Node 20" bug I had filed a few days ago, but testing with some debugging loader hooks I have show that is incorrect. Unfortunately, because of that same bug, I cannot step through in the Chrome Developer Tools to give more information.

@RedYetiDev
Copy link
Member

HIi! Thanks for the report. Currently, your report includes several different files and dependencies. It would easier for members of the community to reproduce if you shorten your example to be minimally reproducible.

When you get a chance, please edit your issue to include a reproducible example (preferably with few files, and no dependencies)

@RedYetiDev RedYetiDev added the process Issues and PRs related to the process subsystem. label Jul 4, 2024
@ajvincent
Copy link
Author

I will do so as soon as I can isolate this. I just discovered a key detail. In Node 18, process.cwd() and cwd() (imported from node:process) give the same result in my original code (far less reduced than this). In Node 20, they give different results, which is bizarre.

@RedYetiDev
Copy link
Member

Weird! They should (IIRC) be the exact same, for example:

async function main() {
    const assert = require('assert');

    const cjsProcess = require('node:process');
    const mjsProcess = await import('node:process');

    for (const [nKey, nValue] of Object.entries(process)) {
        const cValue = cjsProcess[nKey];
        const mValue = mjsProcess[nKey];
        assert.strictEqual(cValue, nValue);
        assert.strictEqual(mValue, nValue);
        assert.strictEqual(cValue, mValue);
    }
}

main();
@ajvincent
Copy link
Author

Continuing to get weirder. I was able to trigger this craziness with just an import statement:

import {
  Extractor,
  ExtractorConfig,
  ExtractorResult
} from '@microsoft/api-extractor';

Yes, I know that API Extractor isn't part of my testcase. That's not the point. The point is cwd() can diverge from process.cwd() (the outputs of these calls) in Node 20 simply by including a given import after calling chdir(pushDir).

Figuring out what statement inside my imported libraries caused that is going to take a while. Thus, so will reducing this testcase further.

@ajvincent
Copy link
Author

This bug is invalid. Deep in the dependency chain is graceful-fs, which has a lovely replacement of process.cwd().

I'm really sorry for all the noise.

@ajvincent ajvincent closed this as not planned Won't fix, can't repro, duplicate, stale Jul 4, 2024
@ajvincent
Copy link
Author

But wait a minute, why did Node 18 let that go and Node 20 didn't?

@RedYetiDev
Copy link
Member

If your issue is with a dependency, it's hard to tell what could cause the issue. If you find the root cause, and you're able to reproduce without any dependencies, please open a new issue :-).

@RedYetiDev RedYetiDev added invalid Issues and PRs that are invalid. and removed process Issues and PRs related to the process subsystem. labels Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid Issues and PRs that are invalid.
2 participants