Use consistent error codes for MODULE_NOT_FOUND #360

Activity

1. guybedford commented on Jul 24, 2019

   

   guybedford

   on Jul 24, 2019

   Last edited by guybedford

   Contributor

   More actions

   Great point, agreed this should be MODULE_NOT_FOUND for both.

   Update - see below.
2. guybedford commented on Jul 31, 2019

   

   guybedford

   on Jul 31, 2019

   Contributor

   More actions

   ERR_MODULE_NOT_FOUND was actually an intentional upgrade path in the ESM resolver since all Node.js error codes start with ERR_ these days.

   If we want to change it to MODULE_NOT_FOUND though I'm fine with that too.
3. hybrist commented on Jul 31, 2019

   

   hybrist

   on Jul 31, 2019

   Last edited by hybrist

   ContributorAuthor

   More actions

   I think for the same reason that we do nodejs/node#28905, we should have one MODULE_NOT_FOUND error code for ESM and CJS. EDIT: "same reason" meaning "so people don't have to test X different codes when handling that a dependency can't be loaded because it doesn't exist (or isn't visible)".

   👍React with 👍1Reacted by Jordan Harband
4. MylesBorins commented on Nov 17, 2019

   

   MylesBorins

   on Nov 17, 2019

   Contributor

   More actions

   Is there more work to do here?
5. guybedford commented on Nov 17, 2019

   

   guybedford

   on Nov 17, 2019

   Contributor

   More actions

   The status is still:

   • CJS resolver throws MODULE_NOT_FOUND
   • ESM resolver throws ERR_MODULE_NOT_FOUND
   • configuration errors throw ERR_INVALID_PACKAGE_CONFIG for both

   As a user, you will either be calling the CJS require.resolve and have to watch for MODULE_NOT_FOUND and ERR_INVALID_PACKAGE_CONFIG, or you will be writing a loader and calling the ESM default resolve and have to watch for ERR_MODULE_NOT_FOUND or ERR_INVALID_PACKAGE_CONFIG.

   If someone feels strongly they want the new resolver to throw the same MODULE_NOT_FOUND as the old one without the ERR_ prefix now would be the time to post a PR.

   Personally I do prefer the upgrade to ERR_MODULE_NOT_FOUND as an error standardization.
6. hybrist commented on Nov 17, 2019

   

   hybrist

   on Nov 17, 2019 via email

   ContributorAuthor

   More actions

   As a user of (dynamic) import, an indirect require would still bubble up the non-ERR variant while a direct require/import would case a different error code IIRC. That does seem confusing. The same issue (a CJS module doesn’t exist) gets different error codes depending on if it’s imported directly or required from something that’s imported. That being said, I suspect in practice this won’t change much / affect too many people.

   …

   On Sun, Nov 17, 2019 at 11:28 AM Guy Bedford ***@***.***> wrote: The status is still: - CJS resolver throws MODULE_NOT_FOUND - ESM resolver throws ERR_MODULE_NOT_FOUND - configuration errors throw ERR_INVALID_PACKAGE_CONFIG for both As a user, you will either be calling the CJS require.resolve and have to watch for MODULE_NOT_FOUND and ERR_INVALID_PACKAGE_CONFIG, or you will be writing a loader and calling the ESM default resolve and have to watch for ERR_MODULE_NOT_FOUND or ERR_INVALID_PACKAGE_CONFIG. If someone feels strongly they want the new resolver to throw the same MODULE_NOT_FOUND as the old one without the ERR_ prefix now would be the time to post a PR. Personally I do prefer the upgrade to ERR_MODULE_NOT_FOUND as an error standardization. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#360?email_source=notifications&email_token=AAEKR5ATVXIIKBEKZ4ARYDLQUGLL7A5CNFSM4IGSHNZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEITZEA#issuecomment-554777744>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEKR5HA5YAPHLF6PEWMIRDQUGLL7ANCNFSM4IGSHNZQ> .
7. ljharb commented on Nov 18, 2019

   

   ljharb

   on Nov 18, 2019

   SponsorMember

   More actions

   It’ll affect everyone using resolve, and probably other resolvers as well.
8. hybrist commented on Nov 18, 2019

   

   hybrist

   on Nov 18, 2019

   Last edited by hybrist

   ContributorAuthor

   More actions

   It shouldn't affect anybody using require.resolve because it shouldn't ever encounter the import-loader ERR_ codes. The only scenario where the streams cross would be somebody writing the following:

   import('./foo.js').catch(e => {
     if (e.code === 'ERR_MODULE_NOT_FOUND') {
       // triggers if ./foo.js isn't there or if foo.js is using `import` to load other files
       // and one of them isn't there.
     }
     if (e.code === 'MODULE_NOT_FOUND') {
       // triggers if ./foo.js is there but something in the dependency graph
       // is using `require` to load other files and one of them isn't there.
     }
   });

   The reason why I'd say "not an issue" is because most of the use cases for checking the errors that I'm aware of aren't interested in indirect dependencies. I would expect a guard in the code snippet above saying "but only if this error is about foo itself". Because indirect dependencies missing files is almost always a bug, not some optional feature.
9. ljharb commented on Nov 18, 2019

   

   ljharb

   on Nov 18, 2019

   SponsorMember

   More actions

   I meant https://npmjs.com/resolve, which does look at those error codes.

   👍React with 👍1Reacted by Jan Olaf Martin