Create Module not found error #4826
Conversation
| `${filepath} is attempting to import a module that does not exist: ${err}`, | ||
| filepath | ||
| ); | ||
| } |
There was a problem hiding this comment.
It's not clear to me how you are going to distinguish module not found errors caused by invalid path (see parsers) and by invalid imports.
| * @param {string} importedFilename - File name of imported module | ||
| * @returns {Error} Error with code {@link constants.MODULE_NOT_FOUND} | ||
| */ | ||
| function createModuleNotFoundError( |
There was a problem hiding this comment.
We don't need a new Mocha error code, we can't wrap each and every Node error in a new Mocha error object. We just have to rethrow Node's error somehow or include Node's error message in Mocha's UnparsableFileError object.
| @@ -5,7 +5,6 @@ | |||
| "requires": true, | |||
| "packages": { | |||
| "": { | |||
| "name": "mocha", | |||
There was a problem hiding this comment.
This is not correct here. There shouldn't be any package-lock.json entry in this PR.
|
It's not the first part which is wrong, but the second error message. It should be: |
|
superseded by #4832 |
Requirements
Description of the Change
In this PR, we throw the "Module Not Found" error when a non-existent module is being imported in the config file. Before this was introduced, an unhelpful message of "Unparsable File Error" is being thrown, which may confuse the users. This aims to resolve issue #4781
Alternate Designs
The alternative would be to ignore the "Module Not Found" error, and just display these errors as "Unparsable File". But "Module Not Found" should not be under "Unparsable File" since "Unparsable File" implies that there might be a syntax error somewhere in the program.
Why should this be in core?
This should be in the core because one of the main functionalities of Mocha is to show the users the correct error messages.
Benefits
A more helpful error message will be shown to users of Mocha, which leads to less confusion when using this package.
Possible Drawbacks
There are no drawbacks to this change, unless maybe if some old users are not used to seeing this new error message.
Applicable issues
Not a breaking change
This is an enhancement.
It should not impact production code.
Other notes: Hi, I am a new contributor :) Thank you very much for reviewing!