From f9d2e2f0ab6fb2ca50ed81be0ccab4d1072fa3b6 Mon Sep 17 00:00:00 2001 From: Vemparala Surya Vamsi <121419957+vsvamsi1@users.noreply.github.com> Date: Wed, 16 Apr 2025 12:58:09 +0530 Subject: [PATCH] chore: removed last klona (#40257) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description - Removed the final use of klona, which was a significant contributor to performance bottlenecks in our evaluation cycles. - Enhanced the dataTree reduction logic to improve the quality of diff updates. Previously, some diffs were not applied correctly, potentially leading to stale application states. This change should reduce related Sentry-reported issues. - In a configured customer app running on Windows, we observed approximately a 1-second improvement in LCP. Additionally, overall Web Worker scripting time is expected to decrease by about 29.6%. These optimizations primarily target update evaluation cycles and should lead to noticeably improved app responsiveness. ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 9d0b751c6da7675938e53bd11ae5f2a0158e144a > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Tue, 15 Apr 2025 18:35:25 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **Bug Fixes** - Improved error handling for evaluation errors, with enhanced logging and reporting for specific error types. - **Tests** - Added comprehensive test cases covering data tree type changes, update cycles, error handling during state updates, and evaluation flow accuracy. - **Refactor** - Refactored data tree diffing, update logic, and evaluation state management for better modularity, error resilience, and control flow during evaluation cycles. - **New Features** - Introduced granular tracking, serialization, and application of data tree updates to improve performance and reliability. --- app/client/src/sagas/EvalErrorHandler.ts | 9 + app/client/src/utils/DynamicBindingUtils.ts | 1 + .../__tests__/generateOpimisedUpdates.test.ts | 459 +++++++++++++++++- .../Evaluation/evalTreeWithChanges.test.ts | 26 +- .../workers/Evaluation/evalTreeWithChanges.ts | 21 +- .../workers/Evaluation/handlers/evalTree.ts | 23 +- .../src/workers/Evaluation/helpers.test.ts | 128 ++++- app/client/src/workers/Evaluation/helpers.ts | 194 ++++++-- 8 files changed, 785 insertions(+), 76 deletions(-) diff --git a/app/client/src/sagas/EvalErrorHandler.ts b/app/client/src/sagas/EvalErrorHandler.ts index 46777a3efc..016ccd8c3e 100644 --- a/app/client/src/sagas/EvalErrorHandler.ts +++ b/app/client/src/sagas/EvalErrorHandler.ts @@ -309,6 +309,15 @@ export function* evalErrorHandler( }); break; } + case EvalErrorTypes.UPDATE_DATA_TREE_ERROR: { + // Log to Sentry with additional context + Sentry.captureMessage(error.message); + // Log locally with error details + log.error(`Evaluation Error: ${error.message}`, { + type: error.type, + }); + break; + } default: { log.error(error); captureException(error, { errorName: "UnknownEvalError" }); diff --git a/app/client/src/utils/DynamicBindingUtils.ts b/app/client/src/utils/DynamicBindingUtils.ts index 684a546286..2256ae178e 100644 --- a/app/client/src/utils/DynamicBindingUtils.ts +++ b/app/client/src/utils/DynamicBindingUtils.ts @@ -160,6 +160,7 @@ export enum EvalErrorTypes { EXTRACT_DEPENDENCY_ERROR = "EXTRACT_DEPENDENCY_ERROR", CLONE_ERROR = "CLONE_ERROR", SERIALIZATION_ERROR = "SERIALIZATION_ERROR", + UPDATE_DATA_TREE_ERROR = "UPDATE_DATA_TREE_ERROR", } export interface EvalError { diff --git a/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts b/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts index cea35fb62d..5dd2136082 100644 --- a/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts @@ -10,10 +10,12 @@ import type { EvaluationError, } from "utils/DynamicBindingUtils"; import { EvalErrorTypes } from "utils/DynamicBindingUtils"; +import type { DataTree } from "entities/DataTree/dataTreeTypes"; import { generateOptimisedUpdates, generateSerialisedUpdates, + getReducedDataTrees, } from "../helpers"; export const smallDataSet = [ @@ -406,16 +408,15 @@ describe("generateOptimisedUpdates", () => { const parsedUpdates = parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); - expect(parsedUpdates).toEqual([ - { - kind: "D", - path: ["Table1", "__evaluation__", "errors", "transientTableData"], - }, - { - kind: "D", - path: ["Table1", "pageSize"], - }, - ]); + expect(parsedUpdates).toHaveLength(2); + expect(parsedUpdates).toContainEqual({ + kind: "D", + path: ["Table1", "__evaluation__", "errors", "transientTableData"], + }); + expect(parsedUpdates).toContainEqual({ + kind: "D", + path: ["Table1", "pageSize"], + }); const parseAndApplyUpdatesToOldState = create(oldState, (draft) => { // TODO: Fix this the next time the file is edited @@ -671,4 +672,442 @@ describe("generateOptimisedUpdates", () => { }); }); }); + + describe("type change tests", () => { + it("should handle type changes correctly (array to object)", () => { + // this testcase is verify an actual bug that was happening + // Create the old data tree with an array + const oldDataTree = { + JSONForm1: { + fieldState: { + name: [], + }, + }, + } as unknown as DataTree; + + // Create the new data tree with an object + const newDataTree = { + JSONForm1: { + fieldState: { + name: { + isDisabled: false, + isRequired: false, + isVisible: true, + isValid: true, + }, + }, + }, + } as unknown as DataTree; + + const constrainedDiffPaths = [ + "JSONForm1.fieldState.name.isDisabled", + "JSONForm1.fieldState.name.isRequired", + "JSONForm1.fieldState.name.isVisible", + "JSONForm1.fieldState.name.isValid", + ]; + + // Generate the updates + const updates = generateOptimisedUpdates( + oldDataTree, + newDataTree, + constrainedDiffPaths, + ); + + // Verify that the updates contain the expected type change + expect(updates).toHaveLength(1); + expect(updates[0]).toEqual({ + kind: "E", + path: ["JSONForm1", "fieldState", "name"], + lhs: [], + rhs: { + isDisabled: false, + isRequired: false, + isVisible: true, + isValid: true, + }, + }); + }); + + it("should handle type changes correctly (object to array)", () => { + // Create the old data tree with an object + const oldDataTree = { + JSONForm1: { + fieldState: { + name: { + isDisabled: false, + isRequired: false, + isVisible: true, + isValid: true, + }, + }, + }, + } as unknown as DataTree; + + // Create the new data tree with an array + const newDataTree = { + JSONForm1: { + fieldState: { + name: [], + }, + }, + } as unknown as DataTree; + + // creating deep paths to which pulls undefined values, so that the reduced dataTree is able to pick the closest defined ancestor and generate a diff based on that + const constrainedDiffPaths = [ + "JSONForm1.fieldState.name.isDisabled", + "JSONForm1.fieldState.name.isRequired", + "JSONForm1.fieldState.name.isVisible", + "JSONForm1.fieldState.name.isValid", + ]; + // Generate the updates + const updates = generateOptimisedUpdates( + oldDataTree, + newDataTree, + constrainedDiffPaths, + ); + + // Verify that the updates contain the expected type change + expect(updates).toHaveLength(1); + expect(updates[0]).toEqual({ + kind: "E", + path: ["JSONForm1", "fieldState", "name"], + lhs: { + isDisabled: false, + isRequired: false, + isVisible: true, + isValid: true, + }, + rhs: [], + }); + }); + + it("should handle type changes correctly (empty object to object with properties)", () => { + // Create the old data tree with an empty object + const oldDataTree = { + JSONForm1: { + fieldState: { + name: {}, + }, + }, + } as unknown as DataTree; + + // Create the new data tree with an object with properties + const newDataTree = { + JSONForm1: { + fieldState: { + name: { + isDisabled: false, + isRequired: false, + isVisible: true, + isValid: true, + }, + }, + }, + } as unknown as DataTree; + + const constrainedDiffPaths = ["JSONForm1.fieldState.name"]; + + // Generate the updates + const updates = generateOptimisedUpdates( + oldDataTree, + newDataTree, + constrainedDiffPaths, + ); + + // Verify that the updates contain the expected type change + expect(updates).toHaveLength(4); // One update for each property + expect(updates).toContainEqual({ + kind: "N", + path: ["JSONForm1", "fieldState", "name", "isDisabled"], + rhs: false, + }); + expect(updates).toContainEqual({ + kind: "N", + path: ["JSONForm1", "fieldState", "name", "isRequired"], + rhs: false, + }); + expect(updates).toContainEqual({ + kind: "N", + path: ["JSONForm1", "fieldState", "name", "isVisible"], + rhs: true, + }); + expect(updates).toContainEqual({ + kind: "N", + path: ["JSONForm1", "fieldState", "name", "isValid"], + rhs: true, + }); + }); + + it("should handle type changes correctly with complex constrainedDiffPaths", () => { + // Create the old data tree with an array + const oldDataTree = { + JSONForm1: { + fieldState: { + name: [], + }, + schema: { + __root_schema__: { + children: { + name: { + defaultValue: "John", + borderRadius: "4px", + accentColor: "blue", + }, + }, + }, + }, + }, + } as unknown as DataTree; + + // Create the new data tree with an object + const newDataTree = { + JSONForm1: { + fieldState: { + name: { + isDisabled: false, + isRequired: false, + isVisible: true, + isValid: true, + }, + }, + schema: { + __root_schema__: { + children: { + name: { + defaultValue: "John", + borderRadius: "4px", + accentColor: "blue", + }, + }, + }, + }, + }, + } as unknown as DataTree; + + const constrainedDiffPaths = [ + "JSONForm1.fieldState.name", + "JSONForm1.schema.__root_schema__.children.name.borderRadius", + "JSONForm1.schema.__root_schema__.children.name.defaultValue", + "JSONForm1.schema.__root_schema__.children.name.accentColor", + ]; + + // Generate the updates + const updates = generateOptimisedUpdates( + oldDataTree, + newDataTree, + constrainedDiffPaths, + ); + + // Find the update for JSONForm1.fieldState.name + const nameUpdate = updates.find( + (update) => + update.path && + update.path.length === 3 && + update.path[0] === "JSONForm1" && + update.path[1] === "fieldState" && + update.path[2] === "name", + ); + + // Verify that the update for JSONForm1.fieldState.name is correct + expect(nameUpdate).toEqual({ + kind: "E", + path: ["JSONForm1", "fieldState", "name"], + lhs: [], + rhs: { + isDisabled: false, + isRequired: false, + isVisible: true, + isValid: true, + }, + }); + }); + }); + + test("should generate updates for root level changes when constrainedDiffPaths is a deep path and a new entity is added", () => { + // Create a new state with a new entity added at the root level + const newState = create(oldState, (draft) => { + // Add a new entity at the root level with a large collection + (draft as unknown as Record).NewEntity = { + ENTITY_TYPE: "WIDGET", + tableData: largeDataSet, // Using the large dataset defined at the top of the file + type: "TABLE_WIDGET_V2", + __evaluation__: { + errors: {}, + }, + }; + }); + + // Constrained diff paths is a deep path in an existing entity + const updates = generateOptimisedUpdates(oldState, newState, [ + "NewEntity.Table1.tableData", + ]); + + // Should generate an update for the entire new entity since it contains a large collection + expect(updates).toEqual([ + { + kind: "N", + path: ["NewEntity"], + rhs: newState.NewEntity, + }, + ]); + }); +}); + +describe("getReducedDataTrees", () => { + it("should handle type changes and only include relevant root properties as per the constrainedDiffPaths, __evaluation__ are always included", () => { + const oldDataTree = { + JSONForm1: { + fieldState: { + name: [], + }, + __evaluation__: { + errors: { + name: [], + }, + }, + }, + UnrelatedEntity: { + value: "test", + __evaluation__: { + errors: {}, + }, + }, + } as unknown as DataTree; + + const newDataTree = { + JSONForm1: { + fieldState: { + name: { + isDisabled: false, + isRequired: false, + }, + text: { + isDisabled: false, + isRequired: false, + }, + }, + __evaluation__: { + errors: { + name: [], + }, + }, + }, + UnrelatedEntity: { + value: "changed", + __evaluation__: { + errors: {}, + }, + }, + } as unknown as DataTree; + + const constrainedDiffPaths = ["JSONForm1.fieldState.name.isDisabled"]; + + const { newData, oldData } = getReducedDataTrees( + oldDataTree, + newDataTree, + constrainedDiffPaths, + ); + + // Assert the entire oldData and newData objects + expect(oldData).toEqual({ + JSONForm1: { + __evaluation__: { + errors: { + name: [], + }, + }, + fieldState: { + name: [], + }, + }, + // Any __evaluations__ properties are always included in the dataTree even if they are not part of the constrained diff paths + UnrelatedEntity: { + __evaluation__: { + errors: {}, + }, + }, + }); + + expect(newData).toEqual({ + JSONForm1: { + __evaluation__: { + errors: { + name: [], + }, + }, + fieldState: { + name: { + isDisabled: false, + isRequired: false, + }, + // text property is not included in the constrainedDiffPaths hernce it is not included here + }, + }, + // Any __evaluations__ properties are always included in the dataTree even if they are not part of the constrained diff paths + UnrelatedEntity: { + // value property is not included in the constrainedDiffPaths hence it is not included here + __evaluation__: { + errors: {}, + }, + }, + }); + }); + + it("should handle undefined values in paths correctly", () => { + const oldDataTree = { + JSONForm1: { + fieldState: { + name: { + value: "test", + }, + }, + __evaluation__: { + errors: {}, + }, + }, + } as unknown as DataTree; + + const newDataTree = { + JSONForm1: { + fieldState: { + name: undefined, + }, + __evaluation__: { + errors: {}, + }, + }, + } as unknown as DataTree; + + const constrainedDiffPaths = ["JSONForm1.fieldState.name.value"]; + + const { newData, oldData } = getReducedDataTrees( + oldDataTree, + newDataTree, + constrainedDiffPaths, + ); + + expect(oldData).toEqual({ + JSONForm1: { + __evaluation__: { + errors: {}, + }, + fieldState: { + name: { + value: "test", + }, + }, + }, + }); + + expect(newData).toEqual({ + JSONForm1: { + __evaluation__: { + errors: {}, + }, + fieldState: { + name: undefined, + }, + }, + }); + }); }); diff --git a/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts b/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts index a6da206ec9..8053d86fac 100644 --- a/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts +++ b/app/client/src/workers/Evaluation/evalTreeWithChanges.test.ts @@ -5,11 +5,13 @@ import { ENTITY_TYPE } from "ee/entities/DataTree/types"; import type { ConfigTree } from "entities/DataTree/dataTreeTypes"; import { generateDataTreeWidget } from "entities/DataTree/dataTreeWidget"; import { create } from "mutative"; +import { klona } from "klona/json"; import type { WidgetEntity } from "plugins/Linting/lib/entity/WidgetEntity"; import type { UpdateDataTreeMessageData } from "sagas/EvalWorkerActionSagas"; import DataTreeEvaluator from "workers/common/DataTreeEvaluator"; import * as evalTreeWithChanges from "./evalTreeWithChanges"; import { APP_MODE } from "entities/App"; +import { updateEvalProps } from "./helpers"; export const BASE_WIDGET = { widgetId: "randomID", widgetName: "randomWidgetName", @@ -186,6 +188,7 @@ describe("evaluateAndGenerateResponse", () => { }; beforeEach(async () => { + // we are mimicking the first tree evaluation flow here evaluator = new DataTreeEvaluator(WIDGET_CONFIG_MAP); await evaluator.setupFirstTree( unEvalTree, @@ -200,6 +203,10 @@ describe("evaluateAndGenerateResponse", () => { }, ); evaluator.evalAndValidateFirstTree(); + const dataTree = updateEvalProps(evaluator) || {}; + + // over here we are setting the prevState through a klona but in the first tree we set by parsing the serialised update which is functionally the same + evaluator?.setPrevState(klona(dataTree)); }); test("inital evaluation successful should be successful", () => { @@ -369,12 +376,12 @@ describe("evaluateAndGenerateResponse", () => { expect(parsedUpdates).toEqual( expect.arrayContaining([ { - kind: "N", + kind: "E", path: ["Text1", "text"], rhs: "updated Label", }, { - kind: "N", + kind: "E", path: ["Text2", "text"], rhs: "updated Label", }, @@ -511,6 +518,7 @@ describe("evaluateAndGenerateResponse", () => { [], [], ); + const parsedUpdates = getParsedUpdatesFromWebWorkerResp(webworkerResponse); @@ -520,15 +528,11 @@ describe("evaluateAndGenerateResponse", () => { payload: { propertyPath: "Text1.text", value: "" }, }, ]); - expect(parsedUpdates).toEqual( - expect.arrayContaining([ - { - kind: "N", - path: ["Text1", "text"], - rhs: UPDATED_LABEL, - }, - ]), - ); + expect(parsedUpdates).toEqual([ + { kind: "E", path: ["Text1", "text"], rhs: UPDATED_LABEL }, + // Text2 is updated because of the binding + { kind: "E", path: ["Text2", "text"], rhs: UPDATED_LABEL }, + ]); }); test("should ignore generating updates when unEvalUpdates is empty", () => { // TODO: Fix this the next time the file is edited diff --git a/app/client/src/workers/Evaluation/evalTreeWithChanges.ts b/app/client/src/workers/Evaluation/evalTreeWithChanges.ts index bc561de0df..4d4c3f1f4f 100644 --- a/app/client/src/workers/Evaluation/evalTreeWithChanges.ts +++ b/app/client/src/workers/Evaluation/evalTreeWithChanges.ts @@ -1,6 +1,5 @@ import { dataTreeEvaluator } from "./handlers/evalTree"; import type { EvalMetaUpdates } from "ee/workers/common/DataTreeEvaluator/types"; -import { makeEntityConfigsAsObjProperties } from "ee/workers/Evaluation/dataTreeUtils"; import type { EvalTreeResponseData, EvalWorkerSyncRequest, @@ -13,9 +12,12 @@ import { generateOptimisedUpdatesAndSetPrevState, getNewDataTreeUpdates, uniqueOrderUpdatePaths, + updateEvalProps, } from "./helpers"; import type { DataTreeDiff } from "ee/workers/Evaluation/evaluationUtils"; import type DataTreeEvaluator from "workers/common/DataTreeEvaluator"; +import type { Diff } from "deep-diff"; +import type { DataTree } from "entities/DataTree/dataTreeTypes"; const getDefaultEvalResponse = (): EvalTreeResponseData => ({ updates: "[]", @@ -104,6 +106,8 @@ export const evaluateAndGenerateResponse = ( {}, dataTreeEvaluator, [], + undefined, + false, ); defaultResponse.updates = updates; @@ -126,12 +130,7 @@ export const evaluateAndGenerateResponse = ( unEvalUpdates, ); - const dataTree = makeEntityConfigsAsObjProperties( - dataTreeEvaluator.evalTree, - { - evalProps: dataTreeEvaluator.evalProps, - }, - ); + const dataTree = updateEvalProps(dataTreeEvaluator) || {}; /** Make sure evalMetaUpdates is sanitized to prevent postMessage failure */ defaultResponse.evalMetaUpdates = JSON.parse( @@ -145,7 +144,8 @@ export const evaluateAndGenerateResponse = ( const additionalUpdates = getNewDataTreeUpdates( additionalPathsAddedAsUpdates, dataTree, - ); + ) as Diff[]; + // the affected paths is a combination of the eval order and the uneval updates // we use this collection to limit the diff between the old and new data tree const affectedNodePaths = getAffectedNodesInTheDataTree( @@ -153,12 +153,15 @@ export const evaluateAndGenerateResponse = ( evalOrder, ); - defaultResponse.updates = generateOptimisedUpdatesAndSetPrevState( + const updates = generateOptimisedUpdatesAndSetPrevState( dataTree, dataTreeEvaluator, affectedNodePaths, additionalUpdates, + true, ); + + defaultResponse.updates = updates; dataTreeEvaluator.undefinedEvalValuesMap = dataTreeEvaluator.undefinedEvalValuesMap || {}; diff --git a/app/client/src/workers/Evaluation/handlers/evalTree.ts b/app/client/src/workers/Evaluation/handlers/evalTree.ts index 707c72b8e7..5c0d73e213 100644 --- a/app/client/src/workers/Evaluation/handlers/evalTree.ts +++ b/app/client/src/workers/Evaluation/handlers/evalTree.ts @@ -19,6 +19,7 @@ import { errorModifier } from "../errorModifier"; import { generateOptimisedUpdatesAndSetPrevState, uniqueOrderUpdatePaths, + updateEvalProps, } from "../helpers"; import DataStore from "../dataStore"; import type { TransmissionErrorHandler } from "../fns/utils/Messenger"; @@ -131,9 +132,8 @@ export async function evalTree( ), ); - dataTree = makeEntityConfigsAsObjProperties(dataTreeResponse.evalTree, { - evalProps: dataTreeEvaluator.evalProps, - }); + dataTree = updateEvalProps(dataTreeEvaluator) || {}; + staleMetaIds = dataTreeResponse.staleMetaIds; isNewTree = true; } else if (dataTreeEvaluator.hasCyclicalDependency || forceEvaluation) { @@ -184,10 +184,10 @@ export async function evalTree( (dataTreeEvaluator as DataTreeEvaluator).evalAndValidateFirstTree(), ); - dataTree = makeEntityConfigsAsObjProperties(dataTreeResponse.evalTree, { - evalProps: dataTreeEvaluator.evalProps, - }); + dataTree = updateEvalProps(dataTreeEvaluator) || {}; + staleMetaIds = dataTreeResponse.staleMetaIds; + isNewTree = true; } else { const tree = dataTreeEvaluator.getEvalTree(); @@ -241,14 +241,13 @@ export async function evalTree( ), ); - dataTree = makeEntityConfigsAsObjProperties(dataTreeEvaluator.evalTree, { - evalProps: dataTreeEvaluator.evalProps, - }); + dataTree = updateEvalProps(dataTreeEvaluator) || {}; evalMetaUpdates = JSON.parse( JSON.stringify(updateResponse.evalMetaUpdates), ); staleMetaIds = updateResponse.staleMetaIds; + isNewTree = false; } dependencies = dataTreeEvaluator.inverseDependencies; @@ -303,7 +302,9 @@ export async function evalTree( try { //for new tree send the whole thing, don't diff at all updates = serialiseToBigInt([{ kind: "newTree", rhs: dataTree }]); - dataTreeEvaluator?.setPrevState(dataTree); + const parsedUpdates = JSON.parse(updates); + + dataTreeEvaluator?.setPrevState(parsedUpdates[0].rhs); } catch (e) { updates = "[]"; } @@ -322,6 +323,8 @@ export async function evalTree( dataTree, dataTreeEvaluator, completeEvalOrder, + undefined, + true, ); } diff --git a/app/client/src/workers/Evaluation/helpers.test.ts b/app/client/src/workers/Evaluation/helpers.test.ts index 3e367b01e6..faeea4b53e 100644 --- a/app/client/src/workers/Evaluation/helpers.test.ts +++ b/app/client/src/workers/Evaluation/helpers.test.ts @@ -1,4 +1,6 @@ -import { fn_keys, stringifyFnsInObject } from "./helpers"; +import { fn_keys, stringifyFnsInObject, updatePrevState } from "./helpers"; +import type { DataTree } from "entities/DataTree/dataTreeTypes"; +import { EvalErrorTypes } from "utils/DynamicBindingUtils"; describe("stringifyFnsInObject", () => { it("includes full path of key having a function in the parent object", () => { @@ -93,3 +95,127 @@ describe("stringifyFnsInObject", () => { }); }); }); + +describe("updatePrevState", () => { + it("should update prevState with dataTree when isUpdateCycle is false", () => { + // Create a simple dataTree + const dataTree = { + Button1: { + ENTITY_TYPE: "WIDGET", + text: "Click me", + type: "BUTTON_WIDGET", + widgetId: "button1", + }, + Text1: { + ENTITY_TYPE: "WIDGET", + text: "Hello World", + type: "TEXT_WIDGET", + widgetId: "text1", + }, + } as unknown as DataTree; + + // Create a mock dataTreeEvaluator + const dataTreeEvaluator = { + setPrevState: jest.fn(), + getPrevState: jest.fn(), + }; + + // Call updatePrevState with isUpdateCycle = false + updatePrevState(false, dataTreeEvaluator, "[]", dataTree); + + // Verify setPrevState was called with the dataTree + expect(dataTreeEvaluator.setPrevState).toHaveBeenCalledWith(dataTree); + }); + + it("should update prevState with serialized updates when isUpdateCycle is true", () => { + // Create a simple dataTree + const dataTree = { + Button1: { + ENTITY_TYPE: "WIDGET", + text: "Click me", + type: "BUTTON_WIDGET", + widgetId: "button1", + }, + } as unknown as DataTree; + + // Create a mock dataTreeEvaluator with a prevState + const prevState = { + Button1: { + ENTITY_TYPE: "WIDGET", + text: "Old text", + type: "BUTTON_WIDGET", + widgetId: "button1", + }, + } as unknown as DataTree; + + const dataTreeEvaluator = { + setPrevState: jest.fn(), + getPrevState: jest.fn().mockReturnValue(prevState), + }; + + // Create serialized updates that change Button1.text + const serializedUpdates = JSON.stringify([ + { + kind: "E", + path: ["Button1", "text"], + rhs: "New text", + }, + ]); + + // Call updatePrevState with isUpdateCycle = true + updatePrevState(true, dataTreeEvaluator, serializedUpdates, dataTree); + + // Verify setPrevState was called with an updated state + expect(dataTreeEvaluator.setPrevState).toHaveBeenCalled(); + const updatedState = dataTreeEvaluator.setPrevState.mock.calls[0][0]; + + expect(updatedState.Button1.text).toBe("New text"); + }); + + it("should handle errors during update and push them to dataTreeEvaluator.errors", () => { + // Create a simple dataTree + const dataTree = { + Button1: { + ENTITY_TYPE: "WIDGET", + text: { a: 1 }, + type: "BUTTON_WIDGET", + widgetId: "button1", + }, + } as unknown as DataTree; + + // Create a mock dataTreeEvaluator with a prevState + const prevState = { + Button1: { + ENTITY_TYPE: "WIDGET", + text: [], + type: "BUTTON_WIDGET", + widgetId: "button1", + }, + } as unknown as DataTree; + + const dataTreeEvaluator = { + setPrevState: jest.fn(), + getPrevState: jest.fn().mockReturnValue(prevState), + errors: [] as Array<{ type: string; message: string }>, + }; + + // Create serialized updates with an invalid path to trigger an error + const serializedUpdates = JSON.stringify([ + { + kind: "N", + path: ["Button1", "text", "a"], + rhs: "New text", + }, + ]); + + // Call updatePrevState with isUpdateCycle = true + updatePrevState(true, dataTreeEvaluator, serializedUpdates, dataTree); + + // Verify an error was pushed to dataTreeEvaluator.errors + expect(dataTreeEvaluator.errors.length).toBe(1); + expect(dataTreeEvaluator.errors[0].type).toBe( + EvalErrorTypes.UPDATE_DATA_TREE_ERROR, + ); + expect(dataTreeEvaluator.errors[0].message).toBeTruthy(); + }); +}); diff --git a/app/client/src/workers/Evaluation/helpers.ts b/app/client/src/workers/Evaluation/helpers.ts index 2c4d70b685..62ffffdd97 100644 --- a/app/client/src/workers/Evaluation/helpers.ts +++ b/app/client/src/workers/Evaluation/helpers.ts @@ -1,14 +1,16 @@ import { serialiseToBigInt } from "ee/workers/Evaluation/evaluationUtils"; import type { WidgetEntity } from "ee//entities/DataTree/types"; import type { Diff } from "deep-diff"; -import { diff } from "deep-diff"; +import { applyChange, diff } from "deep-diff"; import type { DataTree } from "entities/DataTree/dataTreeTypes"; import equal from "fast-deep-equal"; import { get, isObject, set } from "lodash"; import { isMoment } from "moment"; import { EvalErrorTypes } from "utils/DynamicBindingUtils"; - +import { create } from "mutative"; export const fn_keys: string = "__fn_keys__"; +import { klona } from "klona/json"; +import type DataTreeEvaluator from "workers/common/DataTreeEvaluator"; export const uniqueOrderUpdatePaths = (updatePaths: string[]) => Array.from(new Set(updatePaths)).sort((a, b) => b.length - a.length); @@ -180,30 +182,89 @@ const isLargeCollection = (val: any) => { return size > LARGE_COLLECTION_SIZE; }; -const getReducedDataTree = ( - dataTree: DataTree, +export const getReducedDataTrees = ( + oldDataTree: DataTree, + newDataTree: DataTree, constrainedDiffPaths: string[], -): DataTree => { - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const withErrors = Object.keys(dataTree).reduce((acc: any, key: string) => { - const widgetValue = dataTree[key] as WidgetEntity; +): { oldData: DataTree; newData: DataTree } => { + // Create base trees with error information + const oldWithErrors = Object.keys(oldDataTree).reduce( + (acc: Record, key: string) => { + const widgetValue = oldDataTree[key] as WidgetEntity; - acc[key] = { - __evaluation__: { - errors: widgetValue.__evaluation__?.errors, - }, - }; + acc[key] = { + __evaluation__: { + errors: widgetValue.__evaluation__?.errors, + }, + }; - return acc; - }, {}); + return acc; + }, + {}, + ); - return constrainedDiffPaths.reduce((acc: DataTree, key: string) => { - set(acc, key, get(dataTree, key)); + const newWithErrors = Object.keys(newDataTree).reduce( + (acc: Record, key: string) => { + const widgetValue = newDataTree[key] as WidgetEntity; - return acc; - }, withErrors); + acc[key] = { + __evaluation__: { + errors: widgetValue.__evaluation__?.errors, + }, + }; + + return acc; + }, + {}, + ); + + // Process each path + constrainedDiffPaths.forEach((path: string) => { + if (path.length === 0) return; + + const pathParts = path.split("."); + + while (pathParts.length > 0) { + if (pathParts.length === 1) { + const newValue = get(newDataTree, pathParts); + const oldValue = get(oldDataTree, pathParts); + + if (newValue === undefined && oldValue === undefined) { + break; + } + + if (newValue !== undefined && oldValue !== undefined) { + set(newWithErrors, pathParts, newValue); + set(oldWithErrors, pathParts, oldValue); + break; + } + + newValue !== undefined + ? set(newWithErrors, pathParts, newValue) + : set(oldWithErrors, pathParts, oldValue); + break; + } + + const newValue = get(newDataTree, pathParts); + const oldValue = get(oldDataTree, pathParts); + + // found a defined ancestor + if (newValue !== undefined && oldValue !== undefined) { + set(newWithErrors, pathParts, newValue); + set(oldWithErrors, pathParts, oldValue); + break; + } + + pathParts.pop(); + } + }); + + return { + oldData: oldWithErrors as DataTree, + newData: newWithErrors as DataTree, + }; }; + const generateDiffUpdates = ( oldDataTree: DataTree, dataTree: DataTree, @@ -212,9 +273,13 @@ const generateDiffUpdates = ( const attachDirectly: Diff[] = []; const attachLater: Diff[] = []; - // we are reducing the data tree to only the paths that are being diffed - const oldData = getReducedDataTree(oldDataTree, constrainedDiffPaths); - const newData = getReducedDataTree(dataTree, constrainedDiffPaths); + // Get reduced data trees for both old and new states + const { newData, oldData } = getReducedDataTrees( + oldDataTree, + dataTree, + constrainedDiffPaths, + ); + const updates = diff(oldData, newData, (path, key) => { if (!path.length || key === "__evaluation__") return false; @@ -399,6 +464,7 @@ export const generateSerialisedUpdates = ( mergeAdditionalUpdates?: any, ): { serialisedUpdates: string; + updates: Diff[]; error?: { type: string; message: string }; } => { const updates = generateOptimisedUpdates( @@ -417,10 +483,11 @@ export const generateSerialisedUpdates = ( try { // serialise bigInt values and convert the updates to a string over here to minismise the cost of transfer // to the main thread. In the main thread parse this object there. - return { serialisedUpdates: serialiseToBigInt(removedLhs) }; + return { serialisedUpdates: serialiseToBigInt(removedLhs), updates }; } catch (error) { return { serialisedUpdates: "[]", + updates: [], error: { type: EvalErrorTypes.SERIALIZATION_ERROR, message: (error as Error).message, @@ -429,28 +496,85 @@ export const generateSerialisedUpdates = ( } }; -export const generateOptimisedUpdatesAndSetPrevState = ( +export function generateOptimisedUpdatesAndSetPrevState( dataTree: DataTree, - // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any dataTreeEvaluator: any, - constrainedDiffPaths: string[], - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - mergeAdditionalUpdates?: any, -) => { + affectedNodePaths: string[], + additionalUpdates?: Diff[], + isUpdateCycle?: boolean, +): string { const { error, serialisedUpdates } = generateSerialisedUpdates( dataTreeEvaluator?.getPrevState() || {}, dataTree, - constrainedDiffPaths, - mergeAdditionalUpdates, + affectedNodePaths, + additionalUpdates, ); if (error && dataTreeEvaluator?.errors) { dataTreeEvaluator.errors.push(error); } - dataTreeEvaluator?.setPrevState(dataTree); + updatePrevState( + isUpdateCycle, + dataTreeEvaluator, + serialisedUpdates, + dataTree, + ); return serialisedUpdates; -}; +} + +export function updatePrevState( + isUpdateCycle: boolean | undefined, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + dataTreeEvaluator: any, + serialisedUpdates: string, + dataTree: DataTree, +): void { + const updates = JSON.parse(serialisedUpdates); + + if (isUpdateCycle) { + const updatedState = create(dataTreeEvaluator?.getPrevState(), (draft) => { + updates.forEach((update: Diff) => { + try { + applyChange(draft, undefined, update); + } catch (e) { + const error = e as Error; + + // Push error to dataTreeEvaluator.errors with just the error message + if (dataTreeEvaluator?.errors) { + dataTreeEvaluator.errors.push({ + type: EvalErrorTypes.UPDATE_DATA_TREE_ERROR, + message: error.message, + }); + } + } + }); + }); + + dataTreeEvaluator?.setPrevState(updatedState); + } else { + dataTreeEvaluator?.setPrevState(dataTree); + } +} + +export function updateEvalProps(dataTreeEvaluator: DataTreeEvaluator) { + if (!dataTreeEvaluator) return null; + + const evalProps = dataTreeEvaluator.evalProps; + + return create(dataTreeEvaluator.evalTree, (draft) => { + for (const [entityName, entityEvalProps] of Object.entries(evalProps)) { + if (!entityEvalProps.__evaluation__) continue; + + set( + draft[entityName], + "__evaluation__", + klona({ + errors: entityEvalProps.__evaluation__.errors, + }), + ); + } + }); +}