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.

[scagood]

The think that is in in the back of my head is related to this:

This condition should always come last.

I interpret this as should always be checked last, not should always appear last in the object's properties

I do know that as of es2020 property ordering is expected (eg in for-in). But before that I don't think there was any guarantee of order 🤔

[aladdin-add]

maybe (I'm not a native English speaker). but it's also saying:

Within the "exports" object, key order is significant. During condition matching, earlier entries have higher priority and take precedence over later entries. The general rule is that conditions should be from most specific to least specific in object order.

generally, it should be a best practice to put it in the last.

[scagood]

Fair point! Notes for next time include to read the source! 😅

[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)