Bug: no-missing-require incorrectly flagging module as missing

[nzakas]

Environment

Node version: v20.11.0
npm version: v10.4.0
ESLint version: v9.0.0
eslint-plugin-n version: v17.2.0
Operating System: win32 10.0.19045

What rule do you want to report?

no-missing-require

Link to Minimal Reproducible Example

eslint/eslint#18313

What did you expect to happen?

That the rule would correctly find the module @humanwhocodes/retry.

Worked in v16.6.2, stopped working in v17.0.0.

Participation

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

Additional comments

No response

[scagood]

After some looking, this is related to: #182

The error is that the exports are the wrong way around:
https://github.com/humanwhocodes/retry/blob/8f8af13/package.json#L10-L13

Therefore this error is getting triggered:
https://github.com/webpack/enhanced-resolve/blob/main/lib/util/entrypoints.js#L475

[voxpelli]

For context: Sounds like this regression is caused by #139 then

@scagood Want help to look into it?

[scagood]

Yeah, I would tend to agree. I am thinking about what the best course here is.

We could:

1. Ignore the error (but then not have a path)
2. Pr enhanced resolve to allow the error to be ignored via an option (I don't understand why it throws as opposed to just accepting the default as default)

[aladdin-add]

seems it's working as expected - as said in the node.js docs:

"default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last.

https://nodejs.org/docs/latest/api/packages.html#conditional-exports

the next steps:

1. it should report the exact error message in the rule, than just ignoring. (will be fixed by feat(import-target): Add resolution error reason #231)
2. fix @humanwhocodes/retry to put the default to last.

[voxpelli]

Note that the "types" condition should always come first in "exports".

Taken from https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-7.html#packagejson-exports-imports-and-self-also supports this

[scagood]

I think we should then close this and proceed with #182 (My first attempt to make that more verbose is here: #231)

[voxpelli]

Well, if node.js can resolve the module and the issue this plugin finds isn’t within the project that runs the linting but rather in one of its dependencies, then I think the linting shouldn’t fail as it’s kind of a false alarm?

Such a failed linting should happen in the project that published the incorrect fields instead.

[nzakas]

Well, if node.js can resolve the module and the issue this plugin finds isn’t within the project that runs the linting but rather in one of its dependencies, then I think the linting shouldn’t fail as it’s kind of a false alarm?

Such a failed linting should happen in the project that published the incorrect fields instead.

I agree with this. The current setup for the retry package works fine in Node.js so I don't think it should be considered a problem. This seems more like something that other tooling is trying to enforce rather than an actual problem with the package.

That said, I can update the package to swap the order of exports.

[scagood]

Agreed, I think I will do point two:

Pr enhanced resolve to allow the error to be ignored via an option

It may also be worth seeing if we can add a rule to eslint-plugin-package-json too specifically about this if there is not one already 🤔