Invalid double escaping of nested arrays in SSR

[axel-habermaier]

Consider the following code:

function Test() {
  const a = ["<"];
  return <div>{[a, a]}</div>;
}

During SSR, this incorrectly renders the HTML <div>&amp;lt;&amp;lt;</div>, i.e., the < is escaped twice and is thus shown as << in the browser.

I expected SSR to generate the single-escaped HTML <div>&lt;&lt;</div> which then renders as << in the browser.

[axel-habermaier]

Update: The underlying reason seems to be that the array a is unexpectedly modified during rendering! So if a were at module scope, there would be additional escaping for each render.

[mdynnl]

Yeah, that can also be reproduced using only jsx

import { renderToString } from "solid-js/web"

let x = "<"
let a = <>{x}{x}</>
let v = <>{a}{a}</>

console.log(v)
console.log(renderToString(() => <>{v}</>))
console.log(v)

we definitely should create a new array here before escaping its elements

dom-expressions/packages/dom-expressions/src/server.js

Lines 412 to 418 in 8f9f432

	export function escape(s, attr) {
	const t = typeof s;
	if (t !== "string") {
	if (!attr && t === "function") return escape(s());
	if (!attr && Array.isArray(s)) {
	for (let i = 0; i < s.length; i++) s[i] = escape(s[i]);
	return s;

also created https://github.com/mdynnl/solid-ssr-escape-bench to benchmark which approach is faster although it may vary depending on the runtime

after trying with both bun(1.2.3-canary) and node(v20.17.0), i honestly don't know which one is faster as the results are entirely different, but .slice() and Array(length) seems to be on top in most cases which makes sense somewhat. and although i don't know enough js engine internals, i'd expect .slice() to only allocate memory for the array itself, so it's probably the answer here.

[edemaine]

@mdynnl Can you benchmark return s.map(escape)? That seems like the obvious way to write it, and hopefully map is smart enough to preallocate the array and maybe do the loop slightly more efficiently.