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

React component not interpreted as HTML when an attribute has an object value with multiple properties #1194

Closed
bryandowning opened this issue Apr 2, 2018 · 5 comments

Comments

@bryandowning
Copy link

Marked version: Broken in 0.3.17 and later

Markdown flavor: GitHub Flavored Markdown

This issue is caused by these two commits:
b15e42b
560c5cb

Expectation

A React component with a prop/attribute whose value is an object containing multiple properties should be treated as an html element. This used to work prior to the previously mentioned commits.

For example, this used to work, but no longer does:

<SomeReactComponent
  title="A React component with a prop accepting an object value with multiple properties."
  objectProp={{ prop1: 'val1', prop2: 'val2' }}
/>

NOTE: I'm using the marksy package which depends on marked.

Result

The code is not interpreted as HTML, and instead gets rendered as text.

What was attempted

This works:

<SomeReactComponent
  title="A React component with a prop accepting an object value with a single property."
  objectProp={{ prop1: 'val1' }}
/>

This works:

<SomeReactComponent
  title="A React component with a prop accepting a string."
/>

This does not work (missing required space after 'val1'):

<SomeReactComponent
  title="A React component with a prop accepting an object value with a single property."
  objectProp={{ prop1: 'val1'}}
/>

This does not work:

<SomeReactComponent
  title="A React component with a prop accepting an object value with multiple properties."
  objectProp={{ prop1: 'val1', prop2: 'val2' }}
/>

I've narrowed the problem down to this line of code: https://github.com/markedjs/marked/blob/v0.3.19/lib/marked.js#L58

Prior to the previously mentioned commits, this line:

/<tag(?:"[^"]*"|'[^']*'|\s[^'"\/>\s]*)*?\/?>/

used to be (note missing \s tokens):

/<tag(?:"[^"]*"|'[^']*'|[^'">])*?>/

Making the first whitespace token (added in 560c5cb) optional seems to fix the issue (although, I'm not sure if that is a proper fix):

/<tag(?:"[^"]*"|'[^']*'|\s?[^'"\/>\s]*)*?\/?>/

Not sure if this impacts the security fix introduced by #1083

NOTE: In order to find that line of code with the problem, I stepped through the code in the debugger until I hit this line: https://github.com/markedjs/marked/blob/v0.3.19/lib/marked.js#L356

...and noticed that it was failing the condition in that if statement.

@UziTech
Copy link
Member

UziTech commented Apr 2, 2018

I don't think JSX is something we want to support.

We are trying to become more spec compliant with CommonMark and they don't seem to support attribute objects. demo

I would expect as we become more spec compliant more of your examples will break.

You are welcome to extend marked with the instructions in our docs and create a markdown converter that works with JSX, but seeing as we are raising marked from the dead I don't think we will have time to support JSX any time soon.

@bryandowning
Copy link
Author

Thanks for the quick response @UziTech. I was afraid that was going to be the answer.

Did you leave this issue open thinking others might chime in?

@UziTech
Copy link
Member

UziTech commented Apr 3, 2018

Sorry going a little too fast yesterday

@UziTech UziTech closed this as completed Apr 3, 2018
@Feder1co5oave
Copy link
Contributor

Sorry @bryandowning, my curiosity:
Shouldn't jsx tags be in jsx files only? Why are you feeding them to marked?

@bryandowning
Copy link
Author

Yeah, good question. I'm using Contentful as a CMS for a website. Contentful allows you to define your own custom content types and fields. One of the field types you can use is a markdown field. So I have content editors logging in to Contentful to write markdown, which then appears on the site. I'm using marksy (which uses marked) to convert markdown string content into HTML. So basically, I have a component that looks like this:

<Markdown content={someMarkdownString} />

So the cool part is that marksy lets you white-list a set of React components that can be used in markdown. This is basically like short tags in Wordpress. So I can have a markdown string like this:

# Some title
This is some page that really needs a button to tie everything together.

<Button>My Cool Custom React Button</Button>

This has been an incredibly powerful feature, because as a developer, I can allow my editors to create some really dynamic content with just a markdown field.

Some of the components I've exposed are super simple (like Button), but others are a lot more dynamic and/or specific. For example, my editors wanted the ability to easily lay out a grid of images in the middle of some article content. No problem:

<ImageGrid
  items={[{
    image: 'https://images.contentful.com/some-path/image1.jpg',
    caption: 'One Quarter'
  }, {
    image: 'https://images.contentful.com/some-path/image2.jpg',
    caption: 'Two Quarters'
  }, {
    image: 'https://images.contentful.com/some-path/image3.jpg',
    caption: 'Three Quarters'
  }, {
    image: 'https://images.contentful.com/some-path/image4.jpg',
    caption: 'Four Quarters'
  }]}/>

How about an easy way to embed Vimeo and YouTube videos? Easy:

<Video url="https://vimeo.com/123456789" />

<Video url="https://www.youtube.com/watch?v=abcdefgh" />

The trick is that marksy relies on marked's Lexer to tokenize React components as HTML nodes. The only solution I can really think of would be to manually run the marked Lexer and sniff the resulting array to see if some React content got stuffed into a p token. That would require me to get a PR accepted by marksy or fork them — not to mention that solution just feels...wrong/janky/brittle. I guess I could fork marksy and marked, and just change the RegExes in marked myself, but that just feels...REALLY wrong.

So, needless to say, I'm now in a situation where I have a bunch of JSX in my CMS that relies on an outdated version of marked. I really have no idea what I'm going to do at this point, lol.

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

3 participants