Skip to content

Commit

Permalink
fix: DH-16463: isEqual returns false for layouts with undefined and m…
Browse files Browse the repository at this point in the history
…issing props in panelState (#1783)

`LayoutUtils.isEqual` gives a false negative when we compare a
dehydrated layout having any undefined property with the same layout
passed through `JSON.serialize`/`JSON.parse`, because all undefined
properties get deleted when serialized.

Fixes DH-16463
  • Loading branch information
vbabich authored Feb 8, 2024
1 parent 51e60c5 commit e90b627
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 4 deletions.
35 changes: 34 additions & 1 deletion packages/dashboard/src/layout/LayoutUtils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import type { ContentItem } from '@deephaven/golden-layout';
import type {
ContentItem,
ReactComponentConfig,
} from '@deephaven/golden-layout';
import LayoutUtils from './LayoutUtils';

function makeContentItem(type = 'root'): Partial<ContentItem> {
Expand Down Expand Up @@ -125,3 +128,33 @@ describe('getContentItemInStack', () => {
expect(found).toBeNull();
});
});

describe('isEqual', () => {
function makeComponentConfig(
props: Record<string, unknown>
): ReactComponentConfig {
return {
component: 'ReactComponent',
type: 'component',
props,
};
}

it('ignores the difference between undefined and missing properties', () => {
expect(
LayoutUtils.isEqual(
[makeComponentConfig({ a: 1, b: undefined })],
[makeComponentConfig({ a: 1 })]
)
).toBe(true);
});

it('correctly differentiates null from undefined', () => {
expect(
LayoutUtils.isEqual(
[makeComponentConfig({ a: 1, b: undefined })],
[makeComponentConfig({ a: 1, b: null })]
)
).toBe(false);
});
});
8 changes: 5 additions & 3 deletions packages/dashboard/src/layout/LayoutUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,14 +379,16 @@ class LayoutUtils {
layout2: DashboardLayoutConfig,
major = false
): boolean {
const layout1Clone = LayoutUtils.cloneLayout(layout1);
const layout2Clone = LayoutUtils.cloneLayout(layout2);
if (major) {
const layout1Clone = LayoutUtils.cloneLayout(layout1);
const layout2Clone = LayoutUtils.cloneLayout(layout2);
LayoutUtils.dropLayoutMinorChange(layout1Clone);
LayoutUtils.dropLayoutMinorChange(layout2Clone);
return deepEqual(layout1Clone, layout2Clone);
}
return deepEqual(layout1, layout2);
// Pass cloned layouts to avoid false negatives
// when comparing layouts with undefined and missing properties
return deepEqual(layout1Clone, layout2Clone);
}

static cloneLayout(layout: DashboardLayoutConfig): DashboardLayoutConfig {
Expand Down

0 comments on commit e90b627

Please sign in to comment.