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

Bug: All ESLint 8 commands fail if the project path contains URL-encoded characters #15766

Closed
1 task
chucknelson opened this issue Apr 7, 2022 · 5 comments · Fixed by #15795
Closed
1 task
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly build This change relates to ESLint's build process repro:needed
Projects

Comments

@chucknelson
Copy link

chucknelson commented Apr 7, 2022

Environment

Node version: 14.18.2
npm version: 6.14.15
Local ESLint version: 8.12.0
Global ESLint version: none
Operating System: Darwin 20.6.0 (macOS Big Sur)

What parser are you using?

@typescript-eslint/parser

What did you do?

Try to run any ESLint command in a project path with a URL encoded character, such as /my%2Fproject/yarn eslint --env-info.

What did you expect to happen?

Command would succeed with the expected output, such as:

/Users/myuser/my%2Fproject/node_modules/.bin/eslint --debug --env-info
eslint:cli CLI args: [ '--debug', '--env-info' ] +0ms
Environment Info:

Node version: v14.18.2
npm version: v6.14.15
Local ESLint version: v8.12.0 (Currently used)
Global ESLint version: Not found
Operating System: darwin 20.6.0

What actually happened?

Example

A project with eslint@8.12.0 installed is in the following path that includes a URL-encoded slash:

/Users/myuser/my%2Fproject

Run any ESLint command, such as --env-info and it fails with the following:

/Users/myuser/my%2Fproject/node_modules/.bin/eslint --debug --env-info

Oops! Something went wrong! :(

ESLint: 8.12.0

TypeError [ERR_INVALID_ARG_VALUE]: The argument 'filename' must be a file URL object, file URL string, or absolute path string. Received 'file:///Users/d108059/Development/tmp/my%2Fproject/node_modules/@eslint/eslintrc/dist/eslintrc.cjs'
    at new NodeError (internal/errors.js:322:7)
    at Function.createRequire (internal/modules/cjs/loader.js:1182:13)
    at Object.<anonymous> (/Users/d108059/Development/tmp/my%2Fproject/node_modules/@eslint/eslintrc/dist/eslintrc.cjs:2383:26)
    at Module._compile (/Users/d108059/Development/tmp/my%2Fproject/node_modules/v8-compile-cache/v8-compile-cache.js:192:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:790:12)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (/Users/d108059/Development/tmp/my%2Fproject/node_modules/v8-compile-cache/v8-compile-cache.js:159:20)
    at Object.<anonymous> (/Users/d108059/Development/tmp/my%2Fproject/node_modules/eslint/lib/cli-engine/cli-engine.js:33:5)
error Command failed with exit code 2.

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

Prior versions of ESLint, such as 7.32.0, work fine with these paths. For example, this works fine:

/Users/myuser/my%2Fproject/node_modules/.bin/eslint --debug --env-info
eslint:cli CLI args: [ '--debug', '--env-info' ] +0ms
Environment Info:

Node version: v14.18.2
npm version: v6.14.15
Local ESLint version: v7.32.0 (Currently used)
Global ESLint version: Not found
Operating System: darwin 20.6.0
@chucknelson chucknelson added bug ESLint is working incorrectly repro:needed labels Apr 7, 2022
@eslint-github-bot eslint-github-bot bot added this to Needs Triage in Triage Apr 7, 2022
@nzakas
Copy link
Member

nzakas commented Apr 8, 2022

This error is coming from require(), which means it’s coming from inside Node.js. Here’s the line:

} = require("@eslint/eslintrc");

Do you see this behavior in ESLint v8.0.0?

@nzakas nzakas moved this from Needs Triage to Triaging in Triage Apr 8, 2022
@fasttime
Copy link
Member

fasttime commented Apr 9, 2022

Looking at the line /@eslint/eslintrc/dist/eslintrc.cjs:2383:26 hinted by the stack trace, I can find this code:

const require$1 = Module.createRequire((typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __filename).href : (document.currentScript && document.currentScript.src || new URL('eslintrc.cjs', document.baseURI).href)));

You can see the whole file here: https://unpkg.com/@eslint/eslintrc@1.2.1/dist/eslintrc.cjs

The corresponding source code is this harmless-looking line in eslintrc:

const require = createRequire(import.meta.url);

which can be found here: https://github.com/eslint/eslintrc/blob/9b719813fc6f023b722168a4f67d895106e875ce/lib/config-array-factory.js#L58

From what I can tell, the generated code is trying to create a file URL from __filename, but doing so in a very error-prone way: 'file:' + __filename rather than using the builtin Node function pathToFileURL. The resulting URL will contain an escaped slash (%2F) rather than the proper escape sequence for "%2F" (%252F).

This is not a bug in ESLint, but rather an issue with the build tools used in eslintrc, most probably Rollup.

@nzakas
Copy link
Member

nzakas commented Apr 12, 2022

@fasttime wow, thanks for the detailed analysis. We will look deeper into this.

@nzakas nzakas added build This change relates to ESLint's build process accepted There is consensus among the team that this change meets the criteria for inclusion labels Apr 12, 2022
mdjermanovic added a commit to eslint/eslintrc that referenced this issue Apr 13, 2022
@mdjermanovic
Copy link
Member

@fasttime thanks for all the details! I prepared a possible fix based on your suggestion.

eslint/eslintrc#77

@mdjermanovic mdjermanovic moved this from Triaging to Pull Request Opened in Triage Apr 21, 2022
mdjermanovic added a commit to eslint/eslintrc that referenced this issue Apr 21, 2022
* fix: use custom Rollup plugin for `import.meta.url`

Refs eslint/eslint#15766

* Use URL#toString()

Co-authored-by: 唯然 <weiran.zsd@outlook.com>

Co-authored-by: 唯然 <weiran.zsd@outlook.com>
Triage automation moved this from Pull Request Opened to Complete Apr 22, 2022
mdjermanovic added a commit that referenced this issue Apr 22, 2022
* fix: allow project paths to have URL-encoded characters

Fixes #15766

* test with eslintrc main branch

* use v1.2.2
@chucknelson
Copy link
Author

Thanks all for the quick response! ❤️

srijan-deepsource pushed a commit to DeepSourceCorp/eslint that referenced this issue May 30, 2022
* fix: allow project paths to have URL-encoded characters

Fixes eslint#15766

* test with eslintrc main branch

* use v1.2.2
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Oct 20, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly build This change relates to ESLint's build process repro:needed
Projects
Archived in project
Triage
Complete
Development

Successfully merging a pull request may close this issue.

4 participants