Onchange function getting triggered on each and every component render

[CJ190795]

Hey,
I am using react excalidraw package. I am trying to do collaboration feature for 2 people. Both the people will share the below component. Let's say person A and person B. If person A changes anything in the whiteboard, it will inturn trigger onChange function, which will send data over socket and will be recieved by person B in the useEffect where I am calling updateScene API.
The only problem is I am running into infinite loop because onChange function is getting triggered by itself on each and every component render. Am I missing something?

This is what my component returns :

  return (
<Excalidraw
          ref={props.innerChildRef}
          width={width}
          height={height}
          initialData={InitialData}
          onChange={onChange}
        />
)

This is the onChange function :

const onChange = (elements, state) => {
    console.log("onChange:::::::::::::::::::::::::::::::::::: :", props.role, elements, "State : ", state);
    props.sendWhiteboardSocket(elements)
  };
  

This is my useEffect for updating elements upon receiving elements prop :

useEffect(() => {
  if (props.elements && props.innerChildRef && props.innerChildRef.current) {
    let newElements = reconcileElements(props.elements)
    let sceneData = {"elements" : newElements}
    props.innerChildRef.current.updateScene(sceneData);
  }
}, [props.elements, props.innerChildRef]);

[CJ190795]

@ad1992 @dwelle Could you please help me here?

[ad1992]

@CV1928 yes currently onChange gets triggered everytime comp updates but this shouldn't happen. Will make the change so that it gets called only on elements or appState change.
As a workaround, you can cache the version you get using getSceneVersion and check if the version has changed and perform the actions accordingly

[dwelle]

Will make the change so that it gets called only on elements or appState change.

I don't think we should be doing that just yet. It'll slow down the update loop, especially when we rewrite the getSceneVersion to account for appState, and change the algo to be more resilient (not simply summing up element versions).

[ad1992]

@dwelle didn't get you. For host triggering onChange callback would make sense only when elements/appstate changes right ? when do you think onChange will be needed to be called apart from these cases?

[dwelle]

Of course it shouldn't. What I'm pointing out that it'll impact perf of:

1. excalidraw.com (unless we do it inside the package).
2. Potentially even the host app, because the host apps may want to do their own diffing. For example, if they debounce whatever they're doing with the changes, there's no point in resolving the changes until needed. Or they may not care about some changes: in excalidraw.com, the getVersion() is super simple right now, but soon we'll want to migrate to a more involved algo (resolve appState...). Host apps may not need that granularity. Also, they may not care about deleted elements, while we do.

If the change resolving was free, then I'd say let's do it. That said, my intuition may be off, so we should measure it — but we should measure the soon-to-be resolver, not what we do right now (summing up element versions).

[ad1992]

AFAIU onChange should not affect the perf of the host and since we are passing appState and elements so unless any of those change the callback is not useful and gets triggered unnecessarily so we can try optimizing that.
Even if we include appState in versioning still we would expect a change in Appstate/elements so it can be helpful.

[CJ190795]

@ad1992 Any tentative time by when you can do this change. Caching also is a problem. I am having n instances of excalidraw on my webpage. Maintaining cached state for all of them is actually a headache. Is there anything else I can do here?

[ad1992]

Completely optimizing onChange would take some time as we need to figure out what all cases we need not call onChange callback. But we will roll out the optimization in patches very soon. Since you are maintaining multiple instances of Excal so you would be having some kind of mapping among the instances right so you can use that to maintain version cache mapping in memory ?

[CJ190795]

@ad1992 @dwelle Suppose I am saving excalidraw 'elements and appState' in Database and on page refresh, I am fetching that state to updateScene in useEffect. How do I make sure that the 'Excali instance is loaded' and saved in 'excalidrawRef' before I invoke updateScene Api of Excali?
I mean useEffect can't listen to 'excalidrawRef' being updated, right?

useEffect(()=>{
    console.log(excalidrawRef.current) //null when useEffect runs due to 'value' change on page refresh
    if(excalidrawRef.current){
      let objectValue = JSON.parse(value) //parsed stringified value saved to db : {elements:[], appState:{}}
      excalidrawRef.current.updateScene(objectValue);
    }
  }, [value, excalidrawRef.current])

By the time excalidrawRef is updated, useEffect due to value has already been executed. And Since excalidrawRef.current is null at that time, It wouldn't update the scene. Now even if excalidrawRef.current is updated after the component returns JSX, it wouldn't again go to useEffect again. So scene couldn't be updated at all.

[dwelle]

You can do two things:

1. subscribe to excalidrawRef.current.readyPromise(callback)
2. use a callbackRef instead: it's what we're doing for excalidraw.com (with a bit of sugar) ↓

   excalidraw/src/excalidraw-app/index.tsx

   Lines 168 to 171 in 9dd2257

   	const [
   	excalidrawAPI,
   	excalidrawRefCallback,
   	] = useCallbackRefState<ExcalidrawImperativeAPI>();

[siliconjungle]

I'm having trouble with this as well, especially if the elements get set back to []. Is there a more robust way to expose versioning information?

[kiraaziz]

i do have the same problem
i had a useEffect function that run each time onChange run but the problem is I do change a state in the useEffect so each time chnage the onChange rerun so the useEffect rerun so I get a infinty loop that make my app crash , so i fix it this way .
the useEffect run undependant on the onChange and run each 50 ms so I thing that sole the probelm
`js
useEffect(() => {
const intervalId = setInterval(() => {
setAppState()
setElements()
setSelectedElements()
setSelectedTypes()
}, 50);

    return () => clearInterval(intervalId)
}, [])

`

[luckyboykg]

Do we have any updates on this issue?