From e6b754f8dd511d6f0bd77357e23121d6c3c5f04c Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 1 Jul 2021 20:33:25 +0100 Subject: [PATCH 1/3] Convert Markdown to TypeScript --- src/{Markdown.js => Markdown.ts} | 57 +++++++++++++++++--------------- 1 file changed, 30 insertions(+), 27 deletions(-) rename src/{Markdown.js => Markdown.ts} (85%) diff --git a/src/Markdown.js b/src/Markdown.ts similarity index 85% rename from src/Markdown.js rename to src/Markdown.ts index f670bded12..64037369db 100644 --- a/src/Markdown.js +++ b/src/Markdown.ts @@ -1,5 +1,6 @@ /* Copyright 2016 OpenMarket Ltd +Copyright 2021 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,16 +16,16 @@ limitations under the License. */ import * as commonmark from 'commonmark'; -import {escape} from "lodash"; +import { escape } from "lodash"; const ALLOWED_HTML_TAGS = ['sub', 'sup', 'del', 'u']; // These types of node are definitely text const TEXT_NODES = ['text', 'softbreak', 'linebreak', 'paragraph', 'document']; -function is_allowed_html_tag(node) { +function isAllowedHtmlTag(node: any) { if (node.literal != null && - node.literal.match('^<((div|span) data-mx-maths="[^"]*"|\/(div|span))>$') != null) { + node.literal.match('^<((div|span) data-mx-maths="[^"]*"|/(div|span))>$') != null) { return true; } @@ -39,21 +40,12 @@ function is_allowed_html_tag(node) { return false; } -function html_if_tag_allowed(node) { - if (is_allowed_html_tag(node)) { - this.lit(node.literal); - return; - } else { - this.lit(escape(node.literal)); - } -} - /* * Returns true if the parse output containing the node * comprises multiple block level elements (ie. lines), * or false if it is only a single line. */ -function is_multi_line(node) { +function isMultiLine(node) { let par = node; while (par.parent) { par = par.parent; @@ -65,8 +57,13 @@ function is_multi_line(node) { * Class that wraps commonmark, adding the ability to see whether * a given message actually uses any markdown syntax or whether * it's plain text. + * + * Types are a bit of a struggle here as commonmark doesn't have types */ export default class Markdown { + private input: string; + private parsed: any; + constructor(input) { this.input = input; @@ -87,7 +84,7 @@ export default class Markdown { // if it's an allowed html tag, we need to render it and therefore // we will need to use HTML. If it's not allowed, it's not HTML since // we'll just be treating it as text. - if (is_allowed_html_tag(node)) { + if (isAllowedHtmlTag(node)) { return false; } } else { @@ -118,7 +115,7 @@ export default class Markdown { // // Let's try sending with

s anyway for now, though. - const real_paragraph = renderer.paragraph; + const realParagraph = renderer.paragraph; renderer.paragraph = function(node, entering) { // If there is only one top level node, just return the @@ -126,8 +123,8 @@ export default class Markdown { // 'inline', rather than unnecessarily wrapped in its own // p tag. If, however, we have multiple nodes, each gets // its own p tag to keep them as separate paragraphs. - if (is_multi_line(node)) { - real_paragraph.call(this, node, entering); + if (isMultiLine(node)) { + realParagraph.call(this, node, entering); } }; @@ -150,19 +147,26 @@ export default class Markdown { } }; - renderer.html_inline = html_if_tag_allowed; + renderer.html_inline = function(node: any) { + if (isAllowedHtmlTag(node)) { + this.lit(node.literal); + return; + } else { + this.lit(escape(node.literal)); + } + }; renderer.html_block = function(node) { -/* + /* // as with `paragraph`, we only insert line breaks // if there are multiple lines in the markdown. const isMultiLine = is_multi_line(node); if (isMultiLine) this.cr(); -*/ - html_if_tag_allowed.call(this, node); -/* + */ + renderer.html_inline(node); + /* if (isMultiLine) this.cr(); -*/ + */ }; return renderer.render(this.parsed); @@ -178,13 +182,12 @@ export default class Markdown { * which has no formatting. Otherwise it emits HTML(!). */ toPlaintext() { - const renderer = new commonmark.HtmlRenderer({safe: false}); - const real_paragraph = renderer.paragraph; + const renderer = new commonmark.HtmlRenderer({ safe: false }); renderer.paragraph = function(node, entering) { // as with toHTML, only append lines to paragraphs if there are // multiple paragraphs - if (is_multi_line(node)) { + if (isMultiLine(node)) { if (!entering && node.next) { this.lit('\n\n'); } @@ -193,7 +196,7 @@ export default class Markdown { renderer.html_block = function(node) { this.lit(node.literal); - if (is_multi_line(node) && node.next) this.lit('\n\n'); + if (isMultiLine(node) && node.next) this.lit('\n\n'); }; return renderer.render(this.parsed); From c1310bcd9f11faeb5414443cc9ece721582e19eb Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 1 Jul 2021 21:31:17 +0100 Subject: [PATCH 2/3] Better types --- package.json | 1 + src/Markdown.ts | 26 ++++++++++++++++---------- yarn.lock | 5 +++++ 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 4ad585ba7d..c505e16dce 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,7 @@ }, "dependencies": { "@babel/runtime": "^7.12.5", + "@types/commonmark": "^0.27.4", "await-lock": "^2.1.0", "browser-encrypt-attachment": "^0.3.0", "browser-request": "^0.3.3", diff --git a/src/Markdown.ts b/src/Markdown.ts index 64037369db..bb6d6d8df0 100644 --- a/src/Markdown.ts +++ b/src/Markdown.ts @@ -23,7 +23,15 @@ const ALLOWED_HTML_TAGS = ['sub', 'sup', 'del', 'u']; // These types of node are definitely text const TEXT_NODES = ['text', 'softbreak', 'linebreak', 'paragraph', 'document']; -function isAllowedHtmlTag(node: any) { +// As far as @types/commonmark is concerned, these are not public, so add them +interface CommonmarkHtmlRendererInternal extends commonmark.HtmlRenderer { + paragraph: (node: commonmark.Node, entering: boolean) => void; + link: (node: commonmark.Node, entering: boolean) => void; + html_inline: (node: commonmark.Node) => void; // eslint-disable-line camelcase + html_block: (node: commonmark.Node) => void; // eslint-disable-line camelcase +} + +function isAllowedHtmlTag(node: commonmark.Node): boolean { if (node.literal != null && node.literal.match('^<((div|span) data-mx-maths="[^"]*"|/(div|span))>$') != null) { return true; @@ -45,7 +53,7 @@ function isAllowedHtmlTag(node: any) { * comprises multiple block level elements (ie. lines), * or false if it is only a single line. */ -function isMultiLine(node) { +function isMultiLine(node: commonmark.Node): boolean { let par = node; while (par.parent) { par = par.parent; @@ -57,12 +65,10 @@ function isMultiLine(node) { * Class that wraps commonmark, adding the ability to see whether * a given message actually uses any markdown syntax or whether * it's plain text. - * - * Types are a bit of a struggle here as commonmark doesn't have types */ export default class Markdown { private input: string; - private parsed: any; + private parsed: commonmark.Node; constructor(input) { this.input = input; @@ -71,7 +77,7 @@ export default class Markdown { this.parsed = parser.parse(this.input); } - isPlainText() { + isPlainText(): boolean { const walker = this.parsed.walker(); let ev; @@ -94,7 +100,7 @@ export default class Markdown { return true; } - toHTML({ externalLinks = false } = {}) { + toHTML({ externalLinks = false } = {}): string { const renderer = new commonmark.HtmlRenderer({ safe: false, @@ -104,7 +110,7 @@ export default class Markdown { // block quote ends up all on one line // (https://github.com/vector-im/element-web/issues/3154) softbreak: '
', - }); + }) as CommonmarkHtmlRendererInternal; // Trying to strip out the wrapping

causes a lot more complication // than it's worth, i think. For instance, this code will go and strip @@ -181,8 +187,8 @@ export default class Markdown { * N.B. this does **NOT** render arbitrary MD to plain text - only MD * which has no formatting. Otherwise it emits HTML(!). */ - toPlaintext() { - const renderer = new commonmark.HtmlRenderer({ safe: false }); + toPlaintext(): string { + const renderer = new commonmark.HtmlRenderer({ safe: false }) as CommonmarkHtmlRendererInternal; renderer.paragraph = function(node, entering) { // as with toHTML, only append lines to paragraphs if there are diff --git a/yarn.lock b/yarn.lock index 6f08372e18..d59dc9d79f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1478,6 +1478,11 @@ resolved "https://registry.yarnpkg.com/@types/classnames/-/classnames-2.2.11.tgz#2521cc86f69d15c5b90664e4829d84566052c1cf" integrity sha512-2koNhpWm3DgWRp5tpkiJ8JGc1xTn2q0l+jUNUE7oMKXUf5NpI9AIdC4kbjGNFBdHtcxBD18LAksoudAVhFKCjw== +"@types/commonmark@^0.27.4": + version "0.27.4" + resolved "https://registry.yarnpkg.com/@types/commonmark/-/commonmark-0.27.4.tgz#8f42990e5cf3b6b95bd99eaa452e157aab679b82" + integrity sha512-7koSjp08QxKoS1/+3T15+kD7+vqOUvZRHvM8PutF3Xsk5aAEkdlIGRsHJ3/XsC3izoqTwBdRW/vH7rzCKkIicA== + "@types/counterpart@^0.18.1": version "0.18.1" resolved "https://registry.yarnpkg.com/@types/counterpart/-/counterpart-0.18.1.tgz#b1b784d9e54d9879f0a8cb12f2caedab65430fe8" From fda17b4959b90de2db2884b126bcb7fc14dd4007 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 1 Jul 2021 23:02:51 +0100 Subject: [PATCH 3/3] Add types to the implementations too Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Markdown.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Markdown.ts b/src/Markdown.ts index bb6d6d8df0..96169d4011 100644 --- a/src/Markdown.ts +++ b/src/Markdown.ts @@ -123,7 +123,7 @@ export default class Markdown { const realParagraph = renderer.paragraph; - renderer.paragraph = function(node, entering) { + renderer.paragraph = function(node: commonmark.Node, entering: boolean) { // If there is only one top level node, just return the // bare text: it's a single line of text and so should be // 'inline', rather than unnecessarily wrapped in its own @@ -153,7 +153,7 @@ export default class Markdown { } }; - renderer.html_inline = function(node: any) { + renderer.html_inline = function(node: commonmark.Node) { if (isAllowedHtmlTag(node)) { this.lit(node.literal); return; @@ -162,7 +162,7 @@ export default class Markdown { } }; - renderer.html_block = function(node) { + renderer.html_block = function(node: commonmark.Node) { /* // as with `paragraph`, we only insert line breaks // if there are multiple lines in the markdown. @@ -190,7 +190,7 @@ export default class Markdown { toPlaintext(): string { const renderer = new commonmark.HtmlRenderer({ safe: false }) as CommonmarkHtmlRendererInternal; - renderer.paragraph = function(node, entering) { + renderer.paragraph = function(node: commonmark.Node, entering: boolean) { // as with toHTML, only append lines to paragraphs if there are // multiple paragraphs if (isMultiLine(node)) { @@ -200,7 +200,7 @@ export default class Markdown { } }; - renderer.html_block = function(node) { + renderer.html_block = function(node: commonmark.Node) { this.lit(node.literal); if (isMultiLine(node) && node.next) this.lit('\n\n'); };