import-js / eslint-plugin-import

ESLint plugin with rules that help validate proper imports.
MIT License
5.54k stars 1.57k forks source link

Wrong file chosen when comparing case sensitivity for import/no-unresolved #2335

Open jeraki opened 2 years ago

jeraki commented 2 years ago

I came across a situation where a file named "OWNERS" and a file named "Owners.jsx" in the same directory caused the "import/no-unresolved" rule to report a case mismatch when another module imported "Owners". Removing or renaming the file named "OWNERS" prevents the error. This behavior can be reproduced with the following minimal example:

Setup

Given the following files in a directory:

package.json:

{
  "devDependencies": {
    "eslint": "^8.5.0",
    "eslint-plugin-import": "^2.25.3"
  }
}

.eslintrc.yml:

parserOptions:
  ecmaVersion: 2015
  sourceType: module
plugins:
  - import
rules:
  import/no-unresolved: 2

src/Foo.js:

export default function Foo() {
}

src/Bar.js:

import Foo from "./Foo";

export default function Bar() {
  return Foo();
}

src/FOO:

(contents are irrelevant)

Action

Run in a shell:

yarn install
yarn run eslint src

Expected behavior

No errors should be reported.

Actual behavior

An error is reported:

/path/to/example/src/Bar.js
  1:17  error  Casing of ./Foo does not match the underlying filesystem  import/no-unresolved

✖ 1 problem (1 error, 0 warnings)

Additional info

tyuqing commented 2 years ago

me too

ljharb commented 2 years ago

Actually I think this is a valid warning.

Essentially, for a specifier Foo, when a file Foo exists with matching casing, Foo.js will be ignored. Having a file named "FOO", then, means that on a case-insensitive filesystem, your import will grab FOO instead of Foo.js, and the linter warning is thus correct.

I'll add this test case - warning, as expected - to close the issue.

I do agree that the same behavior should occur on a case-sensitive disk as well, so if our tests are covering that, then the issue won't be closed and I'll try to fix the discrepancy :-)

ljharb commented 2 years ago

oops, looks like indeed the test does fail to warn on a case-sensitive volume, so i'll use that to fix it.

jeraki commented 2 years ago

Essentially, for a specifier Foo, when a file Foo exists with matching casing, Foo.js will be ignored. Having a file named "FOO", then, means that on a case-insensitive filesystem, your import will grab FOO instead of Foo.js, and the linter warning is thus correct.

In our system, the test that imports "Owners" correctly resolves to "Owners.jsx" rather than "OWNERS", and the test file that does this import works as expected. This observed behavior contradicts what you're saying, although it's possible the resolution is successful because of our build system's configuration, and may not be successful in all projects with this same scenario.

Thank you for looking into it! I appreciate your time.

ljharb commented 2 years ago

@jimmymeraki right - on your system it's correct, but on a case INSENSITIVE system, i think it would resolve to OWNERS.

jeraki commented 2 years ago

This isn't true in our case. From my original report:

The problem is reproducible on macOS 10.15.7 on a case-insensitive APFS volume, but not on Linux on a case-sensitive volume. This suggests file system case sensitivity is a necessary condition for the problem.

But this is only in regards to the eslint-plugin-import warning. The actual import resolves to the correct file in both environments at runtime.

ljharb commented 2 years ago

Interesting, hmm 👀