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

Not closed tags are a problem #17

Closed
timsuchanek opened this issue Jun 25, 2017 · 8 comments
Closed

Not closed tags are a problem #17

timsuchanek opened this issue Jun 25, 2017 · 8 comments
Labels

Comments

@timsuchanek
Copy link
Contributor

I'm currently using react-jsx-parser with output of the remark parser. I just wanted to let you know, that the application/html+xml has more problems than expected.
It's pretty common that in html people forget to close tags like <img src='img.png' >.
With application/html+xml this result in all tags coming after that tag to be ignored.
What is your take on this? Should react-jsx-parser be that strict?
For me unfortunately it's not an option as I can't rely on the html being compliant.

@TroyAlford
Copy link
Owner

Argh... that's painful, indeed. :( Perhaps we should go back to XML parsing, then, to keep the case-sensitivity, and inject closing tags in some fashion, automatically?

@timsuchanek
Copy link
Contributor Author

Yep, I think that's a good idea. It should just be one .replace call with a regex, the only question is how that would perform on bigger html strings

@TroyAlford
Copy link
Owner

TroyAlford commented Jun 26, 2017 via email

@TroyAlford
Copy link
Owner

@timsuchanek - I have a test branch working now, using acorn and acorn-jsx to parse. Unfortunately, it takes this lib from 9kb to 80kb or so - and I haven't found any good way to shake the extra size so far...

Would love it if you could take a look at #19

@anders-kiaer
Copy link

Was this issue solved in #19? Tried the following snippet with react-jsx-parser == 1.21.0:

import JsxParser from 'react-jsx-parser'

const MyComponent = () => (
  <JsxParser
    jsx={`
      <h1>Header</h1>
      <img src="someimage.png">
      <span>Some other text</span>
    `}
  />
)

and the input html appears not to be rendered, unless changing the image element to

<img src="someimage.png" />

Seen downstream in plotly/dash-core-components#746.

@TroyAlford
Copy link
Owner

@anders-kiaer having a tag without either a closing tag, or an end of /> is not valid JSX, even though it's valid HTML. This is because HTML is an (unfortunately and detrimentally) overly forgiving standard. Because things like this cause complex parser problems, JSX doesn't allow for it.

To elucidate this, here's what happens when Acorn hits this kind of scenario:
image

i.e. - we don't even get far enough along to process the node - we simply get a syntax error.

So, unfortunately, to handle this situation, you need to ensure your snippets are valid JSX, not just valid HTML.

@anders-kiaer
Copy link

Thanks for a quick answer @TroyAlford and clarification of scope 🙂

So, unfortunately, to handle this situation, you need to ensure your snippets are valid JSX, not just valid HTML.

No problem 👍 We use react-jsx-parser indirectly downstream, through dash, where the user controls the (markdown + html) input. The user input goes through Mozilla's bleach sanitizer, which (unfortunately) replaces <img ... /> with <img ...>. Our "workaround" is to simply run regex on the sanitized bleach output, before going further to dash / react-jsx-parser, which works quite nice (since the input is already sanitized/cleaned). We will just mark the "workaround" as a more permanent thing. 🙂 Thanks again. 👏

@TroyAlford
Copy link
Owner

Ooh, yeah that makes sense. And yep - that sounds like it's needed. :) Perhaps you could share your workaround snippet?

I do already mirror which JSX components are void, the same way the React library does. If I had a clean, concise snippet of code to run, I could pretty easily "pre-parse" JSX to auto-fix all void element types (such as img). Then this could work ootb again for you and everyone else :)

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

No branches or pull requests

3 participants