10

I'm getting that error, but I'm defining a key. Here's my App.js that it's complaining about.

import React from 'react';
import Relay from 'react-relay';
import AccountTable from './AccountTable';

class App extends React.Component {
  render() {
    return (
      <div>
        <h1>Account list</h1>
          {this.props.viewer.accounts.edges.map(edge =>
            <AccountTable key={edge.node.id} account={edge.node} />
          )}
      </div>
    );
  }
}

export default Relay.createContainer(App, {
    fragments: {
        viewer: () => Relay.QL`
            fragment on User {
                accounts(first: 10) {
                    edges {
                        node {
                            ${AccountTable.getFragment('account')}
                        }
                    }
                }
            }
        `,
    },
});
jonathancardoso
  • 9,319
  • 6
  • 49
  • 63
LoneWolfPR
  • 3,789
  • 12
  • 43
  • 78
  • 3
    bob ross would be proud of your code mountains. But really you should check the uniqueness of your edge.node.id's. – aaaaaa Dec 16 '16 at 16:58
  • do the usual sanity checks I guess. Is the above code the 100% source of the error? are the ids actually unique? maybe you have a couple of undefineds / nulls – azium Dec 16 '16 at 16:59
  • See if you have duplicate IDs: `console.log(this.props.viewer.accounts.edges.map(edge => edge.node.id))` – sdgluck Dec 16 '16 at 17:33
  • 1
    So, it turns out the edge.node object does not have an id defined at this point. I think it's because the node references another fragment. I didn't have this problem when I had accounts just spelled out inside this fragment. This showed up when I broke the account graphql stuff out into it's own fragment. Do any of you know a work around? – LoneWolfPR Dec 16 '16 at 18:39

2 Answers2

10

The easiest way to correct this is to base the key off the index of the map:

{this.props.viewer.accounts.edges.map((edge, i) =>
    <AccountTable key={i} account={edge.node} />
)}

Then, you don't have to worry about how unique the value of edge.node.id is. The key only needs to be unique in the context of all the AccountTable siblings. It doesn't need to be globally unique. So, the index works well.

However, if you have a stable id that is based off of the object, that is obviously better.

Davin Tryon
  • 62,665
  • 13
  • 135
  • 126
  • According to the react documentation: "When you don't have stable IDs for rendered items, you may use the item index as a key as a last resort". So, I'd like to make sure there's no way to get a stable id before I do this. Any suggestions on that front, or is this just not possible because my node references a different fragment? – LoneWolfPR Dec 16 '16 at 18:42
  • Well, unless you do have another id, this might be your "last resort", right? – Davin Tryon Dec 16 '16 at 18:43
  • It certainly fixes the problem. I'm going to do a bit more digging, but if I can't find a way to get the id of the object I'll mark this as the solution. Thanks! – LoneWolfPR Dec 16 '16 at 18:47
  • If you use the item index as the key, it may not work in cases when the array is re-ordered: https://stackoverflow.com/a/43892905/960857 – Chris Nov 02 '17 at 00:54
0

The solution I use is to extract the underlying id of relay fragment as key

{this.props.viewer.accounts.edges.map((edge) =>
    edge && e.node && <AccountTable key={edge.node.__id} account={edge.node} />
)}
Nathaniel
  • 86
  • 1
  • 3