Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't escape HTML &-entities in literal text #176

Closed
broofa opened this issue Nov 28, 2020 · 10 comments
Closed

Don't escape HTML &-entities in literal text #176

broofa opened this issue Nov 28, 2020 · 10 comments

Comments

@broofa
Copy link

broofa commented Nov 28, 2020

See also developit/htm#76

According to the JSX docs, "You can insert HTML entities within literal text in JSX", but this does not appear to work because of how &'s are escaped in the encodeEntities() method.

Could this method be made to ignore &'s that are part of valid html character entities? I could put a PR together, but want to get a sense of how receptive you are to this. (And I suspect I may be missing other, important requirements.)

@JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock transferred this issue from preactjs/preact Nov 28, 2020
@developit
Copy link
Member

This actually is an HTM issue. JSX converts html-encoded entities to UTF8 (see example), it's not part of VDOM rendering.

Because of this, preact-render-to-string is never passed "a & b" and expected to render "a & b". That would be a mismatch for how JSXText works on the client, where html-encoded entities in text are rendered as-is. This is a little confusing, since obviously entities do get decoded, just as the link above shows this actually happens during JSX compilation.

I think developit/htm#76 describes this issue pretty well though. From a Preact standpoint, neither the client-side renderer nor the server-side renderer should be handling this, since both treat JSXText as string literals (to match all other VDOM libraries). It would be too strange/broken if <div>{"&"}</div> and <div>{"&amp;"}</div> rendered the same output.

~~

For context on the HTM issue: the reason this hasn't been done is because HTML entity decoding is disproportionately expensive for no material gain. Entity decoding can only be implemented using a character mapping that is larger than HTM and Preact combined (along with a decent bit of conversion logic). Here's the mapping Babel's JSX transform uses:
https://github.com/babel/babel/blob/328ef420a45d63a12434a07f939ff592e9832727/packages/babel-parser/src/plugins/jsx/xhtml.js

When running in a browser context, HTM could technically use the DOM to perform this conversion, which is slow but cacheable. Personally though, I'm not a huge fan of any of the approaches. There are two use-cases for HTM decoding HTML entities:

  1. to be a 1:1 compile target for JSX - this is already handled by the jsx-to-htm Babel transform, does entity decoding.
  2. using HTM as a general-purposes HTML-to-VDOM converter - something it just isn't built for.

For the second use-case, at some point I would like to extract the old HTM 1 codebase, which was built on DOMParser and actually set out to cater to handling the full spectrum of HTML use-cases. In the meantime, preact-markup is a decent approximation of the same approach (albeit slower and larger).

@shingo-nakanishi
Copy link

shingo-nakanishi commented Sep 5, 2021

I'm not very good at English, so I don't really understand what's going on, but I'm having trouble escaping the following results.

Code:

const outRender = render(html`<link href="https://fonts.googleapis.com/css2?family=Lora&display=swap" rel="stylesheet" />`)
console.log(outRender)

Result:

<link href="https://fonts.googleapis.com/css2?family=Lora&amp;display=swap" rel="stylesheet" />

Lora&display => Lora&amp;display

Is there an option to cancel the escaping?

My workaround:
https://stackoverflow.com/a/59609991/1979953

@rschristian
Copy link
Member

rschristian commented Apr 10, 2022

Should this be occurring in props too? For example,

render(h('div', { class: 'flex(& col)' }));

// Results in

<div class="flex(&amp; col)"></div>

which is rather unexpected.

Edit: Seems React does it too. Interesting. I thought props shouldn't be an issue.

@renhiyama
Copy link

any updates on this?

@torchsmith
Copy link

bump

@broofa
Copy link
Author

broofa commented Apr 3, 2023

[Closing out issues I authored that appear to be stagnant.]

@broofa broofa closed this as completed Apr 3, 2023
@torchsmith
Copy link

@broofa please re-open.

@broofa
Copy link
Author

broofa commented Apr 4, 2023

@torchsmith: No. This issue is 2+ years old and conversation petered out at [in essence], "Won't fix".

If you want this reopened, make the case for that to the maintainers. I've moved on to other things and have unsubscribed from this issue.

@brandon-xyzw
Copy link

brandon-xyzw commented Jul 24, 2023

I fixed it with: <style dangerouslySetInnerHTML={{ __html: myString }} />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants