1

I would like to pass data (which is saved as a state) to a react component that graphs that data. That graph should also be able to filter the data.

The data is a nested object structured as follows.

{
  "mylog": {
    "entries": [
      {"Bool1": false, "Bool2": true, ...},
      {"Bool1": true, "Bool2": true, ...},
      ...
    ]
  },
  "another_log": {...},
  ...
}

My approach has been to define a state called filteredData within the graph component, set it to the data passed to the graph, and then update the filteredData state when I want to filter the data.

function App(props) {
  const [data, setData] = useState({...}); // Initial data here

  return (
    <div>
      <Graph data={data} />
    </div>
  );
}

function Graph(props) {
  const [filteredData, setFilteredData] = useState(props.data);

  const filter = () => {
    setFilteredData(data => {
      ...
    });
  }

  return (
    ...
  );
}

However, when filteredData gets filtered, data in the App component also gets filtered (and that breaks things). I've tried substituting {..props.data} for props.data in a couple of places, but that hasn't fixed it. Any ideas? Thanks in advance.

Here is a minimum, reproducible example: https://codesandbox.io/s/elastic-morse-lwt9m?file=/src/App.js

Simon Richard
  • 43
  • 1
  • 6

3 Answers3

2

The fact that updating the local state is mutating the prop actually tells us that you're mutating state as well.

data[log].entries = in your filter is the offender.

const filter = () => {
  setFilteredData((data) => {
    for (const log in data) {
      data[log].entries = data[log].entries.filter((s) => s.Bool1);
//    ^^^^^^^^^^^^^^^^^^^ Here is the mutation
    }
    return { ...data }; // Copying data ensures React realizes
    // the state has been updated (at least in this component).
  });
};

The return { ...data } part is also a signal that the state is not being updated correctly. It is a workaround that "fixes" the state mutation locally.

You should make a copy of each nested array or object before modifying it.

Here is an option for correcting your state update which will also solve the props issue.

setFilteredData((data) => {
  const newData = {...data};

  for (const log in data) {
    newData[log] = { 
      ...newData[log],
      entries: data[log].entries.filter((s) => s.Bool1)
    }
  }

  return newData;
});

Running example below:

const {useState} = React;

function App() {
  const [data, setData] = useState({
    mylog: {
      entries: [{ Bool1: false }, { Bool1: true }]
    }
  });

  return (
    <div>
      <h3>Parent</h3>
      {JSON.stringify(data)}

      <Graph data={data} />
    </div>
  );
}

function Graph(props) {
  const [filteredData, setFilteredData] = useState(props.data);

  const filter = () => {
    setFilteredData((data) => {

      const newData = {...data};

      for (const log in data) {
        newData[log] = { 
          ...newData[log],
          entries: data[log].entries.filter((s) => s.Bool1)
        }
      }
      return newData;
    });
  };

  return (
    <div>
      <h3>Child</h3>
      <button onClick={filter}>Filter</button>

      {JSON.stringify(filteredData)}
    </div>
  );
}

ReactDOM.render(<App />, document.getElementById('root'));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.4/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.4/umd/react-dom.production.min.js"></script>
<div id="root"></div>
Brian Thompson
  • 13,263
  • 4
  • 23
  • 43
0

I faced a similar issue where my state's initial value is being set from a const object. Same object is being used in multiple components to set the initial value of each component's state. Problem occurs when a component updates the state, this also updates the constant object, since this object is used in other components the initial value of this other component is set to the modified values.

const DefaultValues: Flags = {isLoading: false, error: null, result: null}

const ComponentA () => {
    const [flags, setFlags] = useState<Flags>(DefaultFlags);

    const updateState = () => {
        setFlags({...flags, isLoading: true})
    }
}

const ComponentB () => {
    // DefaultValues: {isLoading: true, error: null, result: null}
    const [flags, setFlags] = useState<Flags>(DefaultFlags);

    const updateState = () => {...}
}

This is breaking the behaviour of ComponentB

Prateek Jha
  • 136
  • 2
  • 4
-1

Primitives, such as integers or Strings are passed down by their value, while Object data-types such as arrays are passed down by their reference. Here in your example - data is by passed reference. which makes it mutable.

In React - props should be immutable and top-down. This means that a parent can send whatever prop values it likes to a child, but the child cannot modify its own props. From ReactJS documentation

Whether you declare a component as a function or a class, it must never modify its own props.

One solution is is to pass a copy of your original data object.

<Graph data={JSON.parse(JSON.stringify(data))} />

Updated Codepen. You're still mutating props - not a good idea but it works.

Edit: JSON.stringify is NOT recommended due to it's issues with dates & non-primitive data types. Other ways to deep clone in JS - How to Deep clone in javascript

A G
  • 21,087
  • 11
  • 87
  • 112
  • 2
    Using `JSON` to serialize/deserialize data should only be a last resort if ***all other*** methods of copying/mutation avoidance are exhausted. This should pretty much ***never*** be done in production code. Its use is a huge code smell. – Drew Reese Jun 11 '21 at 19:19
  • Yes I am well aware. The question is not about how to correctly deep clone an object but related to mutability. I have updated the answer. I would not even recommend using `setFilteredData` as a state function, but that's another discussion. – A G Jun 11 '21 at 19:45
  • 1
    I didn't downvote, BTW, because you *did* technically answer the question; I just wanted to point out that your "one solution" isn't a good one and it just covers up the mutation. – Drew Reese Jun 11 '21 at 20:08