vaadin / react-components

15 stars 4 forks source link

Maximum update depth exceeded with AutoGrid columnRendering set to lazy #255

Closed sagarbendale closed 1 week ago

sagarbendale commented 5 months ago

I see this error on AutoGrid. Whenever I have columnRendering set to lazyI see this error. If I change it back to eagerthis error will go away.

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn’t have a dependency array, or one of the dependencies changes on every render.

console.error @ list:5 r9.error @ useRenderer.ts:60 printWarning @ react-dom.development.js:86 error @ react-dom.development.js:60 checkForNestedUpdates @ react-dom.development.js:27339 scheduleUpdateOnFiber @ react-dom.development.js:25514 dispatchReducerAction @ react-dom.development.js:16633 (anonymous) @ useRenderer.ts:62 flushSync @ react-dom.development.js:26228 flushSync$1 @ react-dom.development.js:29878 (anonymous) @ useRenderer.ts:62 _runRenderer @ vaadin-grid-column-mixin.js:612 (anonymous) @ vaadin-grid-column-mixin.js:643 __renderCellsContent @ vaadin-grid-column-mixin.js:626 _renderBodyCellsContent @ vaadin-grid-column-mixin.js:709 _onRendererOrBindingChanged @ vaadin-grid-column-mixin.js:714 runMethodEffect @ property-effects.js:1038 runEffects @ property-effects.js:140 _propertiesChanged @ property-effects.js:1922 _flushProperties @ properties-changed.js:384 _invalidateProperties @ property-effects.js:1748 Object.defineProperty.set @ properties-changed.js:170 (anonymous) @ vaadin-grid-mixin.js:547 _createScrollerRows @ vaadin-grid-mixin.js:545 _createPool @ virtualizer-iron-list-adapter.js:408 _increasePoolIfNeeded @ iron-list-core.js:434 _increasePoolIfNeeded @ virtualizer-iron-list-adapter.js:724 flush @ debounce.js:124 (anonymous) @ debounce.js:159 flushDebouncers @ debounce.js:157 flush @ debounce.js:172 set size @ virtualizer-iron-list-adapter.js:355 set size @ virtualizer.js:49 _flatSizeChanged @ vaadin-grid-mixin.js:300 runMethodEffect @ property-effects.js:1038 runEffects @ property-effects.js:140 _propertiesChanged @ property-effects.js:1922 _flushProperties @ properties-changed.js:384 _invalidateProperties @ property-effects.js:1748 Object.defineProperty.set @ properties-changed.js:170 _onDataProviderPageReceived @ vaadin-grid-data-provider-mixin.js:348 _onDataProviderPageReceived @ vaadin-grid-array-da…rovider-mixin.js:45 callback @ data-provider-controller.js:254 load @ data-provider.ts:91 await in load (async) loadCachePage @ data-provider-controller.js:265 loadFirstPage @ data-provider-controller.js:215 _ensureFirstPageLoaded @ vaadin-grid-data-provider-mixin.js:440 _dataProviderChanged @ vaadin-grid-data-provider-mixin.js:426 runObserverEffect @ property-effects.js:231 runEffects @ property-effects.js:140 _propertiesChanged @ property-effects.js:1922 _flushProperties @ properties-changed.js:384 _invalidateProperties @ property-effects.js:1748 Object.defineProperty.set @ properties-changed.js:170 (anonymous) @ autogrid.tsx:412 setTimeout (async) (anonymous) @ autogrid.tsx:395 commitHookEffectListMount @ react-dom.development.js:23189 commitPassiveMountOnFiber @ react-dom.development.js:24965 commitPassiveMountEffects_complete @ react-dom.development.js:24930 commitPassiveMountEffects_begin @ react-dom.development.js:24917 commitPassiveMountEffects @ react-dom.development.js:24905 flushPassiveEffectsImpl @ react-dom.development.js:27078 flushPassiveEffects @ react-dom.development.js:27023 performSyncWorkOnRoot @ react-dom.development.js:26115 flushSyncCallbacks @ react-dom.development.js:12042 flushSync @ react-dom.development.js:26240 flushSync$1 @ react-dom.development.js:29878 (anonymous) @ useRenderer.ts:62 _runRenderer @ vaadin-grid-column-mixin.js:612 (anonymous) @ vaadin-grid-column-mixin.js:643 renderCellsContent @ vaadin-grid-column-mixin.js:626 _renderHeaderCellContent @ vaadin-grid-column-mixin.js:672 _onHeaderRendererOrBindingChanged @ vaadin-grid-column-mixin.js:680 runMethodEffect @ property-effects.js:1038 runEffects @ property-effects.js:140 _propertiesChanged @ property-effects.js:1922 _flushProperties @ properties-changed.js:384 _invalidateProperties @ property-effects.js:1748 Object.defineProperty.set @ properties-changed.js:170 (anonymous) @ vaadin-grid-mixin.js:735 _updateRow @ vaadin-grid-mixin.js:669 (anonymous) @ vaadin-grid-mixin.js:900 iterateChildren @ vaadin-grid-helpers.js:26 _renderColumnTree @ vaadin-grid-mixin.js:899 _columnTreeChanged @ vaadin-grid-mixin.js:842 runMethodEffect @ property-effects.js:1038 runEffects @ property-effects.js:140 _propertiesChanged @ property-effects.js:1922 _flushProperties @ properties-changed.js:384 _invalidateProperties @ property-effects.js:1748 Object.defineProperty.set @ properties-changed.js:170 _updateColumnTree @ vaadin-grid-dynamic-columns-mixin.js:112 (anonymous) @ vaadin-grid-dynamic-columns-mixin.js:94 (anonymous) @ debounce.js:84 microtaskFlush @ async.js:35 (anonymous) @ async.js:179

tomivirkki commented 4 months ago

Related https://vaadin.com/forum/t/autogrid-vaadin-grid-needs-the-total-number-of-items-and-maximum-update-depth-exceeded/166507/3

vursen commented 2 weeks ago

It was not easy to reproduce this issue. While it's indeed tied with synchronous rendering as Tomi suggested, it's also related to the <Select> component. This component calls requestContentUpdate within useEffect:

https://github.com/vaadin/react-components/blob/cf253587e88c829888958a8fe0d91663953b1737/packages/react-components/src/Select.tsx#L51-L55

With synchronous rendering, this effect runs every time grid re-renders any renderer, which sometimes (not in all grid configurations) leads to React interpreting it as a possible infinite loop:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn't have a dependency array, or one of the dependencies changes on every render.
vursen commented 2 weeks ago

Simply wrapping renderers in flushSync doesn't guarantee that the DOM will be updated synchronously. React may still defer it to a micro task if flushSync was called while rendering was already in progress, which is explained in the warning that is currently suppressed:

Warning: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.

When there are many deferred sync renderers (GridColumn) that in turn trigger some async renderers (Select), it can create race conditions that React misinterprets as a possible infinite loop:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn’t have a dependency array, or one of the dependencies changes on every render.

Since synchronous rendering isn't guaranteed, it pretty much defeats the purpose of using flushSync in renderers. I think we should rather try to improve web components to support async renderers.