Skip to content

fix: synchronize controlled layout before child render #318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daniil-filippov-dev merged 1 commit into from
Sep 3, 2026
+164 −22
Merged

fix: synchronize controlled layout before child render #318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 112 additions & 5 deletions src/components/DashKit/__tests__/controlled-layout.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,20 @@ import React from 'react';

import {act, render} from '@testing-library/react';

import {DashKitContext, type DashKitCtxShape} from '../../../context';
import {DashKitContext, DashkitOverlayControlsContext} from '../../../context';
import type {DashKitCtxShape, OverlayControlsCtxShape} from '../../../context';
import {type DashKitWithContextProps, withContext} from '../../../hocs/withContext';
import type {Config} from '../../../shared';
import {RegisterManager} from '../../../utils';
import {DashKit, _emitSymbol} from '../DashKit';

const PLUGIN_TYPE = 'controlled-layout-test';

const createConfig = (x: number): Config => ({
const createConfig = (x: number, itemIds = ['item1']): Config => ({
salt: 'test',
counter: 1,
items: [{id: 'item1', type: PLUGIN_TYPE, namespace: 'default', data: {}}],
layout: [{i: 'item1', x, y: 0, w: 2, h: 2}],
items: itemIds.map((id) => ({id, type: PLUGIN_TYPE, namespace: 'default', data: {}})),
layout: itemIds.map((i) => ({i, x, y: 0, w: 2, h: 2})),
aliases: {},
connections: [],
});
Expand Down Expand Up @@ -86,9 +87,30 @@ const createProps = ({
};

let captured: DashKitCtxShape = {} as DashKitCtxShape;
let capturedControlsContext: OverlayControlsCtxShape | undefined;
let renderedItemIds: Array<{config: string[]; layout: string[]}> = [];
let layoutEffectAction:
| ((
dashkitContext: DashKitCtxShape,
controlsContext: OverlayControlsCtxShape | undefined,
) => void)
| undefined;

const ContextCapture = () => {
captured = React.useContext(DashKitContext);
const dashkitContext = React.useContext(DashKitContext);
const controlsContext = React.useContext(DashkitOverlayControlsContext);

captured = dashkitContext;
capturedControlsContext = controlsContext || undefined;
renderedItemIds.push({
config: captured.configItems.map(({id}) => id),
layout: captured.layout.map(({i}) => i),
});

React.useLayoutEffect(() => {
layoutEffectAction?.(dashkitContext, controlsContext || undefined);
}, [controlsContext, dashkitContext]);

return null;
};

Expand All @@ -97,6 +119,91 @@ const TestComponent = withContext(ContextCapture);
describe('DashKit controlled layout strategy', () => {
beforeEach(() => {
captured = {} as DashKitCtxShape;
capturedControlsContext = undefined;
renderedItemIds = [];
layoutEffectAction = undefined;
});

it('updates visual layout in same render as externally added config item', () => {
const initialConfig = createConfig(0);
const externalConfig = createConfig(3, ['item1', 'item2']);
const dashkit = new DashKit(
DashKit.defaultProps as unknown as ConstructorParameters<typeof DashKit>[0],
);

const {rerender} = render(
<TestComponent
{...createProps({config: initialConfig, emitDashKitEvent: dashkit[_emitSymbol]})}
/>,
);

act(() => {
rerender(
<TestComponent
{...createProps({
config: externalConfig,
emitDashKitEvent: dashkit[_emitSymbol],
})}
/>,
);
});

expect(renderedItemIds).toEqual(
expect.arrayContaining([
{config: ['item1'], layout: ['item1']},
{config: ['item1', 'item2'], layout: ['item1', 'item2']},
]),
);
expect(
renderedItemIds.every(({config, layout}) => config.join(',') === layout.join(',')),
).toBe(true);
});

it('uses external baseline in child layout effect', () => {
const initialConfig = createConfig(0);
const externalConfig = createConfig(3, ['item1', 'item2']);
const nextLayout: Config['layout'] = [
{i: 'item1', x: 4, y: 0, w: 2, h: 2},
externalConfig.layout[1],
];
const dashkit = new DashKit(
DashKit.defaultProps as unknown as ConstructorParameters<typeof DashKit>[0],
);
const onEventChange = jest.fn();
let layoutItemInLayoutEffect: Config['layout'][number] | undefined;
let layoutItemAfterLayoutChange: Config['layout'][number] | undefined;

dashkit.on('change', onEventChange);

const {rerender} = render(
<TestComponent
{...createProps({config: initialConfig, emitDashKitEvent: dashkit[_emitSymbol]})}
/>,
);

layoutEffectAction = (dashkitContext, controlsContext) => {
layoutEffectAction = undefined;
layoutItemInLayoutEffect = controlsContext?.getLayoutItem('item2') || undefined;
dashkitContext.layoutChange(nextLayout);
layoutItemAfterLayoutChange = controlsContext?.getLayoutItem('item1') || undefined;
};

act(() => {
rerender(
<TestComponent
{...createProps({
config: externalConfig,
emitDashKitEvent: dashkit[_emitSymbol],
})}
/>,
);
});

expect(layoutItemInLayoutEffect).toMatchObject(externalConfig.layout[1]);
expect(onEventChange).toHaveBeenCalledTimes(1);
expect(onEventChange.mock.calls[0][0].previousLayout).toMatchObject(externalConfig.layout);
expect(layoutItemAfterLayoutChange).toMatchObject(nextLayout[0]);
expect(capturedControlsContext?.getLayoutItem('item1')).toMatchObject(nextLayout[0]);
});

it('Case 1: keeps internal layout after change event without config update, then applies external config update', () => {
Expand Down
69 changes: 52 additions & 17 deletions src/hocs/withContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ type AdjustedLayouts = Record<string, ConfigLayout>;

type NowrapAdjustedLayouts = Record<string, number>;

type PendingExternalBaselineLayout = {
layout: ConfigLayout[];
previousBaselineLayout: ConfigLayout[];
};

type UseMemoStateContextResult = {
dashkitContextValue: DashKitCtxShape;
controlsContextValue: OverlayControlsCtxShape;
Expand Down Expand Up @@ -111,6 +116,21 @@ function useMemoStateContext(props: DashKitWithContextProps): UseMemoStateContex

const internalBaselineLayoutRef = React.useRef<ConfigLayout[]>(enrichedPropsLayout);
const [visualLayout, setVisualLayout] = React.useState<ConfigLayout[]>(enrichedPropsLayout);
const [previousEnrichedPropsLayout, setPreviousEnrichedPropsLayout] =
React.useState(enrichedPropsLayout);
const [pendingExternalBaselineLayout, setPendingExternalBaselineLayout] =
React.useState<PendingExternalBaselineLayout>();
const getBaselineLayout = React.useCallback(() => {
if (
pendingExternalBaselineLayout &&
internalBaselineLayoutRef.current ===
pendingExternalBaselineLayout.previousBaselineLayout
) {
return pendingExternalBaselineLayout.layout;
}

return internalBaselineLayoutRef.current;
}, [pendingExternalBaselineLayout]);

const [externalLayoutRevision, setExternalLayoutRevision] = React.useState(0);
const [temporaryLayout, setTemporaryLayout] = React.useState<TemporaryLayout | null>(null);
Expand Down Expand Up @@ -169,7 +189,7 @@ function useMemoStateContext(props: DashKitWithContextProps): UseMemoStateContex

const baselineConfig = {
...props.config,
layout: internalBaselineLayoutRef.current,
layout: getBaselineLayout(),
};

const newConfig = UpdateManager.updateLayout({
Expand All @@ -186,16 +206,19 @@ function useMemoStateContext(props: DashKitWithContextProps): UseMemoStateContex

internalBaselineLayoutRef.current = newConfig.layout;
},
[props.config, props.emitDashKitEvent, onChange],
[getBaselineLayout, props.config, props.emitDashKitEvent, onChange],
);

const getLayoutItem = React.useCallback((id: string) => {
return internalBaselineLayoutRef.current.find(({i}) => i === id);
}, []);
const getLayoutItem = React.useCallback(
(id: string) => {
return getBaselineLayout().find(({i}) => i === id);
},
[getBaselineLayout],
);

const getOuterLayout = React.useCallback((): ConfigLayout[] => {
return convertEnrichedLayoutToConfigLayout(internalBaselineLayoutRef.current);
}, []);
return convertEnrichedLayoutToConfigLayout(getBaselineLayout());
}, [getBaselineLayout]);

// to calculate items, only memorization of items and globalItems is important
const configItems = React.useMemo(
Expand Down Expand Up @@ -348,20 +371,32 @@ function useMemoStateContext(props: DashKitWithContextProps): UseMemoStateContex
}
}, [props.registerManager, props.groups, visualLayout]);

// Synchronize internal baseline and visual layout when external props.layout changes re-init.
// When props.config.layout reference changes, external update occurred re-init.
React.useEffect(() => {
const internalNotEqualProps = !isEqual(
internalBaselineLayoutRef.current,
enrichedPropsLayout,
);
// Synchronize visual layout during render, so children never see new config with old layout.
// Keep the previous external layout separate from the drag baseline: a drag updates the latter.
if (previousEnrichedPropsLayout !== enrichedPropsLayout) {
setPreviousEnrichedPropsLayout(enrichedPropsLayout);

if (internalNotEqualProps) {
internalBaselineLayoutRef.current = enrichedPropsLayout;
if (!isEqual(internalBaselineLayoutRef.current, enrichedPropsLayout)) {
setPendingExternalBaselineLayout({
layout: enrichedPropsLayout,
previousBaselineLayout: internalBaselineLayoutRef.current,
});
setVisualLayout(enrichedPropsLayout);
setExternalLayoutRevision((r) => r + 1);
}
}, [enrichedPropsLayout]);
}

React.useLayoutEffect(() => {
if (pendingExternalBaselineLayout) {
if (
internalBaselineLayoutRef.current ===
pendingExternalBaselineLayout.previousBaselineLayout
) {
internalBaselineLayoutRef.current = pendingExternalBaselineLayout.layout;
}
setPendingExternalBaselineLayout(undefined);
}
}, [pendingExternalBaselineLayout]);

const itemsParams = useDeepEqualMemo(
() =>
Expand Down
Loading