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
Proposal: Add URL for a rule's documentation to the API #6582
Comments
It'd be good to discuss the details a little. For example, first question I have is why not have the URLs collected in a plugin definition file (same as where plugin rules and configurations are exported)? This also allows the same file to contain a URL for overall documentation, if needed. Proposal for that API is here: #6525 |
This can probably be done very easily through rule's metadata. No code changes necessary. Although we do need an API to retrieve metadata. |
Kevin, what you describe is what I have in mind. I thought about putting the URL in [rule] metadata, but that would become very redundant. Rather define one function at the plugin level. The concept is more or less proven by AtomLinter. We'd just be moving the knowledge of URIs from AtomLinter to the plugin itself, and expose it through an API that AtomLinter and any other client can consume.
|
I've updated the issue to clarify exactly what API additions I'm proposing. This would be a new feature without any breaking changes. |
Why would you say putting the URL in the metadata property would be redundant? This seems like the perfect use case for a metadata property. |
I guess I'm not attached one way or the other. I agree it would be great to use some sort of metadata property (either at plugin or rule level) to provide documentation links. @ilyavolodin Perhaps you want to take a look at the first post of this issue, since it has been updated with a concrete proposal? |
I'm in favor of configuring the URI at the plugin level because it saves plugin authors from having to include the URI in every single rule. |
First stab at a PR. Feedback much appreciated. |
I vote to a use of |
I understand redundancy argument. However, I feel like "one URL to rule them all" is too inflexible. For example, there might be plugin authors, that don't want to name all of their pages with the same name as rule name. Or they might use some provider that doesn't, for example, support dashes in the urls, but their rules have dashes. One other thing to keep in mind, most of the editor addons that might/would use this functionality don't know anything about plugins. So when the error occurs, they would need to parse rule name, extract plugin name out of it, then look up rule URI for it in the API. Vs. just calling API and comparing rule name against the list. No parsing required. And last thing is, this proposed solution would not work for custom rules, just for plugins. Metadata would work for both. |
Thanks, @ilyavolodin. Good thoughts. I don't think editors (and their addons) have to deal wirth ESLint plugins. It's all transparent. A consumer of the Node API shouldn't need to do any parsing. It gets the URI for a reported problem and can do what it wants with it. Anyway, how the URI is exposed in the Node API is orthaganol to whether the URI is configured by a plugin-level function or rule-level metadata. AtomLinter has already proven the concept of using a function to map a ruleId to a URI. All of the functions that have been registered so far have been trivial one-liners. Your point about not working for custom rules is valid. However I don't imagine there will be a huge demand for this feature to be used in one-off custom rules. How many people go to the trouble of publishing docs for project-specific rules? And if they do, what's stopping them from moving the rules to a plugin that can be shared across multiple projects? Also there's an advantage to generating a URI on the fly. In the future, the getDocURI() function could take the problem object as an optional second parameter. That would allow a plugin to generate a URI that points to "smart" documentation, which is tailored to use your actual code snippet in the explanation. |
|
@pmcelhaney, do you mean you are proposing "AtomLinter plugins to specify where to find their docs"? I did help get the ESLint docs working. Being out of the loop on the conversation a bit, I think that @ilyavolodin is tending towards a solution I would support. A blanket "base URL" is probably not a flexible solution. Meta-data is probably better. In my opinion, it would be easier to create a build script on the plugins to generate the full URLs for each rule, than it would be to rely on each plugin author to follow a more complex pattern. Just my two cents. Great idea. I've been thinking this should be done for some time now. I'm happy to help in anyway I can. Keep me in the loop. I'm going to loop in @Arcanemagus who helped me implement my code. He's a pretty smart dude who may be able to help us come to a conclusion. |
I agree with @ilyavolodin that this information really belongs in On a larger point: all of ESLint is meant so that rules are rules are rules -- we try very hard not to treat rules from the core different from custom rules or different from plugin rules. Any solution we choose must keep that design principle in mind even if it leads to some redundancies. |
Okay, thanks. I'll send another PR that puts the config in meta. |
@pmcelhaney it would be helpful (and arguably a better use of your time) to propose a design here for review before submitting a new PR. |
Thanks @nzakas. Unfortunately by the time I saw your comment I already submitted a new PR. As yourself and others have suggested, I simply exposed the rule metadata through the API (as part of the problem object) and documented (Aside: I know you've expressed frustration about people reporting bugs / requesting features and not being respectful of the fact that you're all volunteers. Submitting PRs is my way of showing that I'm here to contribute, not just to pester you and expect something for nothing.) |
@pmcelhaney and we appreciate the PRs, we're just trying to be respectful of your time by encouraging you to describe what you're thinking of being digging into the code. As this is a change to the core, we have to be more careful about evaluating and accepting it. |
From my point of view, the clearest and most efficient way to express my thinking is to just send a PR. Not necessarily a complete PR, but one that hits the key tests, doc updates, and code changes. In this case, the change was relatively small so it was just as easy to do the whole thing (sans adding meta.doc.uri to all of the built-in rules). Anyway, if outlining the proposed design in prose works better for you, I'll keep that in mind going forward.
|
Yay! No more undocumented hack needed! |
Code mod: export default function transformer(file, api) { const j = api.jscodeshift; const name = file.path.split('/').pop().replace(/.js$/, ''); return j(file.source) .find(j.ObjectExpression) .filter(path => path.parentPath.node.key && path.parentPath.node.key.name === 'docs') .forEach(path => { const prop = j.property('init', j.identifier('url'), j.literal(`https://eslint.org/docs/rules/${name}`)); j(path).replaceWith(j.objectExpression([...path.node.properties, prop])); }) .toSource({lineTerminator: '\n'}); }
Code mod: export default function transformer(file, api) { const j = api.jscodeshift; const name = file.path.split('/').pop().replace(/.js$/, ''); return j(file.source) .find(j.ObjectExpression) .filter(path => path.parentPath.node.key && path.parentPath.node.key.name === 'docs') .forEach(path => { const prop = j.property('init', j.identifier('url'), j.literal(`https://eslint.org/docs/rules/${name}`)); j(path).replaceWith(j.objectExpression([...path.node.properties, prop])); }) .toSource({lineTerminator: '\n'}); }
Add a lint rule to ensure that each rule has a meta.docs.url property with the correct value.
Add a lint rule to ensure that each rule has a meta.docs.url property with the correct value.
* Docs: add url to each of the rules (refs #6582) Code mod: export default function transformer(file, api) { const j = api.jscodeshift; const name = file.path.split('/').pop().replace(/.js$/, ''); return j(file.source) .find(j.ObjectExpression) .filter(path => path.parentPath.node.key && path.parentPath.node.key.name === 'docs') .forEach(path => { const prop = j.property('init', j.identifier('url'), j.literal(`https://eslint.org/docs/rules/${name}`)); j(path).replaceWith(j.objectExpression([...path.node.properties, prop])); }) .toSource({lineTerminator: '\n'}); } * New: internal-rules/consistent-docs-url (refs #6582) Add a lint rule to ensure that each rule has a meta.docs.url property with the correct value.
I think this issue can be closed now that we've added |
The version of ESLint you are using.
4.12.1
The problem you want to solve
AtomLinter has a neat feature that links to the documentation for each rule.
In order for that link to work in plugins, the plugin needs to be registered in AtomLinter.
Your take on the correct solution to problem
Instead of AtomLinter having to maintain a list of known plugins (and other clients having to duplicate the effort) it would be great if the API could allow a rule to specify where its documentation can be found.
It boils down to:
Original proposal which worked at the plugin level rather than the rule level.
The URL would be exposed to the Node API via the response from the
verify()
method.I've implemented the change in ESLint. I'm waiting for the proposal to be accepted so I can open a PR. If it's accepted I'm happy to do the corresponding update for AtomLinter as well.
The text was updated successfully, but these errors were encountered: