-
Notifications
You must be signed in to change notification settings - Fork 87
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
Configure prettier and eslint to treat DocsShowCode as code examples #861
Comments
This seems difficult to crack (and thus pretty interesting, too!) I have no clue how to crack the same, but I would love to drop resources and links here that, according to me, might help facilitate the fix! From what I've read, one way to solve the problem might be to implement a custom plugin on top of vue-eslint-parser for the same. The AST specification might be a good starting point. We can use AST Explorer with the language set to Update 1:If we are to follow the plugin route, the following implementation nodes might help create a basic demo: Basic Plugin
module.exports = {
rules: {
'no-lint-docs-show-code': {
meta: {
type: "layout",
docs: {
description: "Disables the linting of the code examples in the documentation.",
recommended: true,
},
},
create(context) {
return context.parserServices.defineTemplateBodyVisitor({
VElement(node) {
if (node.rawName === "DocsShowCode") {
console.log(node)
}
},
});
}
}
}
}
esLintConfig.plugins.push('custom');
esLintConfig.rules['custom/no-lint-docs-show-code'] = "warn" You can use commands like Update 2:These examples' errors are due to using the Maybe we can create an ESLint plugin that can kind of "fix" these issues for us by inserting an I am enclosing a sample script if someone from the team wants to try it! Auto Insert ESLint enable and disable comments | Plugin Codemodule.exports = {
rules: {
'no-lint-docs-show-code': {
meta: {
type: 'layout',
fixable: 'code',
docs: {
description:
'Disables the linting of the code examples in the documentation.',
recommended: true,
},
},
create(context) {
const sourceCode = context.getSourceCode();
const template =
sourceCode.parserServices.getTemplateBodyTokenStore &&
sourceCode.parserServices.getTemplateBodyTokenStore();
// Finds the first comment token before/after the given node
// Breaks the search if it encounters any token of a different type
const findCommentToken = (node, searchFn, commentContent) => {
while (node) {
if (
node.type === 'HTMLComment' &&
node.value.trim() === commentContent
) {
return true;
}
if (
node.rowName === 'DocsShowCode' ||
node.type === 'HTMLWhitespace'
) {
node = searchFn(node, { includeComments: true });
} else return false;
}
return false;
};
return context.parserServices.defineTemplateBodyVisitor({
VElement(node) {
if (node.rawName === 'DocsShowCode') {
const openTag = template.getFirstToken(node);
const closeTag = template.getLastToken(node);
if (!openTag || !closeTag) {
return;
}
// There should be a "eslint-disable" comment before the opening tag
const hasPrevComment = findCommentToken(
openTag,
template.getTokenBefore,
'eslint-disable',
);
if (!hasPrevComment) {
context.report({
node,
loc: openTag.loc.start,
message:
'Code examples in the documentation should not be linted. Add an eslint-disable comment before the opening tag of the same.',
fix: (fixer) =>
fixer.insertTextBefore(
openTag,
'<!-- eslint-disable -->\n',
),
});
}
// There should be a "eslint-enable" comment after the closing tag
const hasNextComment = findCommentToken(
closeTag,
template.getTokenAfter,
'eslint-enable',
);
if (!hasNextComment) {
context.report({
node,
loc: closeTag.loc.end,
message:
'Code examples in the documentation should not be linted. Add an eslint-enable comment after the closing tag of the same.',
fix: (fixer) =>
fixer.insertTextAfter(closeTag, '\n<!-- eslint-enable -->'),
});
}
}
},
});
},
},
},
}; |
Hey @EshaanAgg! Good to hear from you again! I definitely think that a solution for this will include some AST hacking! Although keep in mind that Would be great if you can open a draft PR including the findings you've already had. And I can assign this to you if you are interested in continuing to work on this issue :). Just let us know 🤗. |
The way to do this without writing to Note that the eslint part of the problem is probably the easier - getting prettier to either ignore the text, or properly format it as code is the harder part. |
Hi @AlexVelezLl @rtibbles! It feels great to be back! I would love to work on this issue!
Even I had tried the same earlier, but yarn allows workspaces to be configured only for
In my experience, suppressing the eslint warnings is enough! (As the |
We have both private and public packages in the yarn workspace in the Kolibri repository.
Prettier runs on the entire component though - so if you don't add prettier ignore, it will reformat code in the blocks. This is why we have all the prettier-ignore comments, because otherwise the code in the DocsShowCode components get reformatted as if they were plain text. |
I have assigned you, nevertheless :) |
Ahh, that makes sense! Thanks for the clarification. As a status update, would like to mention that I have now kind of worked out the edge cases related to the adding of the But this approach has a couple of major problems:
These components are fine as they are and do not raise any prettier or eslint errors themselves! If we make such a rule, we will also blindly surround them with these comments.
Considering all these factors, I strongly feel that trying to hack at this at the plugin level seems like an uphill battle that might not be for the best (until we have a better approach than just adding these disabling comments). I saw that we are planning to rework the documentation comments a bit (as in #845), which led me to think maybe we can refactor the API of these components to address these problems. What I would like to propose is to adopt a more Vuetify-like documentation API, where all the examples live in separate folders and as independent complete Vue files (refer to one such example). Then the documentation components loads these files, separated their template, CSS and JS, (can use simple RegEx for the same as in this component) and displays them in different tabs! This approach has some obvious advantages:
The only major con of this approach I can think of is that we would need to dedicate considerable effort towards this migration! But I do believe that there would be many open source enthusiasts that would be willing to help us out in the same if we proceed with the same, so the same shouldn't be as daunting as it seems :) Do you think this can be a good way to tackle this? |
@EshaanAgg thank you for looking into this, we appreciate it. I agree that the original approach we wanted to explore here may end up causing more trouble than benefit. I think separating the examples to separate files, that lint could simply ignore, could be a way to go and as you mention it has another benefits. In the case of very large documentation pages, this would help with their maintenance. I was just actually planning to extract examples from the KCard docs to separate files. What you suggest would be a step further towards standardized way of doing it. One thing to note - as you have noticed, we don't always show the full code for each example and I think it's important to retain this behavior to prevent from clutter. So we should still be able to tweak the displayed code snippet to whatever we wish, even if it wasn't strictly Also I'd suggest to plan this work iteratively, where in the first version, we would use Let me mention this on Slack and invite more team members so we can see if we're all on the same board. If there's an agreement, would you still be interested in this work? And after we have some basis, I agree that refactoring the pages would indeed be many good first issues for the community. |
I think this provides a reasonable alternative. If your snippet is very short and it's too much overhead to put in a separate file, you can just add the ignore statements and move on. If your snippet is long and it could benefit from being properly code formatted (which will be interpreted properly by prettier), you can put it in a separate file and have it be inlined. |
@rtibbles Ah yes! If we could retain both ways that'd be great and the migration would be much easier. |
Yes, most definitely! As @rtibbles proposed, having the |
Thank you @EshaanAgg. So far I've only heard positive feedback from the team. Let me do final confirmation tomorrow during the design system circle meeting and then I think this could be ready for work. |
Hi @EshaanAgg, so everybody is on board with this plan. You're welcome to proceed with the parts I commented on in #861 (comment). Just note that |
@EshaanAgg Thanks again for thinking carefully and improving the way we do things! |
Product
KDS.
Current behavior
When we want to show code examples in the KDS docs, we use the
DocsShowCode
component. We can show HTML, JavaScript, and CSS code with this component. For example:The problem is that eslint and prettier treat the code inside DocsShowCode as HTML code, so eslint raises exceptions for linting rules and prettier formats this code as html instead of treating this as a code example. So always that we have a css or javascript code examples, we need to write
<!-- eslint-disable -->
and<!-- prettier-ignore -->
comments around it for them to avoid exceptions and unwanted formatting. So we end up with something like:Which adds unnecessary complexity to writing our code examples.
Desired behavior
We should configure prettier and eslint to at least ignore the code inside the
DocsShowCode
component as it's just a code example, and there is no need to apply linting rules and HTML formatting.At the end, we should be able to write code examples with just:
without the need to write the
<!-- eslint-disable -->
and<!-- prettier-ignore -->
comments.The Value Add
We will be able to write code examples more easily. And we won't have to worry about unwanted formatting in our code examples.
Possible Tradeoffs
If prettier just ignores the code inside, it will be the responsibility of the developer to write the code example in a well-formatted way.
Additional notes
@rtibbles tried it and it was difficult
The text was updated successfully, but these errors were encountered: