Compare commits

...
Author SHA1 Message Date
sonarly-bot 96a06547d4 fix(front): prevent dashboard layout onLayoutChange feedback loop
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).
2026-05-21 13:10:17 +00:00
2 changed files with 61 additions and 4 deletions
@@ -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);
});
});
@@ -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);