rework parsing to use substitutions

[43081j]

I have a feeling we're going to need this when we want to start detecting things like:

<${tag}>

<div ${directive}>

It is very delicate right now in master because it depends entirely on length of the parsed HTML matching length of the input JS, which means we're limited on what we can use as placeholders.

If we want to detect tag bindings and other things, we need the placeholder to be a valid tag name (so parse5 is happy), too.

This is one option i think, where we track a set of substitutions instead of arbitrary ____ placeholders.

Feedback would be appreciated, if this is the wrong way to go then we can close it

Depends on #168 to fix the vscode CI error

[justinfagnani]

@43081j one option here is to use the tokenization algorithm that lit-html itself uses which detects syntax position. Then you can insert the correct marker in all cases.

[43081j]

i actually now realised i think the substitution is pointless in fact, it could just be a constant value! like lit$analyzer$

we can infer from those placeholders where the expressions were, so we don't actually care what each one is individually...

ill have a re-read through lit's parsing though too as that sounds useful here

[justinfagnani]

@43081j these are placeholders inserted before parsing with parse5, yeah? If so, we do care about what the markers are because they have to be valid for the syntax position of the expression. The main thing is that you want comment makers for child expressions so that you avoid any "adoption agency" side effects (that's the algorithm that moves disallowed nodes out of <table> and such). But you can't use comments for tag-position expressions, element expressions, or unquoted attribute value expressions.

I think the best thing might be to copy the getTemplateHtml code from lit-html 2.0. I'll be fine to use against lit-html 1.x templates.

[43081j]

Yes exactly. In master it is already wrong, as the placeholder is '_'.repeat(len) which results in directives in tag and attribute positions causing parse errors (parse5).

I'll have a look at copying that function over, i almost did but wasn't sure if we needed it yet. would be a lot nicer than a constant placeholder i think

[justinfagnani]

Oh, and I bet the '_'.repeat(len) is to keep the mapping from parse5 source locations to .ts source locations simple. With the proper marker types we'll have a minimum length, so we can't quite match that strategy. @rictic had to deal with some similar issues before. Maybe the mapping isn't so bad as long as newlines are preserved?

[43081j]

It is but i already fixed the positioning so it can be variable length now (the placeholder).

We just calculate the correct offsets on the way out based on the placeholder length. Thats constant for now but if it wasn't, we'd just keep a map of expressions to placeholders for the same reason.

It should work fine this way unless im missing something

[justinfagnani]

@43081j looking at how we could add element binding support, I do think we will need to use the HTML tokenization code from lit-html to determine which marker to insert.

[justinfagnani]

I'm working on this today.

[43081j]

sure, feel free to close this if you open another PR for it.

would be nice i think to use the same logic as lit itself like you mentioned