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

Allow using workspace folder as working directory for eslint server start when in a trusted workspace #1602

Open
koshic opened this issue Feb 8, 2023 · 8 comments
Labels
feature-request Request for new features or functionality
Milestone

Comments

@koshic
Copy link

koshic commented Feb 8, 2023

Use case: to enable ESLint configs written in TypeScript, we should pass '--loader TS_LOADER_PACKAGE_OR_PATH' to Node runtime. Also, it would be nice to use host Node binary (not bundled with VSCode).

Theoretically, we can use "eslint.runtime": "node" & "eslint.execArgv": ["--experimental-loader", "TS_LOADER_PACKAGE_OR_PATH"] settings combination. But problem is that 'runtime' binary executed with cwd: process.cwd() in client/src/client.ts:

const result: ServerOptions = {
	run: { module: serverModule, transport: TransportKind.ipc, runtime, options: { execArgv, cwd: process.cwd(), env } },
	debug: { module: serverModule, transport: TransportKind.ipc, runtime, options: { execArgv: execArgv !== undefined ? execArgv.concat(debugArgv) : debugArgv, cwd: process.cwd(), env } }
};

In my case, process.cwd() is '/', so Node just can't find TS loader installed in my project directory. Sure, absolute path can be used... but it machine dependent. And there is no ability to use something like "${WORKSPACE_DIR}/node-modules/ts-loader-path" in VSCode settings.

As a workaround, I use special script passed to extension via "eslint.runtime": ".vscode/spawn-node.js", then call 'process.chdir(dirname(fileURLToPath(import.meta.url)))' before spawn real Node process (which is required a bit magic with IPC, but it's out of scope).

So, my proposal is to provide ability to use project directory as cwd on runtime execution. As additional parameter like 'eslint.executeRuntimeInWorkspaceDir', or by default - it doesn't matter.

@dbaeumer
Copy link
Member

dbaeumer commented Feb 8, 2023

The underlying problem here is that ESLint (the npm module) is very sensitive to the working directory when it comes top config file resolving.

The working directories used can be control by the eslint.workingDirectory. Have to tried to use one setting that awlays uses the root of the workspace?

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Feb 8, 2023
@koshic
Copy link
Author

koshic commented Feb 8, 2023

Problem appears before Node loads something (ESLint, configs, etc.) - I'm talking about Node startup flags, and module resolution at this stage: '--experimental-loader' CLI flag requires absolute path / file URL / global package name (which is not suitable), or relative path / package name which is resolved in current directory (== Node process cwd).

You can easily reproduce this issue: just create empty 'loader.mjs' (it's 100% valid from Node perspective) in project directory, add "eslint.runtime": "node"
& "eslint.execArgv": ["--experimental-loader", "./loader.mjs"]" to the VSCode settings and open project in VSCode. Node failed to start because can't find './loader.mjs' file.

@dbaeumer
Copy link
Member

Understood: the problem is that for security reasons the working directory the node executable is started on is not the workspace folder. This is why it will not find the file right now. As a workaround you might want to try an absolute path.

Need to think about if I could change this in trusted workspaces.

@dbaeumer dbaeumer added feature-request Request for new features or functionality and removed info-needed Issue requires more information from poster labels Feb 10, 2023
@dbaeumer dbaeumer changed the title 'eslint.runtime' binary executed with wrong cwd Allow using workspace folder as working directory for eslint server start when in a trusted workspace Feb 10, 2023
@dbaeumer dbaeumer added this to the Backlog milestone Feb 10, 2023
@psychobolt
Copy link

psychobolt commented Apr 26, 2023

In order for ESLint to load eslint.config.js for ESM (type: modules) projects, we have to include --loader ./.pnp.loader.mjs in execArgv, otherwise modules are resolve through pnp as commonjs.

Unfortunately, I had to create a note on the workaround in the devs readme for configuring the execArgv in user settings. psychobolt/react-rollup-boilerplate@c376122#diff-fc2a310fcfedfefb0046def697f932def80cbb1e78c689bc0af3c8eab3e33eceR65.

Supposedly shell script is doable, yarn node but not supported on Windows without WSL. e.g. "eslint.runtime": "./yarn-node.sh"

@koshic
Copy link
Author

koshic commented Apr 26, 2023

@psychobolt my yarn-node.js (I prefer to store it inside .vscode dir), works fine with PnP + ESM:

#!/usr/bin/env node

import { execSync, spawn } from "node:child_process";
import { dirname } from "node:path";
import { fileURLToPath } from "node:url";

process.chdir(dirname(fileURLToPath(import.meta.url)));

const nodeOptions = execSync("yarn node -p process.env.NODE_OPTIONS")
  .toString()
  .slice(0, -1);

const child = spawn("node", [...process.argv.slice(2)], {
  env: {
    ...process.env,
    NODE_OPTIONS: nodeOptions,
  },
  stdio: ["inherit", "inherit", "inherit", "ipc"],
}).on("message", (data) => {
  if (process.send !== undefined) {
    process.send(data);
  }
});

process.on("message", (data) => {
  // @ts-expect-error
  child.send(data);
});

@psychobolt
Copy link

psychobolt commented Apr 27, 2023

Above solution works on OSX. I had to update my file permissions to give spawn access. https://edwardbeazer.com/how-to-fix-this-error-spawn-eacces/

Doesn't work on Windows though :/ .

Error: spawn UNKNOWN
	at ChildProcess.spawn (node:internal/child_process:413:11)
	at Object.spawn (node:child_process:720:9)
	at c:\Users\psychobolt\.vscode\extensions\dbaeumer.vscode-eslint-2.4.0\client\out\extension.js:1:269273
	at async w.createConnection (c:\Users\psychobolt\.vscode\extensions\dbaeumer.vscode-eslint-2.4.0\client\out\extension.js:1:107849)
	at async w.start (c:\Users\psychobolt\.vscode\extensions\dbaeumer.vscode-eslint-2.4.0\client\out\extension.js:1:98364)

@psychobolt
Copy link

psychobolt commented Dec 18, 2023

Back at it trying to troubleshoot Windows.

Seems like for Windows, passing the script file as the spawn command will create UKNOWN error. Reference: https://stackoverflow.com/questions/43419893/how-do-i-fix-error-spawn-unknown-with-node-js-v7-8-0-on-windows-10

I'm not sure if it was the intention of the language client API, to allow a script command. So I tried the following:

{
  "eslint.runtime": "node",
  "eslint.execArgv": ["./bin/eslint-runtime.js"],
}

This time I got around the previous error however, the entry point is resolved to working directory of VSCode.

Error: Cannot find module 'C:\Users\psychobolt\AppData\Local\Programs\Microsoft VS Code\bin\eslint-runtime.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1144:15)
    at Module._load (node:internal/modules/cjs/loader:985:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:135:12)
    at node:internal/main/run_main_module:28:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

So for Windows you'll have to end up pointing to the same issues as OP, having to use absolute path in the settings. The following works, if you need to pass your runtime with additional NODE_OPTIONS.

{
  "eslint.runtime": "node",
  "eslint.execArgv": ["D://User/my-project/bin/eslint-runtime.js"],
}

So once again, I have to include this workaround in junction with my previous update, the devs readme: https:/psychobolt/vite-storybook-boilerplate/blob/f3a19cfbddbb5f7ff9c4a9894089b0aba2f2cf8f/DEVELOPMENT.md#windows

I'd prefer if we can have the some predefined variables. Some other extensions that support variables:

VSCode Flow:

"flow.pathToFlow": "${workspaceFolder}/.yarn/sdks/flow-bin/cli.js",

and also VSCode-Python:

"python.venvPath": "${workspaceRoot}/venv"

@dbaeumer
Copy link
Member

@psychobolt IMO the right approach is to fix microsoft/vscode#2809 in VS Code instead of every exension coming up with their own story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

3 participants