refactor: simplify no-missing-link-fragments with ESQuery selector
#495
Conversation
| const customHeadingIdPattern = /\{#(?<id>[^}\s]+)\}\s*$/u; | ||
| const htmlIdNamePattern = | ||
| /(?<!<)<(?:[^>]+)\s(?:id|name)\s*=\s*["']?([^"'\s>]+)["']?/giu; | ||
|
|
||
| /** | ||
| * Checks if the fragment is a valid GitHub line reference | ||
| * @param {string} fragment The fragment to check | ||
| * @returns {boolean} Whether the fragment is a valid GitHub line reference | ||
| */ | ||
| function isGitHubLineReference(fragment) { | ||
| return githubLineReferencePattern.test(fragment); | ||
| } | ||
|
|
||
| /** | ||
| * Extracts the text recursively from a node | ||
| * @param {Node} node The node from which to recursively extract text | ||
| * @returns {string} The extracted text | ||
| */ | ||
| function extractText(node) { | ||
| if (node.type === "html") { | ||
| return ""; | ||
| } | ||
| if ("value" in node) { | ||
| return /** @type {string} */ (node.value); | ||
| } | ||
| if ("children" in node) { | ||
| return /** @type {Node[]} */ (node.children).map(extractText).join(""); | ||
| } | ||
| return ""; | ||
| } | ||
| /(?<!<)<(?:[^>]+)\s(?:id|name)\s*=\s*["']?(?<id>[^"'\s>]+)["']?/giu; |
There was a problem hiding this comment.
I’ve only added named capture groups to improve readability.
| @@ -103,8 +76,8 @@ export default { | |||
| }, | |||
|
|
|||
| create(context) { | |||
| const { allowPattern: allowPatternString, ignoreCase } = | |||
| context.options[0]; | |||
| const [{ allowPattern: allowPatternString, ignoreCase }] = | |||
There was a problem hiding this comment.
In many other existing Markdown rules, we typically use full destructuring syntax.
| heading() { | ||
| headingText = ""; | ||
| }, | ||
|
|
||
| "heading *:not(html)"({ value }) { | ||
| headingText += value ?? ""; | ||
| }, | ||
|
|
||
| "heading:exit"() { |
There was a problem hiding this comment.
I’ve replaced the extractText helper function with an ESQuery selector logic.
| const baseId = customIdMatch | ||
| ? customIdMatch.groups.id | ||
| : headingText; |
There was a problem hiding this comment.
if (customIdMatch) {
baseId = customIdMatch[1];
} else {
const tempSlugger = new GithubSlugger();
baseId = tempSlugger.slug(rawHeadingText);
}Previously, we used GithubSlugger here again, but it wasn't necessary, so I removed the duplicate slugging.
Since the next line of code already calls slug(), this was redundant.
I've reviewed this carefully and confirmed that removing it does not cause any side effects.
| const htmlTextWithoutComments = node.value | ||
| .trim() | ||
| .replace(htmlCommentPattern, ""); |
There was a problem hiding this comment.
Just a simple cleanup here.
| if ( | ||
| allowPattern?.test(decodedFragment) || | ||
| githubLineReferencePattern.test(decodedFragment) | ||
| ) { |
There was a problem hiding this comment.
Just a simple cleanup here too.
| dedent` | ||
| # foo bar baz | ||
| # foo-bar-baz | ||
|
|
||
| [Link](#foo-bar-baz) | ||
| [Link](#foo-bar-baz-1) | ||
| `, |
There was a problem hiding this comment.
Just a simple addition of test cases to ensure https://github.com/eslint/markdown/pull/495/files#r2255966805.
…with-esquery-selector
nzakas
deleted the
refactor-simplify-no-missing-link-fragments-with-esquery-selector
branch
github-project-automation
bot
moved this from Second Review Needed
to Complete
in Triage
Prerequisites checklist
What is the purpose of this pull request?
Hi,
In this PR, I've refactored the
no-missing-link-fragmentsrule, thanks to #469 (review).Previously, we used a separate traversing logic with the
extractTexthelper function. This was somewhat redundant, since ESLint already traverses the AST for us, resulting in duplicate traversing.I've updated the code to use an ESQuery selector instead, which simplifies the traversing logic.
I've also made some additional cleanups to make the code more consistent and easier to read.
What changes did you make? (Give an overview)
In this PR, I've refactored the
no-missing-link-fragmentsrule.Related Issues
Ref: #469 (review)
Is there anything you'd like reviewers to focus on?
I've made sure all existing test cases are passing.