Skip to content

Commit

Permalink
Call createTree on parser close, not open (#289)
Browse files Browse the repository at this point in the history
* Call createTree on parser close, not open

In the new parser createTree was called with only a tag name and not the
attributes or childNodes. This PR fixes the new parser to wait until the
element is closed before calling createTree. This work uncovered some
inconsistencies and improved the parser by closing out the pointer and
resetting to the parent in a function.

* Update comment

* Remove unused prop

* Satisfy type check
  • Loading branch information
tbranyen committed Nov 25, 2022
1 parent 1c86a9e commit 960c84f
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 22 deletions.
68 changes: 49 additions & 19 deletions packages/diffhtml/lib/util/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ export default function parse(html, options = {}) {
// Cursor into the markup that we use when parsing.
let i = 0;

// The active element being crawled.
/**
* The active element being crawled.
* @type {VTree}
*/
let pointer = root;

// The pointer is open when looking for attributes, self closing, or open
Expand All @@ -171,6 +174,19 @@ export default function parse(html, options = {}) {
*/
let lastCommentIndex = html.indexOf('<!--');

// Closes the current element and calls createTree to allow middleware to tap
// into it. Resets the pointer to the parent. This function should never be
// called with the root element, otherwise it will set a null pointer.
const resetPointer = () => {
// Create tree is called to normalize the stack into VTree and allow
// middleware to hook into the parser.
const newTree = createTree(stack.pop());

// Reset the pointer to the parent.
pointer = stack[stack.length - 1];
pointer.childNodes.push(newTree);
};

// This loop treats the `i` as a cursor into the markup determining what is
// being parsed. This is useful for setting the `lastIndex` values of the
// regular expressions defined above. Once this value matches the length of
Expand Down Expand Up @@ -212,6 +228,8 @@ export default function parse(html, options = {}) {
lastCommentIndex = null;
}

const isNotRoot = pointer !== root;

// If an element is a block text element (such as script) we should not
// parse anything under it, except as text.
const isBlockElement = pointer && blockText.has(pointer.nodeName);
Expand Down Expand Up @@ -252,18 +270,25 @@ export default function parse(html, options = {}) {
continue;
}

// If not a doctype, create an element.
const newTree = createTree(tagName);

// Don't call createTree yet, otherwise we won't have access to the
// completed element. So create a fake VTree, to build up the object
// until we have attributes and child nodes.
const newTree = {
rawNodeName: tagName,
nodeName: tagName,
childNodes: [],
attributes: {},
nodeType: EMPTY.NUM,
nodeValue: EMPTY.STR,
key: EMPTY.STR,
};
const supportsEndTagOmission = endTagOmissionRules[tagName];

// We can't nested a div inside a p, we can't nest an li inside an li
if (supportsEndTagOmission && supportsEndTagOmission[pointer.nodeName]) {
stack.pop();
pointer = stack[stack.length - 1];
resetPointer();
}

pointer.childNodes.push(newTree);
pointer = newTree;
stack.push(pointer);

Expand All @@ -287,11 +312,13 @@ export default function parse(html, options = {}) {

// TBD Refactor this so its not duplicated
if (html[i - 1] === '>') {
const isEnd = i === html.length;

// Self closing
if (html[i - 2] === '/' || voidElements.has(pointer.nodeName)) {
stack.pop();
pointer = stack[stack.length - 1];
if (html[i - 2] === '/' || voidElements.has(pointer.nodeName) || isEnd) {
resetPointer();
}

isOpen = false;
}

Expand All @@ -316,7 +343,7 @@ export default function parse(html, options = {}) {
// use the entire input.
if (isBlockElement) {
const closeTag = `</${pointer.nodeName}>`;
let closeTagIndex = html.indexOf(closeTag, i + 1);
let closeTagIndex = html.indexOf(closeTag, i);

if (closeTagIndex === -1) {
closeTagIndex = html.length;
Expand All @@ -330,8 +357,7 @@ export default function parse(html, options = {}) {

i = closeTagIndex + closeTag.length;
isOpen = false;
stack.pop();
pointer = stack[stack.length - 1];
resetPointer();
continue;
}

Expand All @@ -342,9 +368,9 @@ export default function parse(html, options = {}) {

// Automatically close void elements.
if (voidElements.has(pointer.nodeName)) {
stack.pop();
pointer = stack[stack.length - 1];
resetPointer();
}

continue;
}

Expand All @@ -354,14 +380,13 @@ export default function parse(html, options = {}) {
index: closeTagIndex,
} = closeTag.exec(html) || EMPTY.OBJ;


// Look for closing tags
if (closeTagIndex === i && fullCloseTagMatch) {
if (fullCloseTagMatch[1] === '/' && pointer !== root) {
stack.pop();
pointer = stack[stack.length - 1];
if (fullCloseTagMatch[1] === '/' && isNotRoot) {
resetPointer();
}
isOpen = false;

i = closeTagIndex + fullCloseTagMatch.length;
continue;
}
Expand All @@ -376,6 +401,11 @@ export default function parse(html, options = {}) {
const newTree = createTree('#text', fullTextMatch);
pointer.childNodes.push(newTree);
i = textIndex + fullTextMatch.length;

if (i === html.length && isNotRoot) {
resetPointer();
}

continue;
}

Expand Down
42 changes: 39 additions & 3 deletions packages/diffhtml/test/use.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,8 @@ describe('Use (Middleware)', function() {
const Fn = ({ message }) => html`<marquee>${message}</marquee>`;
const domNode = document.createElement('div');

let i = 0;

this.createTreeHook = ({ rawNodeName, attributes }) => {
if (typeof rawNodeName === 'function') {
i++;
return rawNodeName(attributes);
}
};
Expand All @@ -125,6 +122,45 @@ describe('Use (Middleware)', function() {
release(domNode);
});

it('will allow access to attributes during create', () => {
const domNode = document.createElement('div');

let message = null;

this.createTreeHook = ({ nodeName, attributes }) => {
if (nodeName === 'div' && attributes.message) {
message = attributes.message;
}
};

innerHTML(domNode, html`
<div message="Hello world" />
`);

equal(message, 'Hello world');
release(domNode);
});

it('will allow access to childNodes during create', () => {
const domNode = document.createElement('div');

let message = null;

this.createTreeHook = ({ nodeName, childNodes }) => {
if (nodeName === 'p') {
// Access the parsed childNodes
message = childNodes[0].attributes.message;
}
};

innerHTML(domNode, html`
<p><div message="Hello world" /></p>
`);

equal(message, 'Hello world');
release(domNode);
});

it('will allow modifying a vTree when created from a DOM Node', () => {
const domNode = document.createElement('div');

Expand Down

0 comments on commit 960c84f

Please sign in to comment.