From 5edcc2f14e1f106167d0820a68260c7d300f50ed Mon Sep 17 00:00:00 2001 From: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Tue, 21 Mar 2023 09:46:16 +0800 Subject: [PATCH] Refactor core package to use Node types (#2221) Refactor core package to use Node types In #2201, we introduced a unifying type for nodes. Most of the core package is not yet using the new types. Let's refactor the core package to utilise these new types, and add a utility parseHTML method that avoids further `as unknown as` typecasts throughout the codebase. Doing so allows us to make full use of TypeScript's type-checking abilities while avoiding code duplication. For more information on the rationale of the unifying types, refer to #2201. --- packages/core/src/html/FootnoteProcessor.ts | 16 +++--- packages/core/src/html/MdAttributeRenderer.ts | 37 ++++++------- packages/core/src/html/NodeProcessor.ts | 22 ++++---- packages/core/src/html/SiteLinkManager.ts | 14 ++--- packages/core/src/html/codeblockProcessor.ts | 12 ++-- packages/core/src/html/headerProcessor.ts | 14 ++--- packages/core/src/html/linkProcessor.ts | 55 +++++++++---------- .../core/src/html/siteAndPageNavProcessor.ts | 39 +++++-------- .../core/src/html/vueSlotSyntaxProcessor.ts | 20 +++---- packages/core/src/html/warnings.ts | 14 ++--- packages/core/src/plugins/Plugin.ts | 10 ++-- packages/core/src/plugins/PluginManager.ts | 6 +- packages/core/src/utils/node.ts | 9 +++ .../core/src/variables/VariableProcessor.ts | 10 ++-- .../core/test/unit/html/NodeProcessor.test.ts | 13 +++-- .../test/unit/html/SiteLinkManager.test.ts | 15 +++-- .../core/test/unit/html/linkProcessor.test.ts | 42 +++++++------- 17 files changed, 163 insertions(+), 185 deletions(-) diff --git a/packages/core/src/html/FootnoteProcessor.ts b/packages/core/src/html/FootnoteProcessor.ts index 09dc6b9663..8fded4a3d6 100644 --- a/packages/core/src/html/FootnoteProcessor.ts +++ b/packages/core/src/html/FootnoteProcessor.ts @@ -1,5 +1,5 @@ import cheerio from 'cheerio'; -import { DomElement } from 'htmlparser2'; +import { MbNode, parseHTML } from '../utils/node'; import { MARKBIND_FOOTNOTE_POPOVER_ID_PREFIX } from './constants'; /* @@ -13,7 +13,7 @@ export class FootnoteProcessor { this.renderedFootnotes = []; } - processMbTempFootnotes(node: DomElement) { + processMbTempFootnotes(node: MbNode) { const $ = cheerio(node); const content = $.html(); if (content) { @@ -22,7 +22,7 @@ export class FootnoteProcessor { $.remove(); } - combineFootnotes(processNode: (nd: DomElement) => any): string { + combineFootnotes(processNode: (nd: MbNode) => any): string { let hasFootnote = false; const prefix = '
\n
\n
    \n'; @@ -30,17 +30,17 @@ export class FootnoteProcessor { const $ = cheerio.load(footNoteBlock); let popoversHtml = ''; - $('li.footnote-item').each((index, li) => { + $('li.footnote-item').each((_index, li) => { hasFootnote = true; const popoverId = `${MARKBIND_FOOTNOTE_POPOVER_ID_PREFIX}${(li as any).attribs.id}`; - const popoverNode = cheerio.parseHTML(` + const popoverNode = parseHTML(`
    ${$(li).html()}
    -
    `)[0]; - processNode(popoverNode as unknown as DomElement); +
    `)[0] as MbNode; + processNode(popoverNode); - popoversHtml += cheerio.html(popoverNode as any); + popoversHtml += cheerio.html(popoverNode); }); return `${popoversHtml}\n${footNoteBlock}\n`; diff --git a/packages/core/src/html/MdAttributeRenderer.ts b/packages/core/src/html/MdAttributeRenderer.ts index 0a0da8ef1e..656902db2f 100644 --- a/packages/core/src/html/MdAttributeRenderer.ts +++ b/packages/core/src/html/MdAttributeRenderer.ts @@ -1,11 +1,9 @@ -import cheerio from 'cheerio'; import has from 'lodash/has'; -import { DomElement } from 'htmlparser2'; import { getVslotShorthandName } from './vueSlotSyntaxProcessor'; import type { MarkdownProcessor } from './MarkdownProcessor'; import * as logger from '../utils/logger'; import { createSlotTemplateNode } from './elements'; -import { NodeOrText } from '../utils/node'; +import { MbNode, NodeOrText, parseHTML } from '../utils/node'; const _ = { has, @@ -29,11 +27,8 @@ export class MdAttributeRenderer { * @param isInline Whether to process the attribute with only inline markdown-it rules * @param slotName Name attribute of the element to insert, which defaults to the attribute name */ - processAttributeWithoutOverride(node: DomElement, attribute: string, + processAttributeWithoutOverride(node: MbNode, attribute: string, isInline: boolean, slotName = attribute): void { - if (!node.attribs) { - return; - } const hasAttributeSlot = node.children && node.children.some(child => getVslotShorthandName(child) === slotName); @@ -63,7 +58,7 @@ export class MdAttributeRenderer { * @returns {boolean} whether the node has both the slot and attribute */ // eslint-disable-next-line class-methods-use-this - hasSlotOverridingAttribute(node: DomElement, attribute: string, slotName = attribute): boolean { + hasSlotOverridingAttribute(node: MbNode, attribute: string, slotName = attribute): boolean { const hasNamedSlot = node.children && node.children.some(child => getVslotShorthandName(child) === slotName); if (!hasNamedSlot || !node.attribs) { @@ -80,7 +75,7 @@ export class MdAttributeRenderer { return hasAttribute; } - processPopoverAttributes(node: DomElement) { + processPopoverAttributes(node: MbNode) { if (!this.hasSlotOverridingAttribute(node, 'header')) { this.processAttributeWithoutOverride(node, 'header', true); } @@ -103,11 +98,11 @@ export class MdAttributeRenderer { this.processAttributeWithoutOverride(node, 'content', true); } - processTooltip(node: DomElement) { + processTooltip(node: MbNode) { this.processAttributeWithoutOverride(node, 'content', true); } - processModalAttributes(node: DomElement) { + processModalAttributes(node: MbNode) { if (!this.hasSlotOverridingAttribute(node, 'header')) { this.processAttributeWithoutOverride(node, 'header', true); } @@ -117,7 +112,7 @@ export class MdAttributeRenderer { * Panels */ - processPanelAttributes(node: DomElement) { + processPanelAttributes(node: MbNode) { this.processAttributeWithoutOverride(node, 'alt', false, '_alt'); if (!this.hasSlotOverridingAttribute(node, 'header')) { this.processAttributeWithoutOverride(node, 'header', false); @@ -128,17 +123,17 @@ export class MdAttributeRenderer { * Questions, QOption, and Quizzes */ - processQuestion(node: DomElement) { + processQuestion(node: MbNode) { this.processAttributeWithoutOverride(node, 'header', false); this.processAttributeWithoutOverride(node, 'hint', false); this.processAttributeWithoutOverride(node, 'answer', false); } - processQOption(node: DomElement) { + processQOption(node: MbNode) { this.processAttributeWithoutOverride(node, 'reason', false); } - processQuiz(node: DomElement) { + processQuiz(node: MbNode) { this.processAttributeWithoutOverride(node, 'intro', false); } @@ -146,7 +141,7 @@ export class MdAttributeRenderer { * Tabs */ - processTabAttributes(node: DomElement) { + processTabAttributes(node: MbNode) { this.processAttributeWithoutOverride(node, 'header', true); } @@ -154,7 +149,7 @@ export class MdAttributeRenderer { * Boxes */ - processBoxAttributes(node: DomElement) { + processBoxAttributes(node: MbNode) { this.processAttributeWithoutOverride(node, 'icon', true); this.processAttributeWithoutOverride(node, 'header', false); } @@ -163,7 +158,7 @@ export class MdAttributeRenderer { * Dropdowns */ - processDropdownAttributes(node: DomElement) { + processDropdownAttributes(node: MbNode) { if (!this.hasSlotOverridingAttribute(node, 'header')) { this.processAttributeWithoutOverride(node, 'header', true); } @@ -173,7 +168,7 @@ export class MdAttributeRenderer { * Thumbnails */ - processThumbnailAttributes(node: DomElement) { + processThumbnailAttributes(node: MbNode) { if (!node.attribs) { return; } @@ -189,11 +184,11 @@ export class MdAttributeRenderer { } const renderedText = this.markdownProcessor.renderMdInline(text); - node.children = cheerio.parseHTML(renderedText) as unknown as DomElement[]; + node.children = parseHTML(renderedText); delete node.attribs.text; } - processScrollTopButtonAttributes(node: DomElement) { + processScrollTopButtonAttributes(node: MbNode) { this.processAttributeWithoutOverride(node, 'icon', true); } } diff --git a/packages/core/src/html/NodeProcessor.ts b/packages/core/src/html/NodeProcessor.ts index 885294a731..bd842f0800 100644 --- a/packages/core/src/html/NodeProcessor.ts +++ b/packages/core/src/html/NodeProcessor.ts @@ -115,7 +115,7 @@ export class NodeProcessor { /* * Frontmatter collection */ - _processFrontmatter(node: MbNode, context: Context) { + private _processFrontmatter(node: MbNode, context: Context) { let currentFrontmatter = {}; const frontmatter = cheerio(node); if (!context.processingOptions.omitFrontmatter && frontmatter.text().trim()) { @@ -153,18 +153,16 @@ export class NodeProcessor { * Removes the node if modal id already exists, processes node otherwise */ private processModal(node: MbNode) { - if (node.attribs) { - if (this.processedModals[node.attribs.id]) { - cheerio(node).remove(); - } else { - this.processedModals[node.attribs.id] = true; + if (this.processedModals[node.attribs.id]) { + cheerio(node).remove(); + } else { + this.processedModals[node.attribs.id] = true; - // Transform deprecated slot names; remove when deprecating - renameSlot(node, 'modal-header', 'header'); - renameSlot(node, 'modal-footer', 'footer'); + // Transform deprecated slot names; remove when deprecating + renameSlot(node, 'modal-header', 'header'); + renameSlot(node, 'modal-footer', 'footer'); - this.mdAttributeRenderer.processModalAttributes(node); - } + this.mdAttributeRenderer.processModalAttributes(node); } } @@ -397,7 +395,7 @@ export class NodeProcessor { return; } const mainHtmlNodes = dom.map((d) => { - let processed; + let processed: NodeOrText; try { processed = this.traverse(d, context); } catch (err: any) { diff --git a/packages/core/src/html/SiteLinkManager.ts b/packages/core/src/html/SiteLinkManager.ts index ee041d720c..eb835b25dd 100644 --- a/packages/core/src/html/SiteLinkManager.ts +++ b/packages/core/src/html/SiteLinkManager.ts @@ -1,7 +1,7 @@ import has from 'lodash/has'; -import { DomElement } from 'htmlparser2'; import * as linkProcessor from './linkProcessor'; import type { NodeProcessorConfig } from './NodeProcessor'; +import { MbNode } from '../utils/node'; const _ = { has }; @@ -51,16 +51,14 @@ export class SiteLinkManager { * Add a link to the intralinkCollection to be validated later, * if the node should be validated and intralink validation is not disabled. */ - collectIntraLinkToValidate(node: DomElement, cwf: string) { - if (node.name && !tagsToValidate.has(node.name)) { + collectIntraLinkToValidate(node: MbNode, cwf: string) { + if (!tagsToValidate.has(node.name)) { return 'Should not validate'; } - if (node.attribs) { - const hasIntralinkValidationDisabled = _.has(node.attribs, 'no-validation'); - if (hasIntralinkValidationDisabled) { - return 'Intralink validation disabled'; - } + const hasIntralinkValidationDisabled = _.has(node.attribs, 'no-validation'); + if (hasIntralinkValidationDisabled) { + return 'Intralink validation disabled'; } const resourcePath = linkProcessor.getDefaultTagsResourcePath(node); diff --git a/packages/core/src/html/codeblockProcessor.ts b/packages/core/src/html/codeblockProcessor.ts index 27a88037a5..8c1fffb6e2 100644 --- a/packages/core/src/html/codeblockProcessor.ts +++ b/packages/core/src/html/codeblockProcessor.ts @@ -1,6 +1,6 @@ import cheerio from 'cheerio'; -import { DomElement } from 'htmlparser2'; import has from 'lodash/has'; +import { NodeOrText, MbNode } from '../utils/node'; const md = require('../lib/markdown-it'); const util = require('../lib/markdown-it/utils'); @@ -20,9 +20,9 @@ interface TraverseLinePartData { * @param node The node of the line part to be traversed * @param hlStart The highlight start position, relative to the start of the line part * @param hlEnd The highlight end position, relative to the start of the line part - * @returns {object} An object that contains data to be used by the node's parent. + * @returns An object that contains data to be used by the node's parent. */ -function traverseLinePart(node: DomElement, hlStart: number, hlEnd: number): TraverseLinePartData { +function traverseLinePart(node: NodeOrText, hlStart: number, hlEnd: number): TraverseLinePartData { const resData: TraverseLinePartData = { numCharsTraversed: 0, shouldParentHighlight: false, @@ -128,7 +128,7 @@ function traverseLinePart(node: DomElement, hlStart: number, hlEnd: number): Tra * traverses over the line and applies the highlight. * @param node Root of the code block element, which is the 'pre' node */ -export function highlightCodeBlock(node: DomElement) { +export function highlightCodeBlock(node: MbNode) { if (!node.children) { return; } @@ -155,8 +155,8 @@ export function highlightCodeBlock(node: DomElement) { * @param node the code block element, which is the 'code' node * @param showCodeLineNumbers true if line numbers should be shown, false otherwise */ -export function setCodeLineNumbers(node: DomElement, showCodeLineNumbers: boolean) { - const existingClass = node.attribs?.class || ''; +export function setCodeLineNumbers(node: MbNode, showCodeLineNumbers: boolean) { + const existingClass = node.attribs.class || ''; const styleClassRegex = /(^|\s)(no-)?line-numbers($|\s)/; const hasStyleClass = styleClassRegex.test(existingClass); if (hasStyleClass) { diff --git a/packages/core/src/html/headerProcessor.ts b/packages/core/src/html/headerProcessor.ts index 751beda441..d8370a9d01 100644 --- a/packages/core/src/html/headerProcessor.ts +++ b/packages/core/src/html/headerProcessor.ts @@ -1,9 +1,9 @@ import cheerio from 'cheerio'; import slugify from '@sindresorhus/slugify'; import has from 'lodash/has'; -import { DomElement } from 'htmlparser2'; import { getVslotShorthandName } from './vueSlotSyntaxProcessor'; import type { NodeProcessorConfig } from './NodeProcessor'; +import { MbNode, NodeOrText } from '../utils/node'; const _ = { has, @@ -12,7 +12,7 @@ const _ = { /* * h1 - h6 */ -export function setHeadingId(node: DomElement, config: NodeProcessorConfig) { +export function setHeadingId(node: MbNode, config: NodeProcessorConfig) { const textContent = cheerio(node).text(); // remove the '<' and '>' symbols that markdown-it uses to escape '<' and '>' const cleanedContent = textContent.replace(/<|>/g, ''); @@ -27,16 +27,15 @@ export function setHeadingId(node: DomElement, config: NodeProcessorConfig) { headerIdMap[slugifiedHeading] = 2; } - node.attribs = node.attribs ?? {}; node.attribs.id = headerId; } /** * Traverses the dom breadth-first from the specified element to find a html heading child. * @param node Root element to search from - * @returns {undefined|*} The header element, or undefined if none is found. + * @returns The header element, or undefined if none is found. */ -function _findHeaderElement(node: DomElement) { +function _findHeaderElement(node: MbNode): undefined | NodeOrText { const elements = node.children; if (!elements || !elements.length) { return undefined; @@ -64,7 +63,7 @@ function _findHeaderElement(node: DomElement) { * This is to ensure anchors still work when panels are in their minimized form. * @param node The root panel element */ -export function assignPanelId(node: DomElement) { +export function assignPanelId(node: MbNode) { const slotChildren = node.children ? node.children.filter(child => getVslotShorthandName(child) !== '') : []; @@ -72,7 +71,7 @@ export function assignPanelId(node: DomElement) { const headerSlot = slotChildren.find(child => getVslotShorthandName(child) === 'header'); if (headerSlot) { - const header = _findHeaderElement(headerSlot); + const header = _findHeaderElement(headerSlot as MbNode); if (!header) { return; } @@ -82,7 +81,6 @@ export function assignPanelId(node: DomElement) { + 'Please report this to the MarkBind developers. Thank you!'); } - node.attribs = node.attribs ?? {}; node.attribs.panelId = header.attribs.id; } } diff --git a/packages/core/src/html/linkProcessor.ts b/packages/core/src/html/linkProcessor.ts index aa5df522eb..bbb011897f 100644 --- a/packages/core/src/html/linkProcessor.ts +++ b/packages/core/src/html/linkProcessor.ts @@ -3,7 +3,6 @@ import has from 'lodash/has'; import parse from 'url-parse'; import ignore from 'ignore'; -import { DomElement } from 'htmlparser2'; import * as fsUtil from '../utils/fsUtil'; import * as logger from '../utils/logger'; import * as urlUtil from '../utils/urlUtil'; @@ -11,6 +10,7 @@ import * as urlUtil from '../utils/urlUtil'; import { PluginManager } from '../plugins/PluginManager'; import type { NodeProcessorConfig } from './NodeProcessor'; import type { PageSources } from '../Page/PageSources'; +import { MbNode } from '../utils/node'; const _ = { has }; @@ -28,12 +28,11 @@ const defaultTagLinkMap: { [key: string]: string } = { script: 'src', }; -export function hasTagLink(node: DomElement) { - return node.name && (node.name in defaultTagLinkMap || node.name in pluginTagConfig); +export function hasTagLink(node: MbNode) { + return node.name in defaultTagLinkMap || node.name in pluginTagConfig; } -export function getDefaultTagsResourcePath(node: DomElement): string { - if (!node.name || !node.attribs) return ''; +export function getDefaultTagsResourcePath(node: MbNode) { const linkAttribName = defaultTagLinkMap[node.name]; const resourcePath = node.attribs[linkAttribName]; return resourcePath; @@ -55,7 +54,7 @@ export function isIntraLink(resourcePath: string | undefined): boolean { && !MAILTO_OR_TEL_REGEX.test(resourcePath); } -function _convertRelativeLink(node: DomElement, cwf: string, rootPath: string, +function _convertRelativeLink(node: MbNode, cwf: string, rootPath: string, baseUrl: string, resourcePath: string | undefined, linkAttribName: string) { if (!resourcePath || !isIntraLink(resourcePath)) { return; @@ -70,9 +69,7 @@ function _convertRelativeLink(node: DomElement, cwf: string, rootPath: string, const fullResourcePath = path.join(cwd, resourcePath); const resourcePathFromRoot = _getResourcePathFromRoot(rootPath, fullResourcePath); - if (node.attribs) { - node.attribs[linkAttribName] = path.posix.join(baseUrl || '/', resourcePathFromRoot); - } + node.attribs[linkAttribName] = path.posix.join(baseUrl || '/', resourcePathFromRoot); } /** @@ -82,20 +79,19 @@ function _convertRelativeLink(node: DomElement, cwf: string, rootPath: string, * * TODO allow plugins to tap into this process / extend {@link defaultTagLinkMap} * - * @param {DomElement} node from the dom traversal - * @param {string} cwf as flagged from {@link NodeProcessor} - * @param {string} rootPath of the root site - * @param {string} baseUrl + * @param node from the dom traversal + * @param cwf as flagged from {@link NodeProcessor} + * @param rootPath of the root site + * @param baseUrl */ -export function convertRelativeLinks(node: DomElement, cwf: string, rootPath: string, baseUrl: string) { - if (!node.name) return; +export function convertRelativeLinks(node: MbNode, cwf: string, rootPath: string, baseUrl: string) { if (node.name in defaultTagLinkMap) { const resourcePath = getDefaultTagsResourcePath(node); const linkAttribName = defaultTagLinkMap[node.name]; _convertRelativeLink(node, cwf, rootPath, baseUrl, resourcePath, linkAttribName); } - if (node.name in pluginTagConfig && pluginTagConfig[node.name].attributes && node.attribs) { + if (node.name in pluginTagConfig && pluginTagConfig[node.name].attributes) { pluginTagConfig[node.name].attributes.forEach((attrConfig) => { if (attrConfig.isRelative && node.attribs) { const resourcePath = node.attribs[attrConfig.name]; @@ -105,8 +101,8 @@ export function convertRelativeLinks(node: DomElement, cwf: string, rootPath: st } } -export function convertMdExtToHtmlExt(node: DomElement) { - if (node.name === 'a' && node.attribs && node.attribs.href) { +export function convertMdExtToHtmlExt(node: MbNode) { + if (node.name === 'a' && node.attribs.href) { const hasNoConvert = _.has(node.attribs, 'no-convert'); if (hasNoConvert) { return; @@ -164,10 +160,10 @@ function isValidFileAsset(resourcePath: string, config: NodeProcessorConfig) { * Serves as an internal intra-link validator. Checks if the intra-links are valid. * If the intra-links are suspected to be invalid, a warning message will be logged. * - * @param {string} resourcePath parsed from the node's relevant attribute - * @param {string} cwf as flagged from {@link NodePreprocessor} - * @param {NodeProcessorConfig} config passed for page metadata access - * @returns {string} these string return values are for unit testing purposes only + * @param resourcePath parsed from the node's relevant attribute + * @param cwf as flagged from {@link NodePreprocessor} + * @param config passed for page metadata access + * @returns these string return values are for unit testing purposes only */ export function validateIntraLink(resourcePath: string, cwf: string, config: NodeProcessorConfig): string { if (!isIntraLink(resourcePath)) { @@ -230,22 +226,21 @@ export function validateIntraLink(resourcePath: string, cwf: string, config: Nod * Resolves and collects source file paths pointed to by attributes in nodes for live reload. * Only necessary for plugins for now. * - * @param {DomElement} node from the dom traversal - * @param {string} rootPath site root path to resolve the link from - * @param {string} baseUrl base url to strip off the link (if any) - * @param {PageSources} pageSources {@link PageSources} object to add the resolved file path to - * @returns {string | void} these string return values are for unit testing purposes only + * @param node from the dom traversal + * @param rootPath site root path to resolve the link from + * @param baseUrl base url to strip off the link (if any) + * @param pageSources {@link PageSources} object to add the resolved file path to + * @returns these string return values are for unit testing purposes only */ -export function collectSource(node: DomElement, rootPath: string, +export function collectSource(node: MbNode, rootPath: string, baseUrl: string, pageSources: PageSources): string | void { - if (!node.name) return; const tagConfig = pluginTagConfig[node.name]; if (!tagConfig || !tagConfig.attributes) { return; } tagConfig.attributes.forEach((attrConfig) => { - if (!attrConfig.isSourceFile || !node.attribs) { + if (!attrConfig.isSourceFile) { return; } diff --git a/packages/core/src/html/siteAndPageNavProcessor.ts b/packages/core/src/html/siteAndPageNavProcessor.ts index feee6ea526..12aa51b6f8 100644 --- a/packages/core/src/html/siteAndPageNavProcessor.ts +++ b/packages/core/src/html/siteAndPageNavProcessor.ts @@ -1,6 +1,8 @@ import { v4 as uuidv4 } from 'uuid'; -import { DomElement } from 'htmlparser2'; import cheerio from 'cheerio'; +import { + MbNode, NodeOrText, parseHTML, TextElement, +} from '../utils/node'; require('../patches/htmlparser2'); const md = require('../lib/markdown-it'); @@ -38,21 +40,18 @@ const SITE_NAV_DROPDOWN_ICON_ROTATED_HTML = '