Adding anchor links to <dt> should not modify Markdown engine

[wbamberg]

Summary

#8413 added anchor links to <dt> elements, which is nice. But it did it inside the Markdown engine, which makes the Markdown output less clean. Ideally the Markdown engine should just be as close to standard GFM as we can get, and Yari should add extra features like this on top of the HTML output.

(For example, we also add anchor links to headings in Yari, but we don't do it by modifying the Markdown engine.)

See also the discussion in #8413 (comment).

URL

Any MDN page that includes a <dl>, such as https://developer.mozilla.org/en-US/docs/Web/CSS/margin.

Reproduction steps

Have Yari build a page from Markdown.

Expected behavior

Markdown engine should just generate plain <dl> structures from the <dl> syntax we have invented, and any additions to that should be done on top of that output.

Actual behavior

Markdown engine generates <dl> structures that include anchors.

Device

Desktop

Browser

Chrome

Browser version

Stable

Operating system

Mac OS

Screenshot

No response

Anything else?

No response

Validations

• I have read the Community Participation Guidelines.
• I have verified that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• I have checked that this is a concrete bug. For Q&A open a GitHub Discussion.

[KartikSoneji]

Looks like this is enough to add hyperlinks to <dt> elements, with 0 modifications to the markdown engine.
It looks like it produces equivalent output on the subset of files I tested but ideally the current version and this would be tested on a full build of the site, if they're identical then we can switch to this version.

diff --git a/kumascript/src/api/util.ts b/kumascript/src/api/util.ts
index 0e7f0c2e9..6cda399a0 100644
--- a/kumascript/src/api/util.ts
+++ b/kumascript/src/api/util.ts
@@ -223,16 +223,8 @@ export class HTMLTool {
       knownIDs.add(id);
       $element.attr("id", id);
 
-      if (isDt) {
-        // Remove empty anchor links.
-        // This happens if the term already links to a page.
-        $element.find("a[data-link-to-id = true]:empty").remove();
-
-        // Link remaining anchor links to the term's ID.
-        $element
-          .find("a[data-link-to-id = true]")
-          .attr("href", "#" + id)
-          .removeAttr("data-link-to-id");
+      if (isDt && $element.is("a:not([href])")) {
+        $element.attr("href", "#" + id);
       }
     });
     return this;
diff --git a/markdown/m2h/handlers/dl.ts b/markdown/m2h/handlers/dl.ts
index 6aca68c03..252356a1d 100644
--- a/markdown/m2h/handlers/dl.ts
+++ b/markdown/m2h/handlers/dl.ts
@@ -38,21 +38,19 @@ export function asDefinitionList(h, node) {
       DEFINITION_PREFIX.length
     );
 
-    const [firstDtChild, ...dtChildren] = all(h, {
-      ...node,
-      children:
-        terms.length == 1 && terms[0].type == "paragraph"
-          ? terms[0].children
-          : terms,
-    });
-    if (firstDtChild) {
-      dtChildren.unshift(
-        h(node, "a", { "data-link-to-id": "true" }, [firstDtChild])
-      );
-    }
-
     return [
-      h(node, "dt", {}, dtChildren),
+      h(
+        node,
+        "dt",
+        {},
+        all(h, {
+          ...node,
+          children:
+            terms.length == 1 && terms[0].type == "paragraph"
+              ? terms[0].children
+              : terms,
+        })
+      ),
       h(
         node,
         "dd",

[caugner]

@KartikSoneji Would you mind opening a PR for these changes? 🙏

[KartikSoneji]

@caugner I would be happy to, but can someone please confirm if at least the en-us repo has identical outputs with the current and new version? I wouldn't want to make a hasty change that results in a regression.
Last time I tried a full build it timed out GitPod, or I can try on my local machine overnight.

[caugner]

@KartikSoneji Can you open the PR? Then I can easily run two full builds locally and compare.

[KartikSoneji]

@caugner done: #9862