grommet / jsx-to-string

Parse your React JSX component to string
MIT License
124 stars 26 forks source link

key prop is ignored #19

Closed erasmo-marin closed 7 years ago

erasmo-marin commented 7 years ago

Hi, thanks for this work, I'm currently using it on a code Highlight component and works pretty well. But one of the problems I could find is that the key prop is removed. I need that to show my code examples so the user can just copy/paste the code without getting any warnings.

Is there any way to enable this?

You can see the problem here: https://erasmo-marin.github.io/suitup-toolkit-website/

alansouzati commented 7 years ago

Can you please provide an example on how you are trying to call jsx-to-string and what is the expected output. Sorry but I could not understand what the problem is.

erasmo-marin commented 7 years ago

The problem is that if you do something like this:

let c = <Component>
               <Item key={1}/>
               <Item key={2}/>
        </Component>;

console.log(jsxToString(c));

the output will be:

<Component>
    <Item/>
    <Item/>
</Component>

but should be:

<Component>
    <Item key={1}/>
    <Item key={2}/>
</Component>
alansouzati commented 7 years ago

Weird. thanks for sharing. let me try that locally.

alansouzati commented 7 years ago

I cannot seem to reproduce it. And even in your site I can see the props are there:

<Container>
  <Box rows={6}>
    <BoxChild wide={1}>
      <div style={{"backgroundColor": "red", "color": "white", "textAlign": "center"}}>
        Hola
      </div>
    </BoxChild>
    <BoxChild wide={2}>
      <div style={{"backgroundColor": "red", "color": "white", "textAlign": "center"}}>
        Soy
      </div>
    </BoxChild>
    <BoxChild wide={1}>
      <div style={{"backgroundColor": "red", "color": "white", "textAlign": "center"}}>
        una
      </div>
    </BoxChild>
    <BoxChild wide={2}>
      <div style={{"backgroundColor": "red", "color": "white", "textAlign": "center"}}>
        grilla
      </div>
    </BoxChild>
    <BoxChild wide={3}>
      <div style={{"backgroundColor": "red", "color": "white", "textAlign": "center"}}>
        con 6 rows
      </div>
    </BoxChild>
    <BoxChild wide={3}>
      <div style={{"backgroundColor": "red", "color": "white", "textAlign": "center"}}>
        y 6 Box.Child
      </div>
    </BoxChild>
  </Box>
</Container>
erasmo-marin commented 7 years ago

Yep, props are there but the key prop isn't. Use the chrome inspector + React plugin and you will see that every BoxChild has a key prop but it's not displayed. So the problem is not with all the props, but the key prop.

I will try to find out what's the problem, because if you couldn't reproduce it, maybe is related with dependencies versions.

alansouzati commented 7 years ago

I see the problem you are facing now.

key is coming straight from react and they hide it in the react props.

I cannot think about a good way of solving this with jsx-to-string. If you have any ideas on how to solve this please advise.

gaearon commented 7 years ago

To read the key from an element, just read it: element.key. It is not in props, but it is on the element (and that is how React reads it).

erasmo-marin commented 7 years ago

element.key is undefined and access to it is disabled, but maybe a setting can be added to define a prop name as an alternative to display the key.

It's not a clean solution but it's useful.

gaearon commented 7 years ago

@erasmo-marin

Access is not “disabled” to element.key. You’ll get a warning if you try to access element.props.key (that doesn’t exist), but <Foo key="123" />.key is "123".

Otherwise React itself wouldn’t be able to read it. 😉

erasmo-marin commented 7 years ago

EDIT: element.key is not undefined, my mistake

gaearon commented 7 years ago

Could you explain the problem to me?

Key is not on props intentionally. This is explained in https://github.com/facebook/react/issues/2429: you don't want to accidentally copy it over to another element, for example. It doesn't behave like a prop, and shouldn't be one.

But a library like jsx-to-string can read element.key without any issues.

alansouzati commented 7 years ago

Thanks @gaearon I will make the modification on jsx-to-string. Did not know that key is on the element so that is great I learned something new :)

alansouzati commented 7 years ago

Fixed in https://github.com/grommet/jsx-to-string/commit/a48363c81d4a448cdf1bccac5a3c986acbd3b29c.

Released 1.0.1 with support for this.