iddan / react-spreadsheet

Simple, customizable yet performant spreadsheet for React
https://iddan.github.io/react-spreadsheet
MIT License
1.34k stars 157 forks source link

Example Controlled Spreadsheet does not work #351

Closed Aaron-Hunter closed 10 months ago

Aaron-Hunter commented 1 year ago

The example Controlled spreadsheet code does not work. It will throw errors because 'd' (within onChange) is not defined. https://iddan.github.io/react-spreadsheet/docs/usage#Controlled

Inline with this, I can't get the Controlled spreadsheet to work properly. I have set up a code sandbox modeled on the example so that it doesn't throw errors, but have not been able to get onChange working. How should I write onChange so that changes in the spreadsheet are saved in the data? Edit: I've fixed the code sandbox so that the data updates using setData

https://codesandbox.io/s/sad-sound-txjvtt?file=/src/App.js

Boooober commented 1 year ago

Actually, there is another problem hidden in the example of Controlled spreadsheet. When you pass data={data} from useState hook and onChange={setData} from the same hook, this results in an infinite render loop.

I have added console.log to the example above, https://codesandbox.io/s/inifinite-render-pk5k7v Just edit spreadsheet somewhere and check the console.

If someone is looking for a workaround, here is a bit dirty, but working solution:

const dataRef = useRef<Matrix<CellBase<string | undefined>>>([]);
const [, forceRerender] = useState(0);
const [initialData, setInitialData] = useState([
  [{ value: "Vanilla" }, { value: "Chocolate" }, { value: "" }],
  [{ value: "Strawberry" }, { value: "Cookies" }, { value: "" }]
]);

const handleChange = (data: Matrix<CellBase<string | undefined>>) => {
  dataRef.current = data;
  forceRerender((i) => ++i);
};

console.log(dataRef.current); // Safe to use

return <Spreadsheet data={initialData} onChange={handleChange} />;
iddan commented 1 year ago

I'll try getting around this when I have some time. In the midtime, PRs are welcome.

Aaron-Hunter commented 1 year ago

@Boooober that workaround doesn't allow us to set the data of the spreadsheet programmatically after initialisation right? I am trying to create a render safe spreadsheet and have a button that allows upload of data to the spreadsheet, and haven't managed to accomplish them together.

Boooober commented 1 year ago

@Aaron-Hunter You are right. I went through my workaround and came up with something like checking the objects for equality before updating. You can do such things using lodash or JSON.stringify.

Here we can even remove the hack with the useRef hook and force rerender. But consider the cases where checking for object equality can affect performance.

https://codesandbox.io/s/inifinite-render-zjw83z?file=/src/App.js

export default function App() {
  const [data, setData] = useState([
    [{ value: "Vanilla" }, { value: "Chocolate" }, { value: "" }],
    [{ value: "Strawberry" }, { value: "Cookies" }, { value: "" }]
  ]);

  const handleChange = (newData) => {
    if (JSON.stringify(data) !== JSON.stringify(newData)) {
      setData(newData);
    }
  };

  const handleAddRow = () => {
    handleChange([
      ...data,
      [
        { value: Math.random().toString(32).slice(2) },
        { value: Math.random().toString(32).slice(2) }
      ]
    ]);
  };

  return (
    <div>
      <Spreadsheet data={data} onChange={handleChange} />
      <p>{JSON.stringify(data)}</p>
      <button onClick={handleAddRow}>Add row programatically</button>
    </div>
  );
}
iddan commented 10 months ago

Updated the code in the docs. Thank you!

nickiepucel commented 9 months ago

I'm still seeing the same infinite render loop that @Boooober described when I follow the example in the Controlled section.

My code:

const [data, setData] = useState<Matrix<CellBase<string>>>([[{ value: "" }]]);
return (
  <Spreadsheet
    data={data}
    onChange={(data) => {
      // this is printed infinitely after typing one character into the spreadsheet
      console.log("spreadsheet onChange called"); 
      setData(data);
    }}
  />
);

The proposed workaround also worked for me, though I still wanted to be able to fully control the component.

In order to do that, I used this workaround: verify that the data has changed before calling setData. This breaks the infinite rerendering loop.

const [data, setData] = useState<Matrix<CellBase<string>>>([[{ value: "" }]]);

const handleChange = (newData: Matrix<CellBase<string>>) => {
  if (JSON.stringify(data) !== JSON.stringify(newData)) {
    setData(newData);
  }
};

return (
  <Spreadsheet data={data} onChange={handleChange} />
);