Skip to content
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.

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

fix: Prevent unnecessary meta property resets in List widget #38639

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
57 changes: 51 additions & 6 deletions app/client/src/widgets/ListWidgetV2/MetaWidgetGenerator.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import hash from "object-hash";
import { difference, omit, set, get, isEmpty, isString, isNil } from "lodash";
import type { VirtualizerOptions } from "@tanstack/virtual-core";
import type {
RowDataChangeOptions,
MetaWidgetRowCache,
RowCache,
} from "./types";
import {
elementScroll,
observeElementOffset,
Expand Down Expand Up @@ -212,10 +217,11 @@ const hasCurrentView = (value: string) =>
const hasLevel = (value: string) =>
isString(value) && value.indexOf("level_") > -1;

class MetaWidgetGenerator {
private siblings: Siblings;
private cachedItemKeys: CachedRows;
private containerParentId: GeneratorOptions["containerParentId"];
export default class MetaWidgetGenerator {
siblings: Siblings;
rowDataCache: MetaWidgetRowCache = {};
cachedItemKeys: CachedRows;
containerParentId: GeneratorOptions["containerParentId"];
private containerWidgetId: GeneratorOptions["containerWidgetId"];
private currTemplateWidgets: TemplateWidgets;
private currViewMetaWidgets: ViewMetaWidget[];
Expand Down Expand Up @@ -537,6 +543,44 @@ class MetaWidgetGenerator {
return Array.from(removedWidgetsFromView);
};

private hasRowDataChanged(key: string, widgetId: string): boolean {
const currentData = this.getCurrentRowData(key);
const cachedData = this.rowDataCache[key]?.data;

// If we don't have cached data, consider it as changed
if (!cachedData) {
this.updateRowDataCache(key, currentData);
return true;
}

// Compare the current data with cached data
const hasChanged = !isEqual(currentData, cachedData);

// Debug logging for row data changes
console.debug(`[RowDataChange] Key: ${key}, Changed: ${hasChanged}`, {
widgetId,
currentData,
cachedData,
});

if (hasChanged) {
this.updateRowDataCache(key, currentData);
}

return hasChanged;
}

private getCurrentRowData(key: string): Record<string, unknown> | undefined {
return this.cachedKeyDataMap[key] || undefined;
}

private updateRowDataCache(key: string, data?: Record<string, unknown>) {
this.rowDataCache[key] = {
data,
lastUpdated: Date.now(),
};
}

private generateMetaWidgetRecursively = ({
options,
parentId,
Expand Down Expand Up @@ -583,7 +627,8 @@ class MetaWidgetGenerator {

if (
!this.shouldGenerateMetaWidgetFor(templateWidget.widgetId, key) &&
!options?.keepMetaWidgetData
!options?.keepMetaWidgetData &&
!this.hasRowDataChanged(key, templateWidget.widgetId)
) {
return { childMetaWidgets, metaWidgetName, metaWidgetId };
}
Expand Down Expand Up @@ -1986,4 +2031,4 @@ class MetaWidgetGenerator {
};
}

export default MetaWidgetGenerator;
// Class is already exported as default above
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import MetaWidgetGenerator from "../MetaWidgetGenerator";
import type { MetaWidgetRowCache } from "../types";
import { isEqual } from "lodash";

// Mock required dependencies
jest.mock("@tanstack/virtual-core", () => ({
elementScroll: jest.fn(),
observeElementOffset: jest.fn(),
observeElementRect: jest.fn(),
Virtualizer: jest.fn(),
}));

jest.mock("lodash", () => ({
...jest.requireActual("lodash"),
isEqual: jest.fn(),
}));

jest.mock("utils/helpers", () => ({
...jest.requireActual("utils/helpers"),
klonaRegularWithTelemetry: jest.fn((data) => data),
}));

describe("MetaWidgetGenerator", () => {
let generator: MetaWidgetGenerator;

beforeEach(() => {
generator = new MetaWidgetGenerator({
getWidgetCache: () => ({}),
getWidgetReferenceCache: () => ({}),
infiniteScroll: false,
isListCloned: false,
level: 1,
onVirtualListScroll: () => {},
prefixMetaWidgetId: "test",
primaryWidgetType: "LIST_WIDGET_V2",
renderMode: "CANVAS",
setWidgetCache: () => {},
setWidgetReferenceCache: () => {},
});
(isEqual as jest.Mock).mockImplementation((a, b) => a === b);
});

afterEach(() => {
jest.clearAllMocks();
});
describe("hasRowDataChanged", () => {
it("should return true when no cached data exists", () => {
const generator = new MetaWidgetGenerator({
getWidgetCache: () => ({}),
getWidgetReferenceCache: () => ({}),
infiniteScroll: false,
isListCloned: false,
level: 1,
onVirtualListScroll: () => {},
prefixMetaWidgetId: "test",
primaryWidgetType: "LIST_WIDGET_V2",
renderMode: "CANVAS",
setWidgetCache: () => {},
setWidgetReferenceCache: () => {},
});

// @ts-expect-error: Testing private method
const result = generator.hasRowDataChanged("test-key", "test-widget");
expect(result).toBe(true);
});

it("should return true when data has changed", () => {
const generator = new MetaWidgetGenerator({
getWidgetCache: () => ({}),
getWidgetReferenceCache: () => ({}),
infiniteScroll: false,
isListCloned: false,
level: 1,
onVirtualListScroll: () => {},
prefixMetaWidgetId: "test",
primaryWidgetType: "LIST_WIDGET_V2",
renderMode: "CANVAS",
setWidgetCache: () => {},
setWidgetReferenceCache: () => {},
});

Object.defineProperty(generator, "rowDataCache", {
value: {
"test-key": {
data: { value: "old" },
lastUpdated: Date.now(),
},
},
writable: true,
});

Object.defineProperty(generator, "cachedKeyDataMap", {
value: {
"test-key": { value: "new" },
},
writable: true,
});

// @ts-expect-error: Testing private method
const result = generator.hasRowDataChanged("test-key", "test-widget");
expect(result).toBe(true);
});

it("should return false when data has not changed", () => {
const generator = new MetaWidgetGenerator({
getWidgetCache: () => ({}),
getWidgetReferenceCache: () => ({}),
infiniteScroll: false,
isListCloned: false,
level: 1,
onVirtualListScroll: () => {},
prefixMetaWidgetId: "test",
primaryWidgetType: "LIST_WIDGET_V2",
renderMode: "CANVAS",
setWidgetCache: () => {},
setWidgetReferenceCache: () => {},
});

const testData = { value: "same" };

Object.defineProperty(generator, "rowDataCache", {
value: {
"test-key": {
data: testData,
lastUpdated: Date.now(),
},
},
writable: true,
});

Object.defineProperty(generator, "cachedKeyDataMap", {
value: {
"test-key": testData,
},
writable: true,
});

// @ts-expect-error: Testing private method
const result = generator.hasRowDataChanged("test-key", "test-widget");
expect(result).toBe(false);
});
});
});
23 changes: 23 additions & 0 deletions app/client/src/widgets/ListWidgetV2/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
export type DataRecord = Record<string, unknown>;

export interface RowDataChangeOptions {
key: string;
widgetId: string;
prevData?: DataRecord;
currData?: DataRecord;
}

export interface MetaWidgetRowCache {
[key: string]: {
data?: DataRecord;
lastUpdated?: number;
};
}

export interface RowCache {
[key: string]: {
data?: DataRecord;
metaProperties?: DataRecord;
lastUpdated?: number;
};
}
31 changes: 24 additions & 7 deletions app/client/src/workers/Evaluation/fns/resetWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,30 @@ function resetWidgetMetaProperty(

continue;
} else {
evalMetaUpdates.push({
widgetId: evaluatedEntity.isMetaWidget
? (evaluatedEntity.metaWidgetId as string)
: evaluatedEntity.widgetId,
metaPropertyPath: propertyPath.split("."),
value: undefined,
});
const unEvalEntity = oldUnEvalTree[widget.widgetName] as WidgetEntity;
// Skip resetting inputText meta property if it's already set and matches expected type
const currentMetaValue = metaObj[propertyPath];
const skipReset =
propertyPath === "inputText" &&
currentMetaValue !== undefined &&
unEvalEntity &&
typeof currentMetaValue === "string";

// Debug logging for meta property reset
console.debug(
`[MetaReset] Property: ${propertyPath}, Value: ${currentMetaValue}, Skip: ${skipReset}`,
{ widgetName: widget.widgetName },
);

if (!skipReset) {
evalMetaUpdates.push({
widgetId: evaluatedEntity.isMetaWidget
? (evaluatedEntity.metaWidgetId as string)
: evaluatedEntity.widgetId,
metaPropertyPath: propertyPath.split("."),
value: undefined,
});
}
}
}

Expand Down
Loading