Invalid double escaping of nested arrays in SSR
Consider the following code:
function Test() {
const a = ["<"];
return <div>{[a, a]}</div>;
}
During SSR, this incorrectly renders the HTML <div>&lt;&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><<</div> which then renders as << in the browser.
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.
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 https://github.com/ryansolid/dom-expressions/blob/8f9f43254cf313d2cda653502da1097c76b991af/packages/dom-expressions/src/server.js#L412-L418
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.
@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.
@mdynnl Can you benchmark
return s.map(escape)? That seems like the obvious way to write it, and hopefullymapis smart enough to preallocate the array and maybe do the loop slightly more efficiently.
No, but be very careful with shorting map as array.map(escape) !
map passes to the callback three arguments. Same as doing array.map((element, index, thisArg) => escape(element, index, thisArg)) !!
The escape function signature is escape(string, attribute) so attribute will be truthy when index > 0
This is easy to forget, I don't even know how I notice that.
PR in https://github.com/ryansolid/dom-expressions/pull/444