Strange marker node bug

[zakstucke]

This is a rather unorthodox bug report, but I thought this would be the easiest way to review and checkout locally.

Whilst adding erasure, I think I've stumbled on a bug, or something very weird.

It seems having HtmlElement extend it's children as Vec<AnyView> breaks leptos somehow. With this PR, running counter_isomorphic cargo leptos watch and checking out the site, console shows:

(index):419 Uncaught SyntaxError: Unexpected token '>' (at (index):419:21)
(index):427 Uncaught SyntaxError: Unexpected token '>' (at (index):427:58)

Which is a random <!> marker in the html in the second line of this snippet:

ws.onclose = () => console.warn('Live-reload stopped. Manual reload necessary.');
 })(3001, 'ws://')<!></script><link rel="modulepreload" href="/pkg/counter_isomorphic.js" nonce="nRIE4NM7UA2nhEng5NCK0g"><link rel="preload" href="/pkg/counter_isomorphic.wasm" as="fetch" type="application/wasm" crossorigin="nRIE4NM7UA2nhEng5NCK0g"><script type="module" nonce="nRIE4NM7UA2nhEng5NCK0g">(function (root, pkg_path, output_name, wasm_output_name) {
	import(`${root}/${pkg_path}/${output_name}.js`)
		.then(mod => {
			mod.default({module_or_path: `${root}/${pkg_path}/${wasm_output_name}.wasm`}).then(() => {
				mod.hydrate();
			});
		})
})

I'm pretty confused how it could be anything wrong with this diff, and I've un erased flagged it so this is verifiable in normal mode. Any ideas @gbj?

I also checked patching 0.8 branch with the solution to #3583, but doesn't help.

[zakstucke]

CI failures show the same issue I'm describing

[gbj]

It's going to be something like "AnyView should check the escape argument to the HTML rendering methods on RenderHtml before inserting its marker node, but doesn't." The output you list would be fine in a normal HTML element (it's just a comment), but is a syntax error in JS (or in CSS), so is suppressed in those situations. The same thing is true for escaping text nodes to replace sensitive characters (<>"" etc) with HTML entities (which is why escape is the argument name).

This is probably true in other situations where a marker <!> is inserted during HTML rendering.

(I have not looked at any source code but that looks like what's happening to me)

[zakstucke]

@gbj thanks! If my second (tiny) commit is what you were describing, then this seems to have fixed it. Seems to work fine locally.

The html diff error in the counters CI example is weird though, I think it might just be a side-effect of the erasure but not actually a problem.

(this erase impl is obviously going to be erase feature flagged in the final version)

[benwis]

@zakstucke Looks like it needs a bit of tweaking, you probably didn't mean to insert more <!>

[zakstucke]

@benwis haven't added any new <!> I don't think, just only including them if escape=true, which I think is what @gbj meant, and I think(?!) makes sense to me.

[benwis]

@benwis haven't added any new <!> I don't think, just only including them if escape=true, which I think is what @gbj meant, and I think(?!) makes sense to me.

I can't find the test that failed, but one of them failed with a weird <!> before a closing tag. Most of the tests seem to fail like this

         assertion `left == right` failed
          left: "<div><button>Clear<!----></button><button>-1<!----></button><span>Value: 0!<!----></span><button>+1<!----></button><!----></div>"
         right: "<div><button>Clear</button><button>-1</button><span>Value: 0!<!----></span><button>+1</button><!----></div>"

Which seems like you're not escaping enough? Or differently than we were before, so the tests fail

[gbj]

Commit looks right to me -- not sure why it adds escape placeholders in the counters example, I can dig in a bit.