feat: Add range to nodes and fix whitespace issue (fixes #137) #138
Conversation
| @@ -1012,88 +1018,109 @@ export function base_parse(data: string, options = { lowerCaseTagName: false, co | |||
| return it.test(tag); | |||
| }); | |||
| } | |||
| const root = new HTMLElement(null, {}, '', null); | |||
There was a problem hiding this comment.
Parser: The logic is the same, albeit cleaned up a bit. I also fixed the whitespace issue, added some comments, and used labels which made it more obvious what's happening. Some efficiency should be gained as well.
| writable: true, | ||
| configurable: true, | ||
| value: range ?? [ -1, -1 ] | ||
| }); |
There was a problem hiding this comment.
We define as non-enumerable, to preserve existing functionality for equality tests between a synthetic node (one created manually) and a parsed node (one created during parse()). It's unlikely that this has a broad impact scope outside of this library's own tests, but it seems a good idea to make it non-enumerable anyway. It also allows us to not have to change any of our existing tests to exclude range property from equality testing.
| @@ -0,0 +1,85 @@ | |||
| const { parse, HTMLElement, TextNode, CommentNode } = require('../dist'); | |||
There was a problem hiding this comment.
This test parses with htmlparser2 and node-html-parser and compares the output ranges and nodes. Although it serves as a test for range comparison, it will also be helpful in catching a broad range of other issues with the parser that other tests may not cover. This is because it ensures the AST structures are the same for all fundamental node types (blockText, comment, self-closing, etc) parsed by the parser.
|
@taoqf Ready for review! |
|
Thank you. |
Abstract
It turns out that other AST parsers do create a
TextNodefor all whitespace (see https://astexplorer.net) The only which does not is the parser for angular templates, which it likely does because it's used in a very narrow context.The issue ended up being that
node-html-parseractually does create aTextNodefor whitespace, but there were some cases in which that was failing to happen. This caused the behaviour to differ from other parsers and is the root of the issue laid out in #137.Features / Fixes
rangeproperty to nodesCode Changes
Version
v4.1.0
Because a feature was added, I recommend bumping minor version.
Related