| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix: correct ESLint typings
#20122
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: correct ESLint typings
#20122
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| calculateConfigForFile(filePath: string): Promise<any>; | ||
|
|
||
| findConfigFile(): Promise<string | undefined>; | ||
| findConfigFile(filePath?: string): Promise<string | undefined>; |
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.
This method appears to be undocumented. Not sure if this was an oversight or if it was intentionally left out of the public API.
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.
First nice catch about the missing parameter.
Do you mean with undocumented that the method is missing from the documentation page?
The types should reflect the public API so it seems to be missing on the documentation page.
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.
Do you mean with undocumented that the method is missing from the documentation page?
Yes, exactly.
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.
This is a public method. It was proposed in #17046 following other discussions. The optional argument was added later as per the design in this document: https://github.com/eslint/rfcs/tree/main/designs/2024-config-lookup-from-file#core-changes. As it seems, the types were not updated.
So yes, this method should be included in the types. The lack of documentation is clearly our fault.
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.
Got it, thanks for clarifying. I opened #20130 to track this.
fasttime
added
the
accepted
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.
LGTM, thanks! Leaving open for @DMartens to verify before merging.
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.
LGTM, thanks!
|
@DMartens can you verify your concerns were addressed? |
|
Oh sorry I missed the commit addressing the concern. |
github-project-automation
bot
moved this from Second Review Needed
to Complete
in Triage
Labels
5 participants
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)
ESLinttypingsIs there anything you'd like reviewers to focus on?