Skip to content
This repository has been archived by the owner on Feb 28, 2022. It is now read-only.

Commit

Permalink
Bugfix: Markdown stages in html pipe should not an extra wrapping div
Browse files Browse the repository at this point in the history 
This wrapping div would be added when there was more then one
element in the markdown (which is the usual case) by hyperscript
because hyperscript has no concept of multiple elements in the
root node.

Our code came to expect this wrapping which led to markdowns with
a single element being rendered as an empty document in some
cases (the single element would be deleted during the emit-html
when generating .content.children).

This commit fixes this issue by using hast-util-to-html instead of
hyperscript for the hast -> html conversion which does indeed have
a concept of multiple elements at the root node.

Fixes #157
  • Loading branch information
@koraa
koraa committed Jan 25, 2019
1 parent 761cadb commit b65211b
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 59 deletions.
9 changes: 3 additions & 6 deletions src/html/emit-html.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@
function emit({ content: { document } }, { logger }) {
logger.debug(`Emitting HTML from ${typeof document}`);

const children = document.body && document.body.firstChild && document.body.firstChild.childNodes
? Array.from(document.body.firstChild.childNodes)
.filter(node => node && node.outerHTML)
.map(node => node.outerHTML) : [];

return { content: { html: document.body.innerHTML || '', children } };
const html = document.body.innerHTML || '';
const children = Array.from(document.body.childNodes).map(node => node.outerHTML).filter(x => x);
return { content: { html, children } };
}

module.exports = emit;
18 changes: 5 additions & 13 deletions src/utils/mdast-to-vdom.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,10 @@
/* eslint no-unused-vars: ["error", { "argsIgnorePattern": "^_" }] */
const { selectAll } = require('unist-util-select');
const handlers = require('mdast-util-to-hast/lib/handlers');
const tohast = require('mdast-util-to-hast');
const mdast2hast = require('mdast-util-to-hast');
const hast2html = require('hast-util-to-html');
const unified = require('unified');
const parse = require('rehype-parse');
const tohyper = require('hast-to-hyperscript');
const h = require('hyperscript');
const { JSDOM } = require('jsdom');
const image = require('./image-handler');
const embed = require('./embed-handler');
Expand Down Expand Up @@ -188,21 +187,14 @@ class VDOMTransformer {
};
}

/**
* Turns the MDAST into a basic DOM-like structure using Hyperscript
* @returns {Node} a simple DOM node (not all DOM functions exposed)
*/
process() {
// turn MDAST to HTAST and then HTAST to VDOM via Hyperscript
return tohyper(h, tohast(this._root, { handlers: this._handlers }));
}

/**
* Turns the MDAST into a full DOM-like structure using JSDOM
* @returns {Node} a full DOM node
*/
getDocument() {
return new JSDOM(this.process().outerHTML).window.document;
// mdast -> hast; hast -> html -> DOM using JSDOM
const hast = mdast2hast(this._root, { handlers: this._handlers });
return new JSDOM(hast2html(hast)).window.document;
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/testEmbedHandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ https://www.youtube.com/watch?v=KOxbO0EI4MA

assert.equal(result.response.status, 201);
assert.equal(result.response.headers['Content-Type'], 'text/html');
assert.equal(result.response.body, `<div><p>Hello World
assert.equal(result.response.body, `<p>Hello World
Here comes an embed.</p>
<esi:include src="https://example-embed-service.com/https://www.youtube.com/watch?v=KOxbO0EI4MA"></esi:include>
<p><img src="easy.png" alt="Easy!" srcset="easy.png?width=480&amp;auto=webp 480w,easy.png?width=1384&amp;auto=webp 1384w,easy.png?width=2288&amp;auto=webp 2288w,easy.png?width=3192&amp;auto=webp 3192w,easy.png?width=4096&amp;auto=webp 4096w" sizes="100vw"></p></div>`);
<p><img src="easy.png" alt="Easy!" srcset="easy.png?width=480&amp;auto=webp 480w,easy.png?width=1384&amp;auto=webp 1384w,easy.png?width=2288&amp;auto=webp 2288w,easy.png?width=3192&amp;auto=webp 3192w,easy.png?width=4096&amp;auto=webp 4096w" sizes="100vw"></p>`);
});
});
3 changes: 3 additions & 0 deletions test/testHTMLFromMarkdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,7 @@ const itMd = (desc, md, html) => {

describe('Testing Markdown conversion', () => {
itMd('Renders empty markdown', '', '');
itMd('Renders single paragraph', // Regression #157
'Hello World.',
'<p>Hello World.</p>');
});
109 changes: 71 additions & 38 deletions test/testMdastToVDOM.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
*/
/* eslint-env mocha */

const assert = require('assert');
const fs = require('fs-extra');
const path = require('path');
const h = require('hyperscript');
const winston = require('winston');
const { JSDOM } = require('jsdom');
const { assertEquivalentNode } = require('@adobe/helix-shared').dom;
const VDOM = require('../').utils.vdom;
const coerce = require('../src/utils/coerce-secrets');

Expand All @@ -30,40 +31,57 @@ const logger = winston.createLogger({

const action = { logger };

const assertTransformerYieldsDocument = (transformer, expected) => {
assertEquivalentNode(
transformer.getDocument(),
new JSDOM(expected).window.document,
);
};

describe('Test MDAST to VDOM Transformation', () => {
before('Coerce defaults', async () => {
await coerce(action);
});

it('Simple MDAST Conversion', () => {
const mdast = fs.readJSONSync(path.resolve(__dirname, 'fixtures', 'simple.json'));
const transformer = new VDOM(mdast, action.secrets);
const node = transformer.process();
assert.equal(node.outerHTML, '<h1>Hello World</h1>');
assertTransformerYieldsDocument(
new VDOM(mdast, action.secrets),
'<h1>Hello World</h1>',
);
});

it('Custom Text Matcher Conversion', () => {
const mdast = fs.readJSONSync(path.resolve(__dirname, 'fixtures', 'simple.json'));
const transformer = new VDOM(mdast, action.secrets);
transformer.match('heading', () => '<h1>All Headings are the same to me</h1>');
const node = transformer.process();
assert.equal(node.outerHTML, '<h1>All Headings are the same to me</h1>');
assertTransformerYieldsDocument(
transformer,
'<h1>All Headings are the same to me</h1>',
);
});

it('Programmatic Matcher Function', () => {
const mdast = fs.readJSONSync(path.resolve(__dirname, 'fixtures', 'simple.json'));
const transformer = new VDOM(mdast, action.secrets);
transformer.match(({ type }) => type === 'heading', () => '<h1>All Headings are the same to me</h1>');
const node = transformer.process();
assert.equal(node.outerHTML, '<h1>All Headings are the same to me</h1>');
assertTransformerYieldsDocument(
transformer,
'<h1>All Headings are the same to me</h1>',
);
});

it('Custom Text Matcher with Multiple Elements', () => {
const mdast = fs.readJSONSync(path.resolve(__dirname, 'fixtures', 'simple.json'));
const transformer = new VDOM(mdast, action.secrets);
transformer.match('heading', () => '<a name="h1"></a><h1>All Headings are the same to me</h1>');
const node = transformer.process();
assert.equal(node.outerHTML, '<div><a name="h1"></a><h1>All Headings are the same to me</h1></div>');
assertTransformerYieldsDocument(
transformer, `
<div>
<a name="h1"></a>
<h1>All Headings are the same to me</h1>
</div>`,
);
});

it('Custom link handler with VDOM Nodes', () => {
Expand All @@ -77,32 +95,36 @@ describe('Test MDAST to VDOM Transformation', () => {
);
return res;
});
const node = transformer.process();
assert.equal(node.outerHTML, `<ul>
<li>
<p><code>body</code>: the unparsed content body as a <code>string</code></p>
</li>
<li>
<p><code>mdast</code>: the parsed <a href="https://github.com/syntax-tree/mdast" rel="nofollow">Markdown AST</a></p>
</li>
<li>
<p><code>meta</code>: a map metadata properties, including</p>
<ul>
<li><code>title</code>: title of the document</li>
<li><code>intro</code>: a plain-text introduction or description</li>
<li><code>type</code>: the content type of the document</li>
</ul>
</li>
<li>
<p><code>htast</code>: the HTML AST</p>
</li>
<li>
<p><code>html</code>: a string of the content rendered as HTML</p>
</li>
<li>
<p><code>children</code>: an array of top-level elements of the HTML-rendered content</p>
</li>
</ul>`);
assertTransformerYieldsDocument(
transformer, `
<ul>
<li>
<p>
<code>body</code>: the unparsed content body as a <code>string</code>
</p>
</li>
<li>
<p><code>mdast</code>: the parsed <a href="https://github.com/syntax-tree/mdast" rel="nofollow">Markdown AST</a></p>
</li>
<li>
<p><code>meta</code>: a map metadata properties, including</p>
<ul>
<li><code>title</code>: title of the document</li>
<li><code>intro</code>: a plain-text introduction or description</li>
<li><code>type</code>: the content type of the document</li>
</ul>
</li>
<li>
<p><code>htast</code>: the HTML AST</p>
</li>
<li>
<p><code>html</code>: a string of the content rendered as HTML</p>
</li>
<li>
<p><code>children</code>: an array of top-level elements of the HTML-rendered content</p>
</li>
</ul>`,
);
});

it('Custom link handler does not interfere with link rewriting', () => {
Expand All @@ -116,7 +138,18 @@ describe('Test MDAST to VDOM Transformation', () => {
);
return res;
});
const node = transformer.process();
assert.equal(node.outerHTML, '<p><a href="https://house.md/syntax-tree/mdast.md" rel="nofollow">External link</a>: the parsed <a href="/mdast.html" title="My title"><img src="/dist/img/ipad.png" alt="ipad" srcset="/dist/img/ipad.png?width=480&amp;auto=webp 480w,/dist/img/ipad.png?width=1384&amp;auto=webp 1384w,/dist/img/ipad.png?width=2288&amp;auto=webp 2288w,/dist/img/ipad.png?width=3192&amp;auto=webp 3192w,/dist/img/ipad.png?width=4096&amp;auto=webp 4096w" sizes="100vw"></a></p>');
assertTransformerYieldsDocument(
transformer, `
<p>
<a href="https://house.md/syntax-tree/mdast.md"
rel="nofollow">External link</a>:
the parsed
<a href="/mdast.html" title="My title">
<img src="/dist/img/ipad.png" alt="ipad"
srcset="/dist/img/ipad.png?width=480&amp;auto=webp 480w,/dist/img/ipad.png?width=1384&amp;auto=webp 1384w,/dist/img/ipad.png?width=2288&amp;auto=webp 2288w,/dist/img/ipad.png?width=3192&amp;auto=webp 3192w,/dist/img/ipad.png?width=4096&amp;auto=webp 4096w"
sizes="100vw">
</a>
</p>`,
);
});
});

0 comments on commit b65211b

Please sign in to comment.