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
feat(experimental-utils): make RuleMetaData.docs optional #1462
feat(experimental-utils): make RuleMetaData.docs optional #1462
Conversation
Thanks for the PR, @MrCrunchwrap! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
Hey there, making it required was an intentional decision to attempt to provide a form of best practice. Similarly, providing How come you want to not provide it? |
@bradzacher Gotcha, makes sense. In my case, we build custom eslint rules for our app that are only used internally, so there's really no point in having a docs property on them. While it's best practice to have docs when creating a rule to share with the community, it's explicitly stated in eslint's documentation that docs is an optional (specifically when creating custom rules/plugins) so I don't think the typing is accurate currently. Perhaps the JSDoc comment for that property could encourage usage? I'm mostly just trying to find a way to use these awesome typings for the custom rules I have written without doing a lot of type asserting, etc 😄 |
@bradzacher its common to not setup docs in rules in a lot of companies (that my experience), usually i'm filling them up to get better IDE support, but not everyone does that, and as eslint docs states, it should not be required. bdw. we already have kind of validation for this in tests. i don't have strong opinion on this, i feel like we should wait what community thinks about this change. |
Nitpicking, but technically you don't even need to export an object, you can just export a function:
If things aren't required, people usually ignore it. Only the dedicated few usually dig in to interrogate all of the optional properties. I'm happy have this change, I just prefer to question first so I can understand motivations. If you can fix the codebase so the CI passes, I'm okay with merging this. I was thinking we should create a local "override" of the rule creator config anyways. There's the two options that we've added ( If we have a local override, we could tweak the types to keep our plugin as strict as we like without worrying as much about the external 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.
LGTM - thanks for this
This change makes it so that the
RuleMetaData
interface hasdocs
as an optional property.docs
should not be required according to the eslint documentation.From the eslint docs:
"In a custom rule or plugin, you can omit docs or include any properties that you need in it."