Skip to content

key prop is ignored #19

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

Closed
erasmo-marin opened this issue Feb 2, 2017 · 13 comments
Closed

key prop is ignored #19

erasmo-marin opened this issue Feb 2, 2017 · 13 comments

Comments

@erasmo-marin
Copy link

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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

@alansouzati
Copy link
Collaborator

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
Copy link
Author

erasmo-marin commented Feb 6, 2017

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
Copy link
Collaborator

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
Copy link

gaearon commented Feb 6, 2017

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
Copy link
Author

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
Copy link

gaearon commented Feb 6, 2017

@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
Copy link
Author

erasmo-marin commented Feb 6, 2017

EDIT: element.key is not undefined, my mistake

@gaearon
Copy link

gaearon commented Feb 6, 2017

Could you explain the problem to me?

Key is not on props intentionally. This is explained in facebook/react#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
Copy link
Collaborator

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
Copy link
Collaborator

Fixed in a48363c.

Released 1.0.1 with support for this.

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