Create unified types for nodes (#2201)

Create unified types for nodes There is no standardised type for node variables, resulting in several undesirable issues: * `as unknown as` typecasts throughout the codebase * unnecessary checks on node attributes to satisfy the linter * incompatible types, treated as interchangeable in conversions Let's add common MbNode, TextElement and NodeOrText types that enforce attributes for nodes throughout the codebase. Doing so avoids the abovementioned issues, and formalises our view of the relationship between cheerio.Element, DomElement, and all the other attributes expected from node variables.
MarkBind · Mar 17, 2023 · 3ca03b5 · 3ca03b5
1 parent 2cebe9e
commit 3ca03b5
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 72 deletions.
diff --git a/packages/core/src/html/MdAttributeRenderer.ts b/packages/core/src/html/MdAttributeRenderer.ts
@@ -5,6 +5,7 @@ import { getVslotShorthandName } from './vueSlotSyntaxProcessor';
 import type { MarkdownProcessor } from './MarkdownProcessor';
 import * as logger from '../utils/logger';
 import { createSlotTemplateNode } from './elements';
+import { NodeOrText } from '../utils/node';
 
 const _ = {
   has,
@@ -44,7 +45,7 @@ export class MdAttributeRenderer {
         rendered = this.markdownProcessor.renderMd(node.attribs[attribute]);
       }
 
-      const attributeSlotElement = createSlotTemplateNode(slotName, rendered);
+      const attributeSlotElement: NodeOrText[] = createSlotTemplateNode(slotName, rendered);
       node.children = node.children
         ? attributeSlotElement.concat(node.children)
         : attributeSlotElement;

diff --git a/packages/core/src/html/NodeProcessor.ts b/packages/core/src/html/NodeProcessor.ts
@@ -26,6 +26,7 @@ import { PageNavProcessor, renderSiteNav, addSitePageNavPortal } from './siteAnd
 import { highlightCodeBlock, setCodeLineNumbers } from './codeblockProcessor';
 import { setHeadingId, assignPanelId } from './headerProcessor';
 import { FootnoteProcessor } from './FootnoteProcessor';
+import { MbNode, NodeOrText, TextElement } from '../utils/node';
 
 const fm = require('fastmatter');
 
@@ -87,7 +88,9 @@ export class NodeProcessor {
    * Private utility functions
    */
 
-  static _trimNodes(node: DomElement) {
+  static _trimNodes(nodeOrText: NodeOrText) {
+    if (NodeProcessor._isText(nodeOrText)) return;
+    const node = nodeOrText as MbNode;
     if (node.name === 'pre' || node.name === 'code') {
       return;
     }
@@ -105,14 +108,14 @@ export class NodeProcessor {
     }
   }
 
-  static _isText(node: DomElement) {
+  static _isText(node: NodeOrText) {
     return node.type === 'text' || node.type === 'comment';
   }
 
   /*
    * Frontmatter collection
    */
-  _processFrontmatter(node: DomElement, context: Context) {
+  _processFrontmatter(node: MbNode, context: Context) {
     let currentFrontmatter = {};
     const frontmatter = cheerio(node);
     if (!context.processingOptions.omitFrontmatter && frontmatter.text().trim()) {
@@ -121,8 +124,8 @@ export class NodeProcessor {
       // The latter case will result in the data being wrapped in a div
       const frontmatterIncludeDiv = frontmatter.find('div');
       const frontmatterData = frontmatterIncludeDiv.length
-        ? ((frontmatterIncludeDiv[0] as DomElement).children as DomElement[])[0].data
-        : ((frontmatter[0] as DomElement).children as DomElement[])[0].data;
+        ? ((frontmatterIncludeDiv[0] as MbNode).children as MbNode[])[0].data
+        : ((frontmatter[0] as MbNode).children as MbNode[])[0].data;
       const frontmatterWrapped = `${FRONTMATTER_FENCE}\n${frontmatterData}\n${FRONTMATTER_FENCE}`;
 
       currentFrontmatter = fm(frontmatterWrapped).attributes;
@@ -139,7 +142,7 @@ export class NodeProcessor {
    * Layout element collection
    */
 
-  private static collectLayoutEl(node: DomElement): string | null {
+  private static collectLayoutEl(node: MbNode): string | null {
     const $ = cheerio(node);
     const html = $.html();
     $.remove();
@@ -149,7 +152,7 @@ export class NodeProcessor {
   /**
    * Removes the node if modal id already exists, processes node otherwise
    */
-  private processModal(node: DomElement) {
+  private processModal(node: MbNode) {
     if (node.attribs) {
       if (this.processedModals[node.attribs.id]) {
         cheerio(node).remove();
@@ -168,11 +171,10 @@ export class NodeProcessor {
   /*
    * API
    */
-  processNode(node: DomElement, context: Context): Context {
+  processNode(nodeOrText: NodeOrText, context: Context): Context {
     try {
-      if (!node.name || !node.attribs) {
-        return context;
-      }
+      if (NodeProcessor._isText(nodeOrText)) return context;
+      const node = nodeOrText as MbNode;
 
       transformOldSlotSyntax(node);
       shiftSlotNodeDeeper(node);
@@ -274,7 +276,10 @@ export class NodeProcessor {
     return context;
   }
 
-  postProcessNode(node: DomElement) {
+  postProcessNode(nodeOrText: NodeOrText) {
+    if (NodeProcessor._isText(nodeOrText)) return;
+    const node = nodeOrText as MbNode;
+
     try {
       switch (node.name) {
       case 'pre':
@@ -316,13 +321,12 @@ export class NodeProcessor {
     }
   }
 
-  private traverse(node: DomElement, context: Context): DomElement {
-    if (NodeProcessor._isText(node)) {
-      return node;
-    }
-    if (node.name) {
-      node.name = node.name.toLowerCase();
+  private traverse(dom: DomElement, context: Context): NodeOrText {
+    if (NodeProcessor._isText(dom)) {
+      return dom as TextElement;
     }
+    const node = dom as MbNode;
+    node.name = node.name.toLowerCase();
     if (linkProcessor.hasTagLink(node)) {
       linkProcessor.convertRelativeLinks(node, context.cwf, this.config.rootPath, this.config.baseUrl);
       linkProcessor.convertMdExtToHtmlExt(node);
@@ -363,16 +367,14 @@ export class NodeProcessor {
 
     addSitePageNavPortal(node);
 
-    if (node.name) {
-      const isHeadingTag = (/^h[1-6]$/).test(node.name);
-      if (isHeadingTag && !node.attribs?.id) {
-        setHeadingId(node, this.config);
-      }
+    const isHeadingTag = (/^h[1-6]$/).test(node.name);
+    if (isHeadingTag && !node.attribs.id) {
+      setHeadingId(node, this.config);
+    }
 
-      // Generate dummy spans as anchor points for header[sticky]
-      if (isHeadingTag && node.attribs?.id) {
-        cheerio(node).prepend(`<span id="${node.attribs.id}" class="anchor"></span>`);
-      }
+    // Generate dummy spans as anchor points for header[sticky]
+    if (isHeadingTag && node.attribs.id) {
+      cheerio(node).prepend(`<span id="${node.attribs.id}" class="anchor"></span>`);
     }
 
     this.pluginManager.postProcessNode(node);
@@ -407,7 +409,7 @@ export class NodeProcessor {
         });
         mainHtmlNodes.forEach(d => NodeProcessor._trimNodes(d));
 
-        const footnotesHtml = this.footnoteProcessor.combineFootnotes((node: DomElement) => this.processNode(
+        const footnotesHtml = this.footnoteProcessor.combineFootnotes(node => this.processNode(
           node, new Context(cwf, [], extraVariables, {}),
         ));
         const mainHtml = cheerio(mainHtmlNodes).html();

diff --git a/packages/core/src/html/elements.ts b/packages/core/src/html/elements.ts
@@ -1,22 +1,22 @@
 import cheerio from 'cheerio';
-import { DomElement } from 'htmlparser2';
 import pick from 'lodash/pick';
+import { MbNode, NodeOrText } from '../utils/node';
 
 const _ = { pick };
 
-export function createErrorNode(element: DomElement, error: any) {
+export function createErrorNode(element: NodeOrText, error: any) {
   const errorElement = cheerio.parseHTML(
     `<div style="color: red">${error.message}</div>`, undefined, true,
   )[0];
-  return Object.assign(element, _.pick(errorElement, ['name', 'attribs', 'children']));
+  return Object.assign(element, _.pick(errorElement, ['name', 'attribs', 'children'])) as MbNode;
 }
 
 export function createEmptyNode() {
   return cheerio.parseHTML('<div></div>', undefined, true)[0];
 }
 
-export function createSlotTemplateNode(slotName: string, content: string): DomElement[] {
+export function createSlotTemplateNode(slotName: string, content: string): MbNode[] {
   return cheerio.parseHTML(
     `<template #${slotName}>${content}</template>`, undefined, true,
-  ) as unknown as DomElement[];
+  ) as unknown as MbNode[];
 }