visgl / react-map-gl

React friendly API wrapper around MapboxGL JS
http://visgl.github.io/react-map-gl/
Other
7.88k stars 1.35k forks source link

[Bug] "Cannot update a component while rendering a different component" error in `dataloading` event handler #2107

Open alex-kowalczyk opened 1 year ago

alex-kowalczyk commented 1 year ago

Description

When there is an event handler hooked to mapRef's dataloading event and this handler calls setState (say, for enabling spinning wheel feedback), the following "Warning"-level exception is dumped:

Warning: Cannot update a component (MyMap) while rendering a different component (Source). To locate the bad setState() call inside Source, follow the stack trace as described in https://reactjs.org/link/setstate-in-render

This seems to be caused by component calling updateSource during rendering. Maybe this should be an in-effect code instead.

Expected Behavior

Event handlers should allow updating state of other components.

Steps to Reproduce

Minimal example provided here. See "Console" tab for the actual error.

https://codesandbox.io/s/optimistic-monad-dyt59s?file=/src/index.js

Environment

Logs

Relevant stack trace fragment below.

    setTrue useBoolean.js:4   (*** state setter ***)
    dataloading (*** my event handler ***)
    fire maplibre-gl.js:535
    _fireEvent mapbox.js:488
    fire maplibre-gl.js:539
    fire maplibre-gl.js:539
    fire maplibre-gl.js:539
    _updateWorkerData maplibre-gl.js:15287
    setData maplibre-gl.js:15262
    updateSource source.js:39
    Source source.js:102
Pessimistress commented 1 year ago

I'm not sure what you expect this library to do about it. You are directly hooking into a maplibre event, not a React callback (e.g. onLoad, onClick). It is out of this library's control when that event is fired.

alex-kowalczyk commented 1 year ago

It might within library control, because the event is a direct result of calling updateSource within render function of Source component. https://github.com/visgl/react-map-gl/blob/master/src/components/source.ts#L127

One consideration which could avoid this user-confusing error is to move some of this code (source.ts:125-130) to useEffect(..., [source]) or similar:

  // @ts-ignore
  let source = map && map.style && map.getSource(id);
  if (source) {
    updateSource(source, props, propsRef.current);
  } else {
    source = createSource(map, id, props);
  }

I also found a workaround which might be useful for others. To avoid the error, I modified my dataloading event handler to call setState within inner _.defer() call. This way I "escape" from the rendering stage.

michaelcgorman commented 1 year ago

I am also noticing this on sourcedataloading events. I am avoiding the issue by wrapping my event handlers in setTimeout(() => handlerFunction(), 0).