preact-render-to-string icon indicating copy to clipboard operation
preact-render-to-string copied to clipboard

HTML Entity encoding in Script Tags

Open frenetic43 opened this issue 1 year ago • 5 comments

Background

I'm new to both 'preact' and 'preact-render-to-string' (and htm) and was surprised to find the contents of my script tag were automatically being entity encoded, which unfortunately breaks it.

Expected Output (Non-Encoded):

<script type="importmap">
  { "imports": {
    "lazyload": "./lazyload.js" }
  }
</script>

<script type="module">
  const iMap = { "imports": {
    "lazyload": "./lazyload.js" }
  };
</script>

Actual Output (Encoded):

<!-- Browser Error: "Could not parse Json" -->
<script type="importmap">
  { &quot;imports&quot;: {
    &quot;lazyload&quot;: &quot;./lazyload.js&quot; }
  }
</script>

<!-- Browser Error: "Unexpected token '&'" at '&'quot; -->
<script type="module">
  const iMap = { &quot;imports&quot;: {
    &quot;lazyload&quot;: &quot;./lazyload.js&quot; }
  };
</script>

Since any content within a script tag is parsed by the appropriate scripting-engine rather than the html-parser and thus doesn't need to be entity escaped, it might be convenient to exclude the 'script' tag from automagical encoding.

NOTE: I'm not sure if this affects non-SSR preact. I haven't gotten far enough to try client-side rendering yet.

Research

It looks like it would be trivial to implement in src/index.js: Around Line 470 add something like...

else if (typeof children === 'string') {
  if (type === 'script') { html = children; }
  else { html = encodeEntities(children); }
}

However, I also see around line 148 'Text VNodes: escape as HTML' and I don't know enough about preact to know how that would be triggered.

Fortunately, by looking at the source code I did find dangerouslySetInnerHTML which solves my immediate problem.

Discussion

I'm not sure what the best solution here would be.

In-my-own-opinion, modern web-apps have grown so massive that the use of inline-scripting (rather than loading an external script file) is a pretty fringe-case. I'm only using it myself because I'm still in the early-hacky-days of my current project.

Further, enforcing the use of dangerouslySetInnerHTML does provide a certain additional level of security against accidental... stuff. Though it could be better documented, I had to dig through the source-code to find it.

On the other hand, web technologies are rapidly expanding with new use-cases; 'import maps' being an easy example. And using dangerouslySetInnerHTML on an element that doesn't technically need to be entity escaped to begin with... well, it just isn't 'pretty'.

I think that's actually what bothers me the most, my code isn't as pretty anymore.

frenetic43 avatar Feb 06 '24 02:02 frenetic43