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

Table shorthand produces key warning if table cell content repeats #4034

Open
CFrez opened this issue Aug 15, 2020 · 3 comments · May be fixed by #4198
Open

Table shorthand produces key warning if table cell content repeats #4034

CFrez opened this issue Aug 15, 2020 · 3 comments · May be fixed by #4198

Comments

@CFrez
Copy link

CFrez commented Aug 15, 2020

While using the table shorthand, if there is repeating data between two different columns/cells in the same row, then it triggers the react duplicate key warning. The row itself is given a key, but the cell is automatically generated by the content.

As far as I can tell this is a different issue than #1205 which was for the table row itself. It might be the same as #1030, but that was closed with the assumption that it was addressed by #599.

I am not sure if any of these took into the consideration the possibility of the data itself matching, so I thought it was important to report. If there is matching data on different rows, then the warning is not triggered.

Starting with the standard table example I modified the data slightly to have two of the data cells be an exact match.

const tableData = [
  {
    name: undefined,
    status: "repeat",
    notes: "repeat",
    key: "1"
  },
  {
    name: "Jimmy",
    status: "Requires Action",
    notes: undefined,
    key: "2"
  },
  {
    name: "Jamie",
    status: undefined,
    notes: "Hostile",
    key: "3"
  },
  {
    name: "Jill",
    status: undefined,
    notes: undefined,
    key: "4"
  }
];

Simplest form of renderBodyRow causes the duplicate key warning to appear:

const renderBodyRow = ({ name, status, notes, key }) => ({
    key: key,
    warning: !!(status && status.match("Requires Action")),
    cells: [
        name || "No name specified", 
        status || "Unknown", 
        notes || "None"
    ]
});

If the content of the cell is modified then the error will not be triggered:

const renderBodyRow = ({ name, status, notes, key }) => ({
    key: key,
    warning: !!(status && status.match("Requires Action")),
    cells: [
        name || "No name specified",
        <Header icon="warning" content={status} /> || "Unknown",
        notes || "None"
    ]
});

If both of the cells have the same content then again the warning is triggered:

const renderBodyRow = ({ name, status, notes, key }) => ({
    key: key,
    warning: !!(status && status.match("Requires Action")),
    cells: [
          name || "No name specified",
          status ? <Header icon="warning" content={status} /> : "Unknown",
          notes ? <Header icon="warning" content={notes} /> : "none",
    ]
});

Since the table cells within a row are siblings, I believe that the error itself is accurate. Can the automatically generated key possibly include the header row or something else that will ensure it is unique? This would allow for using the most basic option for renderBodyRow.

react 16.8.0
react-dom 16.8.0
semantic-ui-react 1.2.0

example created:
https://codesandbox.io/s/suir-table-shorthand-keys-2t2bt

@welcome
Copy link

welcome bot commented Aug 15, 2020

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

@Iceberg1109
Copy link

Hi, @RedFrez

You are right, This should be fixed.

I think on line 62 of TableRow.js, {_.map(cells, (cell) => TableCell.create(cell, { defaultProps: { as: cellAs } }))} is wrong.
This should have the defaultProps - key.

@jhns88
Copy link

jhns88 commented Apr 13, 2021

I will try to fix the problem.

Oh damn, first timer in an open source project =)

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

Successfully merging a pull request may close this issue.

3 participants