New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Ensure FlatESLint#findConfigFile() doesn't throw. #17151
Conversation
If findConfigFile() can't find eslint.config.js, it now returns undefined instead of throwing an error. Fixes #17150
✅ Deploy Preview for docs-eslint canceled.
|
* the config file. | ||
*/ | ||
async function locateConfigFileToUse({ configFile, cwd }) { | ||
|
||
// determine where to load config file from | ||
let configFilePath; | ||
let basePath = cwd; | ||
let error = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name error
sounds generic and I expected it to be an instance of Error
. Can we use something like isConfigFileDetected
/hasConfigFile
? I feel it's better suited for a boolean value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can return an error object error = new Error("Could not find config file.");
, which can be thrown directly where it is used?
const { configFilePath, basePath, error } = await locateConfigFileToUse({ configFile, cwd });
// config file is required to calculate config
if (error) {
throw error;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aladdin-add I like that suggestion. Can you open a PR with that change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, will do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well! If the above discussion is not resolved before today's release, I'll merge this as is and we can consider refactoring in a follow up.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
If findConfigFile() can't find eslint.config.js, it now returns undefined instead of throwing an error.
Fixes #17150
Is there anything you'd like reviewers to focus on?
Did I miss any edge cases?