react-bootstrap-table / react-bootstrap-table2

Next Generation of react-bootstrap-table
https://react-bootstrap-table.github.io/react-bootstrap-table2/
MIT License
1.27k stars 431 forks source link

Circular references in data leads to crash #382

Open machour opened 6 years ago

machour commented 6 years ago

Hi and thank you for this amazing library!

I came across a little issue when using data constructed using denormalize() from the normalizr library.

Considering the following data structure:

Contact {
  id: int
  name: string
  worksAt: [Contact],
  members: [Contact]
}

If "user" works at "company" (both Contact instances), then the structure output from denormalize will look like this:

{
  id: 1,
  name: 'user',
  worksAt: [
    {
      id: 2,
      name: 'company',
      worksAt: [],
      members: [
        {
          id: 1,
          name: 'user',
          worksAt: [
            {
              id: 2,
              name: 'company',
              worksAt: [],
              members: [
                {
                  id: 1,
                  name: 'user',
                  worksAt: [
                              //
                              // and so on..
                              //
                  ],
                },
              ],
            },
          ],
          members: [],
        },
      ],
    },
  ],
  members: [],
}

It have a circular reference, which is the behavior documented by Normalizr.

The problem is, that in react-bootstrap-table, right here, the following code is used, probably to clone the data array:

JSON.parse(JSON.stringify(data))

which causes the following error:

Uncaught TypeError: Converting circular structure to JSON
    at JSON.stringify (<anonymous>)
    at Store.set (index.js:80)

Is cloning the data variable the only thing that JSON.parse(JSON.stringify()) is trying to achieve? If so, maybe using Object.assign({}, data} could be better performance wise (and would make my error go away).

Happy to submit a PR if this is accepted!

mindvr commented 6 years ago

@machour, Object.assign() only makes a shallow copy, while the component is intended to work with arbitrary nested objects.

machour commented 6 years ago

Indeed @mindvr, plus we need data to be an array. [...data] would also make a shallow copy..

Any idea how to get the best of both worlds?

AllenFang commented 6 years ago

Correct by @mindvr

The only way is react-bootstrap-table need to leverage on lodash or underscore, these lib seems like provide some deep copy functionality.

I will think it more about this issue, thanks