From 96a06547d4765df2dc204df7cb990765111ba00f Mon Sep 17 00:00:00 2001 From: sonarly-bot Date: Thu, 21 May 2026 13:10:17 +0000 Subject: [PATCH] fix(front): prevent dashboard layout onLayoutChange feedback loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://sonarly.com/issue/39408?type=bug Editing a dashboard can crash the page with “Maximum update depth exceeded”, blocking further layout editing in-session. Fix: Implemented a guard in `usePageLayoutHandleLayoutChange` to prevent redundant state writes when `react-grid-layout` emits an `onLayoutChange` payload that is effectively identical to the current tab layout. What changed: - Added `areLayoutsEqual(firstLayouts, secondLayouts)` (JSON structural comparison). - In `handleLayoutChange`, before writing `pageLayoutCurrentLayoutsState`, compare `currentTabLayouts[activeTabId]` against `allLayouts`. - Only call `store.set(pageLayoutCurrentLayoutsState, ...)` when the layout actually changed. Why this fixes the loop: - The feedback loop came from unconditional `store.set` on every `onLayoutChange`, which can feed new props back into grid layout and retrigger `onLayoutChange`. - Skipping no-op writes breaks that self-sustaining render/update cycle for identical layout callbacks. Test added: - `usePageLayoutHandleLayoutChange.test.tsx`: new case `"should not rewrite tab layouts when new layout is identical"` asserts that a second identical `handleLayoutChange` call keeps the same stored layout reference for the tab (no rewrite). Validation attempted: - Ran: - `npx jest packages/twenty-front/src/modules/page-layout/hooks/__tests__/usePageLayoutHandleLayoutChange.test.tsx --config=packages/twenty-front/jest.config.mjs` - It failed due missing `ts-jest` in this environment, so I could not execute tests locally. Authored by Sonarly by autonomous analysis (run 44884). --- .../usePageLayoutHandleLayoutChange.test.tsx | 46 +++++++++++++++++++ .../hooks/usePageLayoutHandleLayoutChange.ts | 19 ++++++-- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/packages/twenty-front/src/modules/page-layout/hooks/__tests__/usePageLayoutHandleLayoutChange.test.tsx b/packages/twenty-front/src/modules/page-layout/hooks/__tests__/usePageLayoutHandleLayoutChange.test.tsx index 810fbf8f7f1..6c9cad932e4 100644 --- a/packages/twenty-front/src/modules/page-layout/hooks/__tests__/usePageLayoutHandleLayoutChange.test.tsx +++ b/packages/twenty-front/src/modules/page-layout/hooks/__tests__/usePageLayoutHandleLayoutChange.test.tsx @@ -129,4 +129,50 @@ describe('usePageLayoutHandleLayoutChange', () => { expect(Object.keys(result.current.layouts)).toHaveLength(0); }); + + it('should not rewrite tab layouts when new layout is identical', () => { + const { result } = renderHook( + () => ({ + handler: usePageLayoutHandleLayoutChange({ + pageLayoutId: PAGE_LAYOUT_TEST_INSTANCE_ID, + tabListInstanceId: getTabListInstanceIdFromPageLayoutId( + PAGE_LAYOUT_TEST_INSTANCE_ID, + ), + }), + layouts: useAtomComponentStateValue( + pageLayoutCurrentLayoutsComponentState, + PAGE_LAYOUT_TEST_INSTANCE_ID, + ), + setActiveTabId: useSetAtom( + activeTabIdComponentState.atomFamily({ + instanceId: `${PAGE_LAYOUT_TEST_INSTANCE_ID}-tab-list`, + }), + ), + }), + { + wrapper: PageLayoutTestWrapper, + }, + ); + + act(() => { + result.current.setActiveTabId('tab-1'); + }); + + const newLayouts = { + desktop: [{ i: 'widget-1', x: 1, y: 0, w: 2, h: 2 }], + mobile: [{ i: 'widget-1', x: 0, y: 0, w: 1, h: 2 }], + }; + + act(() => { + result.current.handler.handleLayoutChange([], newLayouts); + }); + + const firstStoredLayouts = result.current.layouts['tab-1']; + + act(() => { + result.current.handler.handleLayoutChange([], newLayouts); + }); + + expect(result.current.layouts['tab-1']).toBe(firstStoredLayouts); + }); }); diff --git a/packages/twenty-front/src/modules/page-layout/hooks/usePageLayoutHandleLayoutChange.ts b/packages/twenty-front/src/modules/page-layout/hooks/usePageLayoutHandleLayoutChange.ts index 2c992127635..ce4aaa49f90 100644 --- a/packages/twenty-front/src/modules/page-layout/hooks/usePageLayoutHandleLayoutChange.ts +++ b/packages/twenty-front/src/modules/page-layout/hooks/usePageLayoutHandleLayoutChange.ts @@ -10,6 +10,10 @@ import { useCallback } from 'react'; import { type Layout, type Layouts } from 'react-grid-layout'; import { isDefined } from 'twenty-shared/utils'; +const areLayoutsEqual = (firstLayouts: Layouts, secondLayouts: Layouts) => { + return JSON.stringify(firstLayouts) === JSON.stringify(secondLayouts); +}; + export const usePageLayoutHandleLayoutChange = ({ pageLayoutId: pageLayoutIdFromProps, tabListInstanceId, @@ -45,11 +49,18 @@ export const usePageLayoutHandleLayoutChange = ({ if (!isDefined(activeTabId)) return; const currentTabLayouts = store.get(pageLayoutCurrentLayoutsState); + const activeTabLayouts = currentTabLayouts[activeTabId]; - store.set(pageLayoutCurrentLayoutsState, { - ...currentTabLayouts, - [activeTabId]: structuredClone(allLayouts), - }); + const hasLayoutChanged = + !isDefined(activeTabLayouts) || + !areLayoutsEqual(activeTabLayouts, allLayouts); + + if (hasLayoutChanged) { + store.set(pageLayoutCurrentLayoutsState, { + ...currentTabLayouts, + [activeTabId]: structuredClone(allLayouts), + }); + } const pageLayoutDraft = store.get(pageLayoutDraftState);