From e151524d1c4b462054f3baf3f3e95e351cd9a137 Mon Sep 17 00:00:00 2001 From: Vemparala Surya Vamsi <121419957+vsvamsi1@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:37:46 +0530 Subject: [PATCH 1/2] feat: constrained diff between states (#31847) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description We are constraining the diff between the old dataTree and the newDataTree to its only affected nodes. The affected nodes list is the evalOrder, unevalUpdates and updatedValuePaths (which represents localStorage and setter updates). Through limiting the diff to the affected node list this should help in reducing the diff computation latency and overall improve the performance of each evaluation cycle. We are also cleaning up code related to compressing and decompressing updates since we deprecated that functionality. Fixes #31272 ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!IMPORTANT] > Workflow run: > Commit: `cb925afbb99577947fbc7233a35e4454223b402a` > Cypress dashboard url: Click here! > All cypress tests have passed πŸŽ‰πŸŽ‰πŸŽ‰ ## Summary by CodeRabbit - **New Features** - Introduced new logic for handling widget state updates efficiently. - Enhanced evaluation logic to support optimized diff updates and serialization. - Improved data tree generation for error handling and evaluation order. - **Bug Fixes** - Addressed issues in generating optimized updates for large collections and handling `undefined` values. - Fixed Cypress tests in the "PgAdmin Clone App" spec for more reliable interaction with UI elements. - **Refactor** - Simplified the logic for widget state cleanup and evaluation paths handling. - Removed unused variables and functions related to widget meta properties and evaluation paths. - **Tests** - Added new spec file `PgAdmin_spec.js` to Cypress limited tests for client-side regression testing. - **Chores** - Updated type imports across various files to reflect changes in evaluation logic. --- .../e2e/Regression/Apps/PgAdmin_spec.js | 2 + app/client/cypress/limited-tests.txt | 1 - app/client/src/actions/evaluationActions.ts | 6 +- .../__tests__/dataTreeUtils.test.ts | 121 +- .../ce/workers/Evaluation/dataTreeUtils.ts | 27 +- .../evaluationReducers/treeReducer.ts | 26 +- app/client/src/sagas/EvaluationSaga.utils.ts | 5 +- .../__tests__/generateOpimisedUpdates.test.ts | 1004 ++++++++--------- .../workers/Evaluation/evalTreeWithChanges.ts | 23 +- .../src/workers/Evaluation/fns/resetWidget.ts | 3 - .../workers/Evaluation/handlers/evalTree.ts | 47 +- app/client/src/workers/Evaluation/helpers.ts | 260 +++-- .../workers/common/DataTreeEvaluator/index.ts | 79 +- .../DataTreeEvaluator/validationUtils.ts | 66 +- 14 files changed, 742 insertions(+), 928 deletions(-) diff --git a/app/client/cypress/e2e/Regression/Apps/PgAdmin_spec.js b/app/client/cypress/e2e/Regression/Apps/PgAdmin_spec.js index 2569414d18..65cc8b4ba2 100644 --- a/app/client/cypress/e2e/Regression/Apps/PgAdmin_spec.js +++ b/app/client/cypress/e2e/Regression/Apps/PgAdmin_spec.js @@ -2,6 +2,7 @@ import { agHelper, deployMode, dataSources, + assertHelper, locators, draggableWidgets, } from "../../../support/Objects/ObjectsCore"; @@ -86,6 +87,7 @@ describe("PgAdmin Clone App", { tags: ["@tag.Datasource"] }, function () { cy.xpath(appPage.submitButton).click({ force: true }); agHelper.AssertElementVisibility(appPage.addColumn); cy.xpath(appPage.submitButton).first().click({ force: true }); + assertHelper.AssertNetworkStatus("@postExecute"); cy.xpath(appPage.closeButton).click({ force: true }); cy.xpath(appPage.addNewtable).should("be.visible"); // viewing the table's columns by clicking on view button diff --git a/app/client/cypress/limited-tests.txt b/app/client/cypress/limited-tests.txt index 4807f10528..31c0c0253f 100644 --- a/app/client/cypress/limited-tests.txt +++ b/app/client/cypress/limited-tests.txt @@ -1,7 +1,6 @@ # To run only limited tests - give the spec names in below format: cypress/e2e/Regression/ClientSide/Templates/Fork_Template_spec.js - # For running all specs - uncomment below: #cypress/e2e/**/**/* diff --git a/app/client/src/actions/evaluationActions.ts b/app/client/src/actions/evaluationActions.ts index 0ae3941c27..a67bb783b6 100644 --- a/app/client/src/actions/evaluationActions.ts +++ b/app/client/src/actions/evaluationActions.ts @@ -4,7 +4,7 @@ import { intersection } from "lodash"; import type { DependencyMap } from "utils/DynamicBindingUtils"; import type { QueryActionConfig } from "entities/Action"; import type { DatasourceConfiguration } from "entities/Datasource"; -import type { DiffWithReferenceState } from "workers/Evaluation/helpers"; +import type { DiffWithNewTreeState } from "workers/Evaluation/helpers"; import { EVALUATE_REDUX_ACTIONS, EVAL_AND_LINT_REDUX_ACTIONS, @@ -57,8 +57,8 @@ export function shouldLog(action: ReduxAction) { } export const setEvaluatedTree = ( - updates: DiffWithReferenceState[], -): ReduxAction<{ updates: DiffWithReferenceState[] }> => { + updates: DiffWithNewTreeState[], +): ReduxAction<{ updates: DiffWithNewTreeState[] }> => { return { type: ReduxActionTypes.SET_EVALUATED_TREE, payload: { updates }, diff --git a/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts b/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts index 9abf1b5281..27c771f9c6 100644 --- a/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts +++ b/app/client/src/ce/workers/Evaluation/__tests__/dataTreeUtils.test.ts @@ -1,8 +1,5 @@ import type { DataTree } from "entities/DataTree/dataTreeTypes"; import { makeEntityConfigsAsObjProperties } from "@appsmith/workers/Evaluation/dataTreeUtils"; -import { smallDataSet } from "workers/Evaluation/__tests__/generateOpimisedUpdates.test"; -import produce from "immer"; -import { cloneDeep } from "lodash"; const unevalTreeFromMainThread = { Api2: { @@ -131,122 +128,22 @@ const unevalTreeFromMainThread = { describe("7. Test util methods", () => { describe("makeDataTreeEntityConfigAsProperty", () => { - it("should not introduce __evaluation__ property", () => { + it("should not introduce __evaluation__ property to the widget state when the corresponding evalProps isn't there for a widget", () => { const dataTree = makeEntityConfigsAsObjProperties( unevalTreeFromMainThread as unknown as DataTree, ); expect(dataTree.Api2).not.toHaveProperty("__evaluation__"); }); - describe("identicalEvalPathsPatches decompress updates", () => { - it("should decompress identicalEvalPathsPatches updates into evalProps and not in state", () => { - const state = { - Table1: { - filteredTableData: smallDataSet, - selectedRows: [], - pageSize: 0, - }, - } as any; + it("should introduce __evaluation__ property to the widget state when it has a corresponding evalProps", () => { + const dataTree = makeEntityConfigsAsObjProperties( + unevalTreeFromMainThread as unknown as DataTree, + { + evalProps: { Api2: { __evaluation__: { errors: { someVal: [] } } } }, + }, + ); - const identicalEvalPathsPatches = { - "Table1.__evaluation__.evaluatedValues.['filteredTableData']": - "Table1.filteredTableData", - }; - const evalProps = { - Table1: { - __evaluation__: { - evaluatedValues: { - someProp: "abc", - }, - errors: { - someProp1: "abc", - }, - }, - }, - } as any; - const dataTree = makeEntityConfigsAsObjProperties(state as any, { - sanitizeDataTree: true, - evalProps, - identicalEvalPathsPatches, - }); - // only errors should be attached to dataTree evaluatedValues should be excluded - const expectedState = produce(state, (draft: any) => { - draft.Table1.__evaluation__ = { errors: { someProp1: "abc" } }; - }); - - expect(dataTree).toEqual(expectedState); - //evalProps should have decompressed updates in it coming from identicalEvalPathsPatches - const expectedEvalProps = produce(evalProps, (draft: any) => { - draft.Table1.__evaluation__.evaluatedValues.filteredTableData = - smallDataSet; - }); - expect(evalProps).toEqual(expectedEvalProps); - }); - - it("should not make any updates to evalProps when the identicalEvalPathsPatches is empty", () => { - const state = { - Table1: { - filteredTableData: smallDataSet, - selectedRows: [], - pageSize: 0, - }, - } as any; - - const identicalEvalPathsPatches = {}; - const initialEvalProps = {} as any; - const evalProps = cloneDeep(initialEvalProps); - const dataTree = makeEntityConfigsAsObjProperties(state, { - sanitizeDataTree: true, - evalProps, - identicalEvalPathsPatches, - }); - - expect(dataTree).toEqual(dataTree); - //evalProps not be mutated with any updates - expect(evalProps).toEqual(initialEvalProps); - }); - - it("should ignore non relevant identicalEvalPathsPatches updates into evalProps and state", () => { - const state = { - Table1: { - filteredTableData: smallDataSet, - selectedRows: [], - pageSize: 0, - }, - } as any; - //ignore non existent widget state - const identicalEvalPathsPatches = { - "SomeWidget.__evaluation__.evaluatedValues.['filteredTableData']": - "SomeWidget.filteredTableData", - }; - - const initialEvalProps = { - Table1: { - __evaluation__: { - evaluatedValues: { - someProp: "abc", - }, - errors: { - someProp1: "efg", - }, - }, - }, - } as any; - const evalProps = cloneDeep(initialEvalProps); - const dataTree = makeEntityConfigsAsObjProperties(state, { - sanitizeDataTree: true, - evalProps, - identicalEvalPathsPatches, - }); - const expectedState = produce(state, (draft: any) => { - // evaluatedValues should not be attached to dataTree only errors should be attached - draft.Table1.__evaluation__ = { errors: { someProp1: "efg" } }; - }); - - expect(dataTree).toEqual(expectedState); - //evalProps not be mutated with any updates - expect(evalProps).toEqual(initialEvalProps); - }); + expect(dataTree.Api2).toHaveProperty("__evaluation__"); }); }); }); diff --git a/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts b/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts index be698729a5..d8756fb566 100644 --- a/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts +++ b/app/client/src/ce/workers/Evaluation/dataTreeUtils.ts @@ -1,5 +1,5 @@ import type { DataTree } from "entities/DataTree/dataTreeTypes"; -import { get, isObject, set } from "lodash"; +import { isObject, set } from "lodash"; import { klona } from "klona/json"; import type { EvalProps } from "workers/common/DataTreeEvaluator"; @@ -12,14 +12,9 @@ export function makeEntityConfigsAsObjProperties( option = {} as { sanitizeDataTree?: boolean; evalProps?: EvalProps; - identicalEvalPathsPatches?: Record; }, ): DataTree { - const { - evalProps, - identicalEvalPathsPatches, - sanitizeDataTree = true, - } = option; + const { evalProps, sanitizeDataTree = true } = option; const newDataTree: DataTree = {}; for (const entityName of Object.keys(dataTree)) { const entity = dataTree[entityName]; @@ -31,24 +26,6 @@ export function makeEntityConfigsAsObjProperties( if (!evalProps) return dataTreeToReturn; - //clean up deletes widget states - Object.entries(identicalEvalPathsPatches || {}).forEach( - ([evalPath, statePath]) => { - const [entity] = statePath.split("."); - if (!dataTreeToReturn[entity]) { - delete identicalEvalPathsPatches?.[evalPath]; - } - }, - ); - - // decompressIdenticalEvalPaths - Object.entries(identicalEvalPathsPatches || {}).forEach( - ([evalPath, statePath]) => { - const referencePathValue = get(dataTreeToReturn, statePath); - set(evalProps, evalPath, referencePathValue); - }, - ); - for (const [entityName, entityEvalProps] of Object.entries(evalProps)) { if (!entityEvalProps.__evaluation__) continue; set( diff --git a/app/client/src/reducers/evaluationReducers/treeReducer.ts b/app/client/src/reducers/evaluationReducers/treeReducer.ts index 7edc07137b..0113854787 100644 --- a/app/client/src/reducers/evaluationReducers/treeReducer.ts +++ b/app/client/src/reducers/evaluationReducers/treeReducer.ts @@ -1,12 +1,10 @@ import type { ReduxAction } from "@appsmith/constants/ReduxActionConstants"; import { ReduxActionTypes } from "@appsmith/constants/ReduxActionConstants"; -import type { Diff } from "deep-diff"; import { applyChange } from "deep-diff"; import type { DataTree } from "entities/DataTree/dataTreeTypes"; import { createImmerReducer } from "utils/ReducerUtils"; import * as Sentry from "@sentry/react"; -import { get } from "lodash"; -import type { DiffWithReferenceState } from "workers/Evaluation/helpers"; +import type { DiffWithNewTreeState } from "workers/Evaluation/helpers"; export type EvaluatedTreeState = DataTree; @@ -17,7 +15,7 @@ const evaluatedTreeReducer = createImmerReducer(initialState, { state: EvaluatedTreeState, action: ReduxAction<{ dataTree: DataTree; - updates: DiffWithReferenceState[]; + updates: DiffWithNewTreeState[]; removedPaths: [string]; }>, ) => { @@ -26,23 +24,13 @@ const evaluatedTreeReducer = createImmerReducer(initialState, { return state; } for (const update of updates) { - // Null check for typescript - if (!Array.isArray(update.path) || update.path.length === 0) { - continue; - } try { - //these are the decompression updates, there are cases where identical values are present in the state - //over here we have the path which has the identical value and apply as an update - if (update.kind === "referenceState") { - const { path, referencePath } = update; - - const patch = { - kind: "N", - path, - rhs: get(state, referencePath), - } as Diff; - applyChange(state, undefined, patch); + if (update.kind === "newTree") { + return update.rhs; } else { + if (!update.path || update.path.length === 0) { + continue; + } applyChange(state, undefined, update); } } catch (e) { diff --git a/app/client/src/sagas/EvaluationSaga.utils.ts b/app/client/src/sagas/EvaluationSaga.utils.ts index 2d6cc7c52f..b1aea041ea 100644 --- a/app/client/src/sagas/EvaluationSaga.utils.ts +++ b/app/client/src/sagas/EvaluationSaga.utils.ts @@ -1,9 +1,9 @@ import log from "loglevel"; -import type { DiffWithReferenceState } from "workers/Evaluation/helpers"; +import type { DiffWithNewTreeState } from "workers/Evaluation/helpers"; export const parseUpdatesAndDeleteUndefinedUpdates = ( updates: string, -): DiffWithReferenceState[] => { +): DiffWithNewTreeState[] => { let parsedUpdates = []; try { //Parse updates from a string @@ -12,6 +12,7 @@ export const parseUpdatesAndDeleteUndefinedUpdates = ( log.error("Failed to parse updates", e, updates); return []; } + //delete all undefined properties from the state const { deleteUpdates, regularUpdates } = parsedUpdates.reduce( (acc: any, curr: any) => { diff --git a/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts b/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts index 1fe77f4ab6..eb4d00a0aa 100644 --- a/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/generateOpimisedUpdates.test.ts @@ -1,5 +1,5 @@ -/* eslint-disable @typescript-eslint/no-empty-function */ import { applyChange } from "deep-diff"; +import type { DataTree } from "entities/DataTree/dataTreeTypes"; import produce from "immer"; import { klona } from "klona/full"; import { range } from "lodash"; @@ -27,9 +27,9 @@ export const smallDataSet = [ }, ]; //size of about 300 elements -const largeDataSet = range(100).flatMap(() => smallDataSet) as any; +const largeDataSet = range(100).flatMap(() => smallDataSet); -const oldState = { +const oldState: DataTree = { Table1: { ENTITY_TYPE: "WIDGET", primaryColumns: { @@ -49,14 +49,19 @@ const oldState = { tableData: [], processedTableData: [], }, - evaluatedValues: { - filteredTableData: smallDataSet, - transientTableData: {}, - tableData: [], - processedTableData: [], - "primaryColumns.customColumn2.isDisabled": false, - }, }, + meta: {}, + widgetId: "232", + widgetName: "Table", + renderMode: "PAGE", + version: 1, + parentColumnSpace: 121, + parentRowSpace: 123, + leftColumn: 123, + rightColumn: 123, + topRow: 0, + bottomRow: 0, + isLoading: false, }, Select1: { value: "", @@ -76,556 +81,537 @@ const oldState = { errors: { options: [], }, - evaluatedValues: { - "meta.value": "", - value: "", - }, }, + meta: {}, + widgetId: "232", + widgetName: "Table", + renderMode: "PAGE", + version: 1, + parentColumnSpace: 121, + parentRowSpace: 123, + leftColumn: 123, + rightColumn: 123, + topRow: 0, + bottomRow: 0, + isLoading: false, }, }; describe("generateOptimisedUpdates", () => { describe("regular diff", () => { - test("should generate regular diff updates when a simple property changes in the widget property segment", () => { - const newState = produce(oldState, (draft) => { + test("should not generate any diff when the constrainedDiffPaths is empty", () => { + const newState = produce(oldState, (draft: any) => { draft.Table1.pageSize = 17; }); - const updates = generateOptimisedUpdates(oldState, newState); + const updates = generateOptimisedUpdates(oldState, newState, []); + // no diff should be generated + expect(updates).toEqual([]); + }); + test("should not generate any diff when the constrainedDiffPaths nodes are the same ", () => { + const newState = produce(oldState, (draft: any) => { + //making an unrelated change + draft.Table1.triggerRowSelection = true; + }); + const updates = generateOptimisedUpdates(oldState, newState, [ + "Table1.pageSize", + ]); + // no diff should be generated and the the unrealted change should be ignored + expect(updates).toEqual([]); + }); + test("should generate regular diff updates when a simple property changes in the widget property segment", () => { + const newState = produce(oldState, (draft: any) => { + draft.Table1.pageSize = 17; + }); + const updates = generateOptimisedUpdates(oldState, newState, [ + "Table1.pageSize", + ]); expect(updates).toEqual([ { kind: "E", path: ["Table1", "pageSize"], lhs: 0, rhs: 17 }, ]); }); test("should generate regular diff updates when a simple property changes in the __evaluation__ segment ", () => { const validationError = "Some validation error"; - const newState = produce(oldState, (draft) => { - draft.Table1.__evaluation__.evaluatedValues.tableData = - validationError as any; + const newState = produce(oldState, (draft: any) => { + draft.Table1.__evaluation__.errors.tableData = validationError; }); - const updates = generateOptimisedUpdates(oldState, newState); + const updates = generateOptimisedUpdates(oldState, newState, [ + "Table1.__evaluation__.errors.tableData", + ]); expect(updates).toEqual([ { kind: "E", - path: ["Table1", "__evaluation__", "evaluatedValues", "tableData"], + path: ["Table1", "__evaluation__", "errors", "tableData"], lhs: [], rhs: validationError, }, ]); }); - }); - - describe("diffs with identicalEvalPathsPatches", () => { - test("should not generate any updates when both the states are the same", () => { - const updates = generateOptimisedUpdates(oldState, oldState); - expect(updates).toEqual([]); - const identicalEvalPathsPatches = { - "Table1.__evaluation__.evaluatedValues.['tableData']": - "Table1.tableData", - }; - const updatesWithCompressionMap = generateOptimisedUpdates( - oldState, - oldState, - identicalEvalPathsPatches, - ); - expect(updatesWithCompressionMap).toEqual([]); - }); - test("should generate the correct table data updates and reference state patches", () => { - const identicalEvalPathsPatches = { - "Table1.__evaluation__.evaluatedValues.['tableData']": - "Table1.tableData", - }; - const newState = produce(oldState, (draft) => { + test("should generate a replace collection patch when the size of the collection exceeds 100 instead of generating granular updates", () => { + const newState = produce(oldState, (draft: any) => { draft.Table1.tableData = largeDataSet; }); - const updates = generateOptimisedUpdates( - oldState, - newState, - identicalEvalPathsPatches, - ); - expect(updates).toEqual([ - { kind: "N", path: ["Table1", "tableData"], rhs: largeDataSet }, - { - kind: "referenceState", - path: ["Table1", "__evaluation__", "evaluatedValues", "tableData"], - referencePath: "Table1.tableData", - }, + const updates = generateOptimisedUpdates(oldState, newState, [ + "Table1.tableData", ]); - }); - test("should not generate granular updates and generate a patch which replaces the complete collection when any change is made to the large collection ", () => { - const largeDataSetWithSomeSimpleChange = produce( - largeDataSet, - (draft: any) => { - //making a change to the first row of the collection - draft[0].address = "some new address"; - //making a change to the second row of the collection - draft[1].address = "some other new address"; - }, - ) as any; - const identicalEvalPathsPatches = { - "Table1.__evaluation__.evaluatedValues.['tableData']": - "Table1.tableData", - }; - const evalVal = "some eval value" as any; - - const newState = produce(oldState, (draft) => { - //this value eval value should be ignores since we have provided identicalEvalPathsPatches which takes precedence - draft.Table1.__evaluation__.evaluatedValues.tableData = evalVal as any; - draft.Table1.tableData = largeDataSetWithSomeSimpleChange; - }); - - const updates = generateOptimisedUpdates( - oldState, - newState, - identicalEvalPathsPatches, - ); - //should not see evalVal since identical eval path patches takes precedence - expect(JSON.stringify(updates)).not.toContain(evalVal); expect(updates).toEqual([ { kind: "N", path: ["Table1", "tableData"], - //we should not see granular updates but complete replacement - rhs: largeDataSetWithSomeSimpleChange, - }, - { - //compression patch - kind: "referenceState", - path: ["Table1", "__evaluation__", "evaluatedValues", "tableData"], - referencePath: "Table1.tableData", + rhs: largeDataSet, }, ]); }); - test("should not generate compression patches when there are no identical eval paths provided ", () => { - const tableDataEvaluationValue = "someValidation error"; - const largeDataSetWithSomeSimpleChange = produce( - largeDataSet, - (draft: any) => { - //making a change to the first row of the collection - draft[0].address = "some new address"; - //making a change to the second row of the collection - draft[1].address = "some other new address"; - }, - ) as any; - // empty indentical eval paths - const identicalEvalPathsPatches = {}; - const newState = produce(oldState, (draft) => { - draft.Table1.tableData = largeDataSetWithSomeSimpleChange; - draft.Table1.__evaluation__.evaluatedValues.tableData = - tableDataEvaluationValue as any; - }); - const updates = generateOptimisedUpdates( - oldState, - newState, - identicalEvalPathsPatches, - ); - expect(updates).toEqual([ - { - //considered as regular diff since there are no identical eval paths provided - kind: "E", - path: ["Table1", "__evaluation__", "evaluatedValues", "tableData"], - lhs: [], - rhs: tableDataEvaluationValue, - }, - { - kind: "N", - path: ["Table1", "tableData"], - //we should not see granular updates but complete replacement - rhs: largeDataSetWithSomeSimpleChange, - }, - ]); - }); - test("should not generate any update when the new state has the same value as the old state", () => { - const oldStateSetWithSomeData = produce(oldState, (draft) => { - draft.Table1.tableData = largeDataSet; - draft.Table1.__evaluation__.evaluatedValues.tableData = largeDataSet; - }); - const identicalEvalPathsPatches = { - "Table1.__evaluation__.evaluatedValues.['tableData']": - "Table1.tableData", - }; - const newStateSetWithTheSameData = produce(oldState, (draft) => { - //deliberating making a new instance of largeDataSet - draft.Table1.tableData = [...largeDataSet] as any; - }); - //since the old state has the same value as the new value..we wont generate a patch unnecessarily... - const updates = generateOptimisedUpdates( - oldStateSetWithSomeData, - newStateSetWithTheSameData, - identicalEvalPathsPatches, - ); - expect(updates).toEqual([]); - }); - }); -}); - -//we are testing the flow of serialised updates generated from the worker thread and subsequently applied to the main thread state -describe("generateSerialisedUpdates and parseUpdatesAndDeleteUndefinedUpdates", () => { - it("should ignore undefined updates", () => { - const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { - draft.Table1.pageSize = undefined; - }); - - const { serialisedUpdates } = generateSerialisedUpdates( - oldStateWithUndefinedValues, - //new state has the same undefined value - oldStateWithUndefinedValues, - {}, - ); - //no change hence empty array - expect(serialisedUpdates).toEqual("[]"); - }); - it("should generate a delete patch when a property is transformed to undefined", () => { - const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { - draft.Table1.pageSize = undefined; - }); - - const { serialisedUpdates } = generateSerialisedUpdates( - oldState, - oldStateWithUndefinedValues, - {}, - ); - const parsedUpdates = - parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); - expect(parsedUpdates).toEqual([ - { - kind: "D", - path: ["Table1", "pageSize"], - }, - ]); - }); - it("should generate an error when there is a serialisation error", () => { - const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { - //generate a cyclical object - draft.Table1.filteredTableData = draft.Table1; - }); - const { error, serialisedUpdates } = generateSerialisedUpdates( - oldState, - oldStateWithUndefinedValues, - {}, - ); - - expect(error?.type).toEqual(EvalErrorTypes.SERIALIZATION_ERROR); - //when a serialisation error occurs we should not return an error - expect(serialisedUpdates).toEqual("[]"); - }); - //when functions are serialised they become undefined and these updates should be deleted from the state - describe("clean out all functions in the generated state", () => { - it("should clean out new function properties added to the generated state", () => { - const newStateWithSomeFnProperty = produce(oldState, (draft: any) => { - draft.Table1.someFn = () => {}; - draft.Table1.__evaluation__.evaluatedValues.someEvalFn = () => {}; - }); - - const { serialisedUpdates } = generateSerialisedUpdates( - oldState, - newStateWithSomeFnProperty, - {}, - ); - - const parsedUpdates = - parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); - //should ignore all function updates - expect(parsedUpdates).toEqual([]); - - const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { - parsedUpdates.forEach((v: any) => { - applyChange(draft, undefined, v); + describe("undefined value updates in a collection", () => { + test("should generate replace patch when a single node is set to undefined in a collection", () => { + const statWithLargeCollection = produce(oldState, (draft: any) => { + draft.Table1.tableData = ["a", "b"]; }); - }); - //no change in state - expect(parseAndApplyUpdatesToOldState).toEqual(oldState); - }); - - it("should delete properties which get updated to a function", () => { - const newStateWithSomeFnProperty = produce(oldState, (draft: any) => { - draft.Table1.pageSize = () => {}; - draft.Table1.__evaluation__.evaluatedValues.transientTableData = - () => {}; - }); - - const { serialisedUpdates } = generateSerialisedUpdates( - oldState, - newStateWithSomeFnProperty, - {}, - ); - - const parsedUpdates = - parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); - - expect(parsedUpdates).toEqual([ - { - kind: "D", - path: ["Table1", "pageSize"], - }, - { - kind: "D", - path: [ - "Table1", - "__evaluation__", - "evaluatedValues", - "transientTableData", - ], - }, - ]); - - const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { - parsedUpdates.forEach((v: any) => { - applyChange(draft, undefined, v); - }); - }); - const expectedState = produce(oldState, (draft: any) => { - delete draft.Table1.pageSize; - delete draft.Table1.__evaluation__.evaluatedValues.transientTableData; - }); - - expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); - }); - it("should delete function properties which get updated to undefined", () => { - const oldStateWithSomeFnProperty = produce(oldState, (draft: any) => { - // eslint-disable-next-line @typescript-eslint/no-empty-function - draft.Table1.pageSize = () => {}; - draft.Table1.__evaluation__.evaluatedValues.transientTableData = - // eslint-disable-next-line @typescript-eslint/no-empty-function - () => {}; - }); - const newStateWithFnsTransformedToUndefined = produce( - oldState, - (draft: any) => { - draft.Table1.pageSize = undefined; - draft.Table1.__evaluation__.evaluatedValues.transientTableData = - undefined; - }, - ); - - const { serialisedUpdates } = generateSerialisedUpdates( - oldStateWithSomeFnProperty, - newStateWithFnsTransformedToUndefined, - {}, - ); - - const parsedUpdates = - parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); - - expect(parsedUpdates).toEqual([ - { - kind: "D", - path: ["Table1", "pageSize"], - }, - { - kind: "D", - path: [ - "Table1", - "__evaluation__", - "evaluatedValues", - "transientTableData", - ], - }, - ]); - - const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { - parsedUpdates.forEach((v: any) => { - applyChange(draft, undefined, v); - }); - }); - const expectedState = produce(oldState, (draft: any) => { - delete draft.Table1.pageSize; - delete draft.Table1.__evaluation__.evaluatedValues.transientTableData; - }); - - expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); - }); - }); - - it("should serialise bigInteger values", () => { - const someBigInt = BigInt(121221); - const newStateWithBigInt = produce(oldState, (draft: any) => { - draft.Table1.pageSize = someBigInt; - }); - const { serialisedUpdates } = generateSerialisedUpdates( - oldState, - newStateWithBigInt, - {}, - ); - - const parsedUpdates = - parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); - - //should generate serialised bigInt update - expect(parsedUpdates).toEqual([ - { - kind: "E", - path: ["Table1", "pageSize"], - rhs: "121221", - }, - ]); - - const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { - parsedUpdates.forEach((v: any) => { - applyChange(draft, undefined, v); - }); - }); - const expectedState = produce(oldState, (draft: any) => { - draft.Table1.pageSize = "121221"; - }); - - expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); - }); - describe("serialise momement updates directly", () => { - test("should generate a null update when it sees an invalid moment object", () => { - const newState = produce(oldState, (draft) => { - draft.Table1.pageSize = moment("invalid value") as any; - }); - const { serialisedUpdates } = generateSerialisedUpdates( - oldState, - newState, - {}, - ); - const serialisedExpectedOutput = JSON.stringify([ - { kind: "E", rhs: null, path: ["Table1", "pageSize"] }, - ]); - expect(serialisedUpdates).toEqual(serialisedExpectedOutput); - }); - test("should generate a regular update when it sees a valid moment object", () => { - const validMoment = moment(); - const newState = produce(oldState, (draft) => { - draft.Table1.pageSize = validMoment as any; - }); - const { serialisedUpdates } = generateSerialisedUpdates( - oldState, - newState, - {}, - ); - const serialisedExpectedOutput = JSON.stringify([ - { kind: "E", rhs: validMoment, path: ["Table1", "pageSize"] }, - ]); - expect(serialisedUpdates).toEqual(serialisedExpectedOutput); - }); - }); - // we are testing a flow from worker thread diff updates to being applied to the main thread's state - describe("test main thread update flow", () => { - //this function takes in serialised updates from the webworker and applies it to the main thread's state - function generateMainThreadStateFromSerialisedUpdates( - serialisedUpdates: any, - prevState: any, - ) { - const parsedUpdates = - parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); - return produce(prevState, (draft: any) => { - parsedUpdates.forEach((v: any) => { - applyChange(draft, undefined, v); - }); - }); - } - let workerStateWithCollection: any; - let mainThreadStateWithCollection: any; - const someDate = "2023-12-07T19:05:11.830Z"; - test("large moment collection updates should be serialised, we should always see ISO string and no moment object properties", () => { - const largeCollection = [] as any; - for (let i = 0; i < 110; i++) { - largeCollection.push({ i, c: moment(someDate) }); - } - //attaching a collection to some property in the workerState - workerStateWithCollection = produce(oldState, (draft) => { - draft.Table1.pageSize = largeCollection as any; - }); - //generate serialised diff updates - const { serialisedUpdates } = generateSerialisedUpdates( - oldState, - workerStateWithCollection, - {}, - ); - // parsing the updates generated by worker and applying it back to the main threadState - mainThreadStateWithCollection = - generateMainThreadStateFromSerialisedUpdates( - serialisedUpdates, - oldState, + const newStateWithAnElementDeleted = produce( + statWithLargeCollection, + (draft: any) => { + draft.Table1.tableData = ["a", undefined]; + }, + ); + const updates = generateOptimisedUpdates( + statWithLargeCollection, + newStateWithAnElementDeleted, + ["Table1.tableData[1]"], ); - const expectedMainThreadState = produce(oldState, (draft) => { - draft.Table1.pageSize = JSON.parse( - JSON.stringify(largeCollection), - ) as any; + expect(updates).toEqual([ + { + kind: "E", + lhs: ["a", "b"], + path: ["Table1", "tableData"], + rhs: ["a", undefined], + }, + ]); }); - //check first value has the correct date - expect(mainThreadStateWithCollection.Table1.pageSize[0].c).toEqual( - someDate, - ); + test("should generate generate regular diff updates for non undefined updates in a collection", () => { + const statWithLargeCollection = produce(oldState, (draft: any) => { + draft.Table1.tableData = ["a", "b"]; + }); + const newStateWithAnElementDeleted = produce( + statWithLargeCollection, + (draft: any) => { + draft.Table1.tableData = ["a", "e"]; + }, + ); + const updates = generateOptimisedUpdates( + statWithLargeCollection, + newStateWithAnElementDeleted, + ["Table1.tableData[1]"], + ); - expect(mainThreadStateWithCollection).toEqual(expectedMainThreadState); + expect(updates).toEqual([ + { kind: "E", path: ["Table1", "tableData", 1], lhs: "b", rhs: "e" }, + ]); + }); }); - test("update in a single moment value in a collection should always be serialised ", () => { - const someNewDate = "2023-12-07T19:05:11.930Z"; - // updating a single value in the prev worker state - const updatedWorkerStateWithASingleValue = produce( - klona(workerStateWithCollection), - (draft: any) => { - draft.Table1.pageSize[0].c = moment(someNewDate) as any; - }, - ); + }); - //generate serialised diff updates + //we are testing the flow of serialised updates generated from the worker thread and subsequently applied to the main thread state + describe("generateSerialisedUpdates and parseUpdatesAndDeleteUndefinedUpdates", () => { + const additionalUpdates = [ + { kind: "E", path: ["Table1", "pageSize"], lhs: 0, rhs: 17 }, + ]; + + it("should merge additional updates with existing updates", () => { const { serialisedUpdates } = generateSerialisedUpdates( - workerStateWithCollection, - updatedWorkerStateWithASingleValue, {}, + {}, + [], + additionalUpdates, ); - - // parsing the updates generated by worker and applying it back to the main threadState - const updatedMainThreadState = - generateMainThreadStateFromSerialisedUpdates( - serialisedUpdates, - mainThreadStateWithCollection, - ) as any; - // check if the main thread state has the updated value - expect(updatedMainThreadState.Table1.pageSize[0].c).toEqual(someNewDate); - - const expectedMainThreadState = produce( - mainThreadStateWithCollection, - (draft: any) => { - draft.Table1.pageSize[0].c = JSON.parse( - JSON.stringify(moment(someNewDate)), - ) as any; - }, - ); - - expect(updatedMainThreadState).toEqual(expectedMainThreadState); + // we should only see additional updates + expect(serialisedUpdates).toEqual(JSON.stringify(additionalUpdates)); }); - test("update in a single moment value to an invalid value should always be serialised ", () => { - //some garbage value - const someNewDate = "fdfdfd"; - // updating a single value in the prev worker state - const updatedWorkerStateWithASingleValue = produce( - klona(workerStateWithCollection), - (draft: any) => { - draft.Table1.pageSize[0].c = moment(someNewDate) as any; - }, - ); + it("should ignore undefined updates", () => { + const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { + draft.Table1.pageSize = undefined; + }); - //generate serialised diff updates const { serialisedUpdates } = generateSerialisedUpdates( - workerStateWithCollection, - updatedWorkerStateWithASingleValue, - {}, + oldStateWithUndefinedValues, + //new state has the same undefined value + oldStateWithUndefinedValues, + ["Table1.pageSize"], + additionalUpdates, ); + //no change hence empty array + expect(serialisedUpdates).toEqual(JSON.stringify(additionalUpdates)); + }); + it("should generate a delete patch when a property is transformed to undefined", () => { + const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { + draft.Table1.pageSize = undefined; + }); - // parsing the updates generated by worker and applying it back to the main threadState - const updatedMainThreadState = - generateMainThreadStateFromSerialisedUpdates( - serialisedUpdates, - mainThreadStateWithCollection, - ) as any; - // check if the main thread state has the updated invalid value which should be null - expect(updatedMainThreadState.Table1.pageSize[0].c).toEqual(null); - - const expectedMainThreadState = produce( - mainThreadStateWithCollection, - (draft: any) => { - draft.Table1.pageSize[0].c = JSON.parse( - JSON.stringify(moment(someNewDate)), - ) as any; + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + oldStateWithUndefinedValues, + ["Table1.pageSize"], + [], + ); + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + expect(parsedUpdates).toEqual([ + { + kind: "D", + path: ["Table1", "pageSize"], }, + ]); + }); + it("should generate an error when there is a serialisation error", () => { + const oldStateWithUndefinedValues = produce(oldState, (draft: any) => { + //generate a cyclical object + draft.Table1.filteredTableData = draft.Table1; + }); + const { error, serialisedUpdates } = generateSerialisedUpdates( + oldState, + oldStateWithUndefinedValues, + ["Table1.filteredTableData"], + [], ); - expect(updatedMainThreadState).toEqual(expectedMainThreadState); + expect(error?.type).toEqual(EvalErrorTypes.SERIALIZATION_ERROR); + //when a serialisation error occurs we should not return an error + expect(serialisedUpdates).toEqual("[]"); + }); + + //when functions are serialised they become undefined and these updates should be deleted from the state + describe("clean out all functions in the generated state", () => { + it("should clean out new function properties added to the generated state", () => { + const newStateWithSomeFnProperty = produce(oldState, (draft: any) => { + draft.Table1.someFn = () => {}; + draft.Table1.__evaluation__.errors.someEvalFn = () => {}; + }); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newStateWithSomeFnProperty, + ["Table1.someFn", "Table1.__evaluation__.errors.someEvalFn"], + [], + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + //should delete all function updates + expect(parsedUpdates).toEqual([]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + //no change in state + expect(parseAndApplyUpdatesToOldState).toEqual(oldState); + }); + + it("should delete properties which get updated to a function", () => { + const newStateWithSomeFnProperty = produce(oldState, (draft: any) => { + draft.Table1.pageSize = () => {}; + draft.Table1.__evaluation__.errors.transientTableData = () => {}; + }); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newStateWithSomeFnProperty, + [ + "Table1.pageSize", + "Table1.__evaluation__.errors.transientTableData", + ], + [], + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + + expect(parsedUpdates).toEqual([ + { + kind: "D", + path: ["Table1", "__evaluation__", "errors", "transientTableData"], + }, + { + kind: "D", + path: ["Table1", "pageSize"], + }, + ]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + const expectedState = produce(oldState, (draft: any) => { + delete draft.Table1.pageSize; + delete draft.Table1.__evaluation__.errors.transientTableData; + }); + + expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); + }); + it("should delete function properties which get updated to undefined", () => { + const oldStateWithSomeFnProperty = produce(oldState, (draft: any) => { + // eslint-disable-next-line @typescript-eslint/no-empty-function + draft.Table1.pageSize = () => {}; + draft.Table1.__evaluation__.errors.transientTableData = + // eslint-disable-next-line @typescript-eslint/no-empty-function + () => {}; + }); + const newStateWithFnsTransformedToUndefined = produce( + oldState, + (draft: any) => { + draft.Table1.pageSize = undefined; + draft.Table1.__evaluation__.errors.transientTableData = undefined; + }, + ); + + const { serialisedUpdates } = generateSerialisedUpdates( + oldStateWithSomeFnProperty, + newStateWithFnsTransformedToUndefined, + [ + "Table1.pageSize", + "Table1.__evaluation__.errors.transientTableData", + ], + [], + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + + expect(parsedUpdates).toEqual([ + { + kind: "D", + path: ["Table1", "__evaluation__", "errors", "transientTableData"], + }, + { + kind: "D", + path: ["Table1", "pageSize"], + }, + ]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + const expectedState = produce(oldState, (draft: any) => { + delete draft.Table1.pageSize; + delete draft.Table1.__evaluation__.errors.transientTableData; + }); + + expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); + }); + }); + + it("should serialise bigInteger values", () => { + const someBigInt = BigInt(121221); + // should generate serialised bigInt updates in additionalUpdates + const someAdditionaTableUpdates = [ + { + kind: "N", + path: ["Table1", "someNewProp"], + rhs: { someOtherKey: BigInt(3323232) }, + }, + ]; + const newStateWithBigInt = produce(oldState, (draft: any) => { + draft.Table1.pageSize = someBigInt; + }); + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newStateWithBigInt, + ["Table1.pageSize"], + someAdditionaTableUpdates, + ); + + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + + //should generate serialised bigInt update + expect(parsedUpdates).toEqual([ + { + kind: "E", + path: ["Table1", "pageSize"], + rhs: "121221", + }, + { + kind: "N", + path: ["Table1", "someNewProp"], + rhs: { + someOtherKey: "3323232", + }, + }, + ]); + + const parseAndApplyUpdatesToOldState = produce(oldState, (draft) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + const expectedState = produce(oldState, (draft: any) => { + draft.Table1.pageSize = "121221"; + draft.Table1.someNewProp = { someOtherKey: "3323232" }; + }); + + expect(parseAndApplyUpdatesToOldState).toEqual(expectedState); + }); + describe("serialise momement updates directly", () => { + test("should generate a null update when it sees an invalid moment object", () => { + const newState = produce(oldState, (draft: any) => { + draft.Table1.pageSize = moment("invalid value"); + }); + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newState, + ["Table1.pageSize"], + ); + const serialisedExpectedOutput = JSON.stringify([ + { kind: "E", rhs: null, path: ["Table1", "pageSize"] }, + ]); + expect(serialisedUpdates).toEqual(serialisedExpectedOutput); + }); + test("should generate a regular update when it sees a valid moment object", () => { + const validMoment = moment(); + const newState = produce(oldState, (draft: any) => { + draft.Table1.pageSize = validMoment; + }); + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + newState, + ["Table1.pageSize"], + ); + const serialisedExpectedOutput = JSON.stringify([ + { kind: "E", rhs: validMoment, path: ["Table1", "pageSize"] }, + ]); + expect(serialisedUpdates).toEqual(serialisedExpectedOutput); + }); + }); + // we are testing a flow from worker thread diff updates to being applied to the main thread's state + describe("test main thread update flow", () => { + //this function takes in serialised updates from the webworker and applies it to the main thread's state + function generateMainThreadStateFromSerialisedUpdates( + serialisedUpdates: any, + prevState: any, + ) { + const parsedUpdates = + parseUpdatesAndDeleteUndefinedUpdates(serialisedUpdates); + return produce(prevState, (draft: any) => { + parsedUpdates.forEach((v: any) => { + applyChange(draft, undefined, v); + }); + }); + } + let workerStateWithCollection: any; + let mainThreadStateWithCollection: any; + const someDate = "2023-12-07T19:05:11.830Z"; + test("large moment collection updates should be serialised, we should always see ISO string and no moment object properties", () => { + const largeCollection = [] as any; + for (let i = 0; i < 110; i++) { + largeCollection.push({ i, c: moment(someDate) }); + } + //attaching a collection to some property in the workerState + workerStateWithCollection = produce(oldState, (draft: any) => { + draft.Table1.pageSize = largeCollection; + }); + //generate serialised diff updates + const { serialisedUpdates } = generateSerialisedUpdates( + oldState, + workerStateWithCollection, + ["Table1.pageSize"], + ); + // parsing the updates generated by worker and applying it back to the main threadState + mainThreadStateWithCollection = + generateMainThreadStateFromSerialisedUpdates( + serialisedUpdates, + oldState, + ); + + const expectedMainThreadState = produce(oldState, (draft: any) => { + draft.Table1.pageSize = JSON.parse(JSON.stringify(largeCollection)); + }); + //check first value has the correct date + expect(mainThreadStateWithCollection.Table1.pageSize[0].c).toEqual( + someDate, + ); + + expect(mainThreadStateWithCollection).toEqual(expectedMainThreadState); + }); + test("update in a single moment value in a collection should always be serialised ", () => { + const someNewDate = "2023-12-07T19:05:11.930Z"; + // updating a single value in the prev worker state + const updatedWorkerStateWithASingleValue = produce( + klona(workerStateWithCollection), + (draft: any) => { + draft.Table1.pageSize[0].c = moment(someNewDate); + }, + ) as any; + + //generate serialised diff updates + const { serialisedUpdates } = generateSerialisedUpdates( + workerStateWithCollection, + updatedWorkerStateWithASingleValue, + ["Table1.pageSize[0].c"], + ); + + // parsing the updates generated by worker and applying it back to the main threadState + const updatedMainThreadState = + generateMainThreadStateFromSerialisedUpdates( + serialisedUpdates, + mainThreadStateWithCollection, + ) as any; + // check if the main thread state has the updated value + expect(updatedMainThreadState.Table1.pageSize[0].c).toEqual( + someNewDate, + ); + + const expectedMainThreadState = produce( + mainThreadStateWithCollection, + (draft: any) => { + draft.Table1.pageSize[0].c = JSON.parse( + JSON.stringify(moment(someNewDate)), + ); + }, + ); + + expect(updatedMainThreadState).toEqual(expectedMainThreadState); + }); + test("update in a single moment value to an invalid value should always be serialised ", () => { + //some garbage value + const someNewDate = "fdfdfd"; + // updating a single value in the prev worker state + const updatedWorkerStateWithASingleValue = produce( + klona(workerStateWithCollection), + (draft: any) => { + draft.Table1.pageSize[0].c = moment(someNewDate) as any; + }, + ) as any; + + //generate serialised diff updates + const { serialisedUpdates } = generateSerialisedUpdates( + workerStateWithCollection, + updatedWorkerStateWithASingleValue, + ["Table1.pageSize[0].c"], + ); + + // parsing the updates generated by worker and applying it back to the main threadState + const updatedMainThreadState = + generateMainThreadStateFromSerialisedUpdates( + serialisedUpdates, + mainThreadStateWithCollection, + ) as any; + // check if the main thread state has the updated invalid value which should be null + expect(updatedMainThreadState.Table1.pageSize[0].c).toEqual(null); + + const expectedMainThreadState = produce( + mainThreadStateWithCollection, + (draft: any) => { + draft.Table1.pageSize[0].c = JSON.parse( + JSON.stringify(moment(someNewDate)), + ); + }, + ); + + expect(updatedMainThreadState).toEqual(expectedMainThreadState); + }); }); }); }); diff --git a/app/client/src/workers/Evaluation/evalTreeWithChanges.ts b/app/client/src/workers/Evaluation/evalTreeWithChanges.ts index 6b3a717a60..be7c34d20e 100644 --- a/app/client/src/workers/Evaluation/evalTreeWithChanges.ts +++ b/app/client/src/workers/Evaluation/evalTreeWithChanges.ts @@ -13,7 +13,11 @@ import { MessageType, sendMessage } from "utils/MessageUtil"; import { MAIN_THREAD_ACTION } from "@appsmith/workers/Evaluation/evalWorkerActions"; import type { UpdateDataTreeMessageData } from "sagas/EvalWorkerActionSagas"; import type { JSUpdate } from "utils/JSPaneUtils"; -import { generateOptimisedUpdatesAndSetPrevState } from "./helpers"; +import { + generateOptimisedUpdatesAndSetPrevState, + getNewDataTreeUpdates, + uniqueOrderUpdatePaths, +} from "./helpers"; export function evalTreeWithChanges( updatedValuePaths: string[][], @@ -49,8 +53,6 @@ export function evalTreeWithChanges( dataTree = makeEntityConfigsAsObjProperties(dataTreeEvaluator.evalTree, { evalProps: dataTreeEvaluator.evalProps, - identicalEvalPathsPatches: - dataTreeEvaluator.getEvalPathsIdenticalToState(), }); /** Make sure evalMetaUpdates is sanitized to prevent postMessage failure */ @@ -62,11 +64,26 @@ export function evalTreeWithChanges( unevalTree = dataTreeEvaluator.getOldUnevalTree(); configTree = dataTreeEvaluator.oldConfigTree; } + const allUnevalUpdates = unEvalUpdates.map( + (update) => update.payload.propertyPath, + ); + const completeEvalOrder = uniqueOrderUpdatePaths([ + ...allUnevalUpdates, + ...evalOrder, + ]); + + const setterAndLocalStorageUpdates = getNewDataTreeUpdates( + uniqueOrderUpdatePaths(updatedValuePaths.map((val) => val.join("."))), + dataTree, + ); const updates = generateOptimisedUpdatesAndSetPrevState( dataTree, dataTreeEvaluator, + completeEvalOrder, + setterAndLocalStorageUpdates, ); + const evalTreeResponse: EvalTreeResponseData = { updates, dependencies, diff --git a/app/client/src/workers/Evaluation/fns/resetWidget.ts b/app/client/src/workers/Evaluation/fns/resetWidget.ts index 81659a813c..0d35c47b95 100644 --- a/app/client/src/workers/Evaluation/fns/resetWidget.ts +++ b/app/client/src/workers/Evaluation/fns/resetWidget.ts @@ -87,8 +87,6 @@ function resetWidgetMetaProperty( const oldUnEvalTree = dataTreeEvaluator.getOldUnevalTree(); const configTree = dataTreeEvaluator.getConfigTree(); const evalProps = dataTreeEvaluator.getEvalProps(); - const evalPathsIdenticalToState = - dataTreeEvaluator.getEvalPathsIdenticalToState(); const evaluatedEntity = evalTree[widget.widgetName]; const evaluatedEntityConfig = configTree[ @@ -135,7 +133,6 @@ function resetWidgetMetaProperty( evalPropertyValue: finalValue, unEvalPropertyValue: expressionToEvaluate, evalProps, - evalPathsIdenticalToState, }); evalMetaUpdates.push({ diff --git a/app/client/src/workers/Evaluation/handlers/evalTree.ts b/app/client/src/workers/Evaluation/handlers/evalTree.ts index 0b84925001..53651e3e15 100644 --- a/app/client/src/workers/Evaluation/handlers/evalTree.ts +++ b/app/client/src/workers/Evaluation/handlers/evalTree.ts @@ -9,6 +9,7 @@ import DataTreeEvaluator from "workers/common/DataTreeEvaluator"; import type { EvalMetaUpdates } from "@appsmith/workers/common/DataTreeEvaluator/types"; import { makeEntityConfigsAsObjProperties } from "@appsmith/workers/Evaluation/dataTreeUtils"; import type { DataTreeDiff } from "@appsmith/workers/Evaluation/evaluationUtils"; +import { serialiseToBigInt } from "@appsmith/workers/Evaluation/evaluationUtils"; import { CrashingError, getSafeToRenderDataTree, @@ -22,7 +23,10 @@ import { clearAllIntervals } from "../fns/overrides/interval"; import JSObjectCollection from "workers/Evaluation/JSObject/Collection"; import { getJSVariableCreatedEvents } from "../JSObject/JSVariableEvents"; import { errorModifier } from "../errorModifier"; -import { generateOptimisedUpdatesAndSetPrevState } from "../helpers"; +import { + generateOptimisedUpdatesAndSetPrevState, + uniqueOrderUpdatePaths, +} from "../helpers"; import DataStore from "../dataStore"; import type { TransmissionErrorHandler } from "../fns/utils/Messenger"; import { MessageType, sendMessage } from "utils/MessageUtil"; @@ -76,6 +80,7 @@ export function evalTree(request: EvalWorkerSyncRequest) { canvasWidgets = widgets; canvasWidgetsMeta = widgetsMeta; metaWidgetsCache = metaWidgets; + let isNewTree = false; try { if (!dataTreeEvaluator) { @@ -107,10 +112,9 @@ export function evalTree(request: EvalWorkerSyncRequest) { dataTree = makeEntityConfigsAsObjProperties(dataTreeResponse.evalTree, { evalProps: dataTreeEvaluator.evalProps, - identicalEvalPathsPatches: - dataTreeEvaluator?.getEvalPathsIdenticalToState(), }); staleMetaIds = dataTreeResponse.staleMetaIds; + isNewTree = true; } else if (dataTreeEvaluator.hasCyclicalDependency || forceEvaluation) { if (dataTreeEvaluator && !isEmpty(allActionValidationConfig)) { //allActionValidationConfigs may not be set in dataTreeEvaluator. Therefore, set it explicitly via setter method @@ -150,8 +154,6 @@ export function evalTree(request: EvalWorkerSyncRequest) { dataTree = makeEntityConfigsAsObjProperties(dataTreeResponse.evalTree, { evalProps: dataTreeEvaluator.evalProps, - identicalEvalPathsPatches: - dataTreeEvaluator?.getEvalPathsIdenticalToState(), }); staleMetaIds = dataTreeResponse.staleMetaIds; } else { @@ -193,8 +195,6 @@ export function evalTree(request: EvalWorkerSyncRequest) { dataTree = makeEntityConfigsAsObjProperties(dataTreeEvaluator.evalTree, { evalProps: dataTreeEvaluator.evalProps, - identicalEvalPathsPatches: - dataTreeEvaluator?.getEvalPathsIdenticalToState(), }); evalMetaUpdates = JSON.parse( @@ -229,21 +229,42 @@ export function evalTree(request: EvalWorkerSyncRequest) { makeEntityConfigsAsObjProperties(unevalTree, { sanitizeDataTree: false, evalProps: dataTreeEvaluator?.evalProps, - identicalEvalPathsPatches: - dataTreeEvaluator?.getEvalPathsIdenticalToState(), }), widgetTypeConfigMap, configTree, ); unEvalUpdates = []; + isNewTree = true; } const jsVarsCreatedEvent = getJSVariableCreatedEvents(jsUpdates); - const updates = generateOptimisedUpdatesAndSetPrevState( - dataTree, - dataTreeEvaluator, - ); + let updates; + if (isNewTree) { + try { + //for new tree send the whole thing, don't diff at all + updates = serialiseToBigInt([{ kind: "newTree", rhs: dataTree }]); + dataTreeEvaluator?.setPrevState(dataTree); + } catch (e) { + updates = "[]"; + } + isNewTree = false; + } else { + const allUnevalUpdates = unEvalUpdates.map( + (update) => update.payload.propertyPath, + ); + + const completeEvalOrder = uniqueOrderUpdatePaths([ + ...allUnevalUpdates, + ...evalOrder, + ]); + + updates = generateOptimisedUpdatesAndSetPrevState( + dataTree, + dataTreeEvaluator, + completeEvalOrder, + ); + } const evalTreeResponse: EvalTreeResponseData = { updates, diff --git a/app/client/src/workers/Evaluation/helpers.ts b/app/client/src/workers/Evaluation/helpers.ts index d42c148d74..0276ed8f2b 100644 --- a/app/client/src/workers/Evaluation/helpers.ts +++ b/app/client/src/workers/Evaluation/helpers.ts @@ -1,22 +1,33 @@ import { serialiseToBigInt } from "@appsmith/workers/Evaluation/evaluationUtils"; +import type { WidgetEntity } from "@appsmith//entities/DataTree/types"; import type { Diff } from "deep-diff"; import { diff } from "deep-diff"; import type { DataTree } from "entities/DataTree/dataTreeTypes"; import equal from "fast-deep-equal"; -import { get, isNumber, isObject, set } from "lodash"; +import { get, isObject, set } from "lodash"; import { isMoment } from "moment"; import { EvalErrorTypes } from "utils/DynamicBindingUtils"; export const fn_keys: string = "__fn_keys__"; -export interface DiffReferenceState { - kind: "referenceState"; - path: any[]; - referencePath: string; +export const uniqueOrderUpdatePaths = (updatePaths: string[]) => + Array.from(new Set(updatePaths)).sort((a, b) => b.length - a.length); + +export const getNewDataTreeUpdates = (paths: string[], dataTree: object) => + paths.map((path) => { + const segmentedPath = path.split("."); + return { + kind: "N", + path: segmentedPath, + rhs: get(dataTree, segmentedPath), + }; + }); + +export interface DiffNewTreeState { + kind: "newTree"; + rhs: any; } -export type DiffWithReferenceState = - | Diff - | DiffReferenceState; +export type DiffWithNewTreeState = Diff | DiffNewTreeState; // Finds the first index which is a duplicate value // Returns -1 if there are no duplicates // Returns the index of the first duplicate entry it finds @@ -72,29 +83,6 @@ export const countOccurrences = ( }; const LARGE_COLLECTION_SIZE = 100; -// for object paths which have a "." in the object key like "a.['b.c']" -const REGEX_NESTED_OBJECT_PATH = /(.+)\.\[\'(.*)\'\]/; - -const generateWithKey = (basePath: any, key: any) => { - const segmentedPath = [...basePath, key]; - - if (isNumber(key)) { - return { - path: basePath.join(".") + ".[" + key + "]", - segmentedPath, - }; - } - if (key.includes(".")) { - return { - path: basePath.join(".") + ".['" + key + "']", - segmentedPath, - }; - } - return { - path: basePath.join(".") + "." + key, - segmentedPath, - }; -}; export const stringifyFnsInObject = ( userObject: Record, @@ -170,68 +158,45 @@ const isLargeCollection = (val: any) => { return size > LARGE_COLLECTION_SIZE; }; -const normaliseEvalPath = (identicalEvalPathsPatches: any) => - Object.keys(identicalEvalPathsPatches || {}).reduce( - (acc: any, evalPath: string) => { - //for object paths which have a "." in the object key like "a.['b.c']", we need to extract these - // paths and break them to appropriate patch paths - - const matches = evalPath.match(REGEX_NESTED_OBJECT_PATH); - if (!matches || !matches.length) { - //regular paths like "a.b.c" - acc[evalPath] = identicalEvalPathsPatches[evalPath]; - return acc; - } - - const [, firstSeg, nestedPathSeg] = matches; - // normalise non nested paths like "a.['b']" - if (!nestedPathSeg.includes(".")) { - const key = [firstSeg, nestedPathSeg].join("."); - acc[key] = identicalEvalPathsPatches[evalPath]; - return acc; - } - // object paths which have a "." like "a.['b.c']" - const key = [firstSeg, `['${nestedPathSeg}']`].join("."); - acc[key] = identicalEvalPathsPatches[evalPath]; - return acc; - }, - {}, - ); +const getReducedDataTree = ( + dataTree: DataTree, + constrainedDiffPaths: string[], +): DataTree => { + const withErrors = Object.keys(dataTree).reduce((acc: any, key: string) => { + const widgetValue = dataTree[key] as WidgetEntity; + acc[key] = { + __evaluation__: { + errors: widgetValue.__evaluation__?.errors, + }, + }; + return acc; + }, {}); + return constrainedDiffPaths.reduce((acc: DataTree, key: string) => { + set(acc, key, get(dataTree, key)); + return acc; + }, withErrors); +}; const generateDiffUpdates = ( - oldDataTree: any, - dataTree: any, - ignoreLargeKeys: any, -): DiffWithReferenceState[] => { - const attachDirectly: DiffWithReferenceState[] = []; - const ignoreLargeKeysHasBeenAttached = new Set(); - const attachLater: DiffWithReferenceState[] = []; + oldDataTree: DataTree, + dataTree: DataTree, + constrainedDiffPaths: string[], +): Diff[] => { + 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); const updates = - diff(oldDataTree, dataTree, (path, key) => { + diff(oldData, newData, (path, key) => { if (!path.length || key === "__evaluation__") return false; - const { path: setPath, segmentedPath } = generateWithKey(path, key); + const segmentedPath = [...path, key]; - // if ignore path is present...this segment of code generates the data compression patches - if (!!ignoreLargeKeys[setPath]) { - const originalStateVal = get(oldDataTree, segmentedPath); - const correspondingStatePath = ignoreLargeKeys[setPath]; - const statePathValue = get(dataTree, correspondingStatePath); - if (!equal(originalStateVal, statePathValue)) { - //reference state patches are a patch that does not have a patch value but it provides a path which contains the same value - //this is helpful in making the payload sent to the main thread small - attachLater.push({ - kind: "referenceState", - path: segmentedPath, - referencePath: correspondingStatePath, - }); - } - ignoreLargeKeysHasBeenAttached.add(setPath); - return true; - } - const rhs = get(dataTree, segmentedPath); + const rhs = get(dataTree, segmentedPath) as DataTree; - const lhs = get(oldDataTree, segmentedPath); + const lhs = get(oldDataTree, segmentedPath) as DataTree; //when a moment value changes we do not want the inner moment object updates, we just want the ISO result of it // which we get during the serialisation process we perform at latter steps @@ -250,7 +215,6 @@ const generateDiffUpdates = ( if (lhs !== undefined) { attachDirectly.push({ kind: "D", lhs, path: segmentedPath }); } - // if the lhs is also undefined ignore diff on this node return true; } @@ -280,20 +244,107 @@ const generateDiffUpdates = ( return [...updates, ...largeDataSetUpdates]; }; +const correctUndefinedUpdatesToDeletesOrNew = ( + updates: Diff[], +) => + updates.reduce( + (acc, update) => { + const { kind, lhs, path, rhs } = update as any; + if (kind === "E") { + if (lhs === undefined && rhs !== undefined) { + acc.push({ kind: "N", path, rhs }); + } + if (lhs !== undefined && rhs === undefined) { + acc.push({ path, lhs, kind: "D" }); + } + if (lhs !== undefined && rhs !== undefined) { + acc.push(update); + } + return acc; + } + acc.push(update); + return acc; + }, + [] as Diff[], + ); + +// whenever an element in a collection is set to undefined, we need to send the entire collection as an update +const generateRootWidgetUpdates = ( + updates: Diff[], + newDataTree: DataTree, + oldDataTree: DataTree, +): Diff[] => + updates + .filter( + (v) => + v.kind === "D" && + v.path && + typeof v.path[v.path.length - 1] === "number", + ) + .map( + ({ path }: any) => { + const pathCopy = [...path]; + pathCopy.pop(); + return { + kind: "E", + path: pathCopy, + lhs: get(oldDataTree, pathCopy) as DataTree, + rhs: get(newDataTree, pathCopy) as DataTree, + }; //push the parent path + }, + [] as Diff[], + ); + +// when a root collection is updated, we need to scrub out updates that are inside the root collection +const getScrubbedOutUpdatesWhenRootCollectionIsUpdated = ( + updates: Diff[], + rootCollectionUpdates: Diff[], +) => { + const rootCollectionPaths = rootCollectionUpdates + .filter((update) => update?.path?.length) + .map((update) => (update.path as string[]).join(".")); + return ( + updates + .map((update: any) => ({ update, condensedPath: update.path.join(".") })) + .filter( + ({ condensedPath }) => + !rootCollectionPaths.some((p) => condensedPath.startsWith(p)), + ) + // remove the condensedPath from the update + .map(({ update }) => update) + ); +}; + export const generateOptimisedUpdates = ( - oldDataTree: any, - dataTree: any, - identicalEvalPathsPatches?: Record, -): DiffWithReferenceState[] => { - const ignoreLargeKeys = normaliseEvalPath(identicalEvalPathsPatches); - const updates = generateDiffUpdates(oldDataTree, dataTree, ignoreLargeKeys); - return updates; + oldDataTree: DataTree, + dataTree: DataTree, + // these are the paths that the diff is limited to, this is a performance optimisation and through this we don't have to diff the entire data tree + constrainedDiffPaths: string[], +): Diff[] => { + const updates = generateDiffUpdates( + oldDataTree, + dataTree, + constrainedDiffPaths, + ); + const correctedUpdates = correctUndefinedUpdatesToDeletesOrNew(updates); + + const rootCollectionUpdates = generateRootWidgetUpdates( + correctedUpdates, + dataTree, + oldDataTree, + ); + const scrubedOutUpdates = getScrubbedOutUpdatesWhenRootCollectionIsUpdated( + correctedUpdates, + rootCollectionUpdates, + ); + return [...scrubedOutUpdates, ...rootCollectionUpdates]; }; export const generateSerialisedUpdates = ( - prevState: any, - currentState: any, - identicalEvalPathsPatches: any, + prevState: DataTree, + currentState: DataTree, + constrainedDiffPaths: string[], + mergeAdditionalUpdates?: any, ): { serialisedUpdates: string; error?: { type: string; message: string }; @@ -301,13 +352,14 @@ export const generateSerialisedUpdates = ( const updates = generateOptimisedUpdates( prevState, currentState, - identicalEvalPathsPatches, + constrainedDiffPaths, ); //remove lhs from diff to reduce the size of diff upload, //it is not necessary to send lhs and we can make the payload to transfer to the main thread smaller for quicker transfer // eslint-disable-next-line @typescript-eslint/no-unused-vars - const removedLhs = updates.map(({ lhs, ...rest }: any) => rest); + let removedLhs = updates.map(({ lhs, ...rest }: any) => rest); + removedLhs = [...removedLhs, ...(mergeAdditionalUpdates || [])]; try { // serialise bigInt values and convert the updates to a string over here to minismise the cost of transfer @@ -325,16 +377,16 @@ export const generateSerialisedUpdates = ( }; export const generateOptimisedUpdatesAndSetPrevState = ( - dataTree: any, + dataTree: DataTree, dataTreeEvaluator: any, + constrainedDiffPaths: string[], + mergeAdditionalUpdates?: any, ) => { - const identicalEvalPathsPatches = - dataTreeEvaluator?.getEvalPathsIdenticalToState(); - const { error, serialisedUpdates } = generateSerialisedUpdates( dataTreeEvaluator.getPrevState(), dataTree, - identicalEvalPathsPatches, + constrainedDiffPaths, + mergeAdditionalUpdates, ); if (error) { diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 4ac73a5ab6..85799638c1 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -114,7 +114,6 @@ import { import { getFixedTimeDifference, replaceThisDotParams } from "./utils"; import { isJSObjectFunction } from "workers/Evaluation/JSObject/utils"; import { - setToEvalPathsIdenticalToState, validateActionProperty, validateAndParseWidgetProperty, validateWidgetProperty, @@ -134,7 +133,6 @@ export interface EvalProps { [entityName: string]: DataTreeEvaluationProps; } -export type EvalPathsIdenticalToState = Record; export default class DataTreeEvaluator { dependencyMap: DependencyMap = new DependencyMap(); sortedDependencies: SortedDependencies = []; @@ -164,9 +162,6 @@ export default class DataTreeEvaluator { * Sanitized eval values and errors */ evalProps: EvalProps = {}; - //when attaching values to __evaluations__ segment of the state there are cases where this value is identical to the widget property - //in those cases do not it to the dataTree and update this map. The main thread can decompress these updates and we can minimise the data transfer - evalPathsIdenticalToState: EvalPathsIdenticalToState = {}; undefinedEvalValuesMap: Record = {}; prevState = {}; @@ -187,9 +182,6 @@ export default class DataTreeEvaluator { this.widgetConfigMap = widgetConfigMap; } - getEvalPathsIdenticalToState(): EvalPathsIdenticalToState { - return this.evalPathsIdenticalToState || {}; - } getEvalTree() { return this.evalTree; } @@ -343,12 +335,11 @@ export default class DataTreeEvaluator { dataTree: DataTree, option: { evalProps: EvalProps; - evalPathsIdenticalToState: EvalPathsIdenticalToState; }, configTree: ConfigTree, ) { const unParsedEvalTree = this.getUnParsedEvalTree(); - const { evalPathsIdenticalToState, evalProps } = option; + const { evalProps } = option; for (const [entityName, entity] of Object.entries(dataTree)) { if (!isWidget(entity)) continue; const entityConfig = configTree[entityName] as WidgetEntityConfig; @@ -363,33 +354,15 @@ export default class DataTreeEvaluator { get(entity, propertyPath), ); // Pass it through parse - const { isValid, messages, parsed, transformed } = - validateWidgetProperty(validationConfig, value, entity, propertyPath); + const { isValid, messages, parsed } = validateWidgetProperty( + validationConfig, + value, + entity, + propertyPath, + ); set(entity, propertyPath, parsed); - const evaluatedValue = isValid - ? parsed - : isUndefined(transformed) - ? value - : transformed; - - const isParsedValueTheSame = parsed === evaluatedValue; - - const evalPath = getEvalValuePath(fullPropertyPath, { - isPopulated: false, - fullPath: true, - }); - - setToEvalPathsIdenticalToState({ - evalPath, - evalPathsIdenticalToState, - evalProps, - isParsedValueTheSame, - fullPropertyPath, - value: evaluatedValue, - }); - resetValidationErrorsForEntityProperty({ evalProps, fullPropertyPath, @@ -442,7 +415,6 @@ export default class DataTreeEvaluator { evaluatedTree, { evalProps: this.evalProps, - evalPathsIdenticalToState: this.evalPathsIdenticalToState, }, this.oldConfigTree, ); @@ -1096,7 +1068,6 @@ export default class DataTreeEvaluator { evalPropertyValue, unEvalPropertyValue, evalProps: this.evalProps, - evalPathsIdenticalToState: this.evalPathsIdenticalToState, }); parsedValue = this.getParsedValueForWidgetProperty({ @@ -1155,16 +1126,6 @@ export default class DataTreeEvaluator { if (!propertyPath) continue; - const evalPath = getEvalValuePath(fullPropertyPath); - setToEvalPathsIdenticalToState({ - evalPath, - evalPathsIdenticalToState: this.evalPathsIdenticalToState, - evalProps: this.evalProps, - isParsedValueTheSame: true, - fullPropertyPath, - value: evalPropertyValue, - }); - /** * Perf optimization very specific to handling actions * Fields like Api1.data doesn't get evaluated since it is not in dynamicBindingPathList @@ -1206,35 +1167,11 @@ export default class DataTreeEvaluator { ? prevEvaluatedValue : evalPropertyValue; - const evalPath = getEvalValuePath(fullPropertyPath, { - isPopulated: true, - //what is the purpose of this argument - fullPath: true, - }); - /** Variables defined in a JS object are not reactive. * Their evaluated values need to be reset only when the variable is modified by the user. * When uneval value of a js variable hasn't changed, it means that the previously evaluated values are in both trees already */ - if (skipVariableValueAssignment) { - setToEvalPathsIdenticalToState({ - evalPath, - evalPathsIdenticalToState: this.evalPathsIdenticalToState, - evalProps: this.evalProps, - isParsedValueTheSame: true, - fullPropertyPath, - value: evalValue, - }); - } else { + if (!skipVariableValueAssignment) { const valueForSafeTree = klona(evalValue); - setToEvalPathsIdenticalToState({ - evalPath, - evalPathsIdenticalToState: this.evalPathsIdenticalToState, - evalProps: this.evalProps, - isParsedValueTheSame: true, - fullPropertyPath, - value: valueForSafeTree, - }); - set(contextTree, fullPropertyPath, evalValue); set(safeTree, fullPropertyPath, valueForSafeTree); JSObjectCollection.setVariableValue(evalValue, fullPropertyPath); diff --git a/app/client/src/workers/common/DataTreeEvaluator/validationUtils.ts b/app/client/src/workers/common/DataTreeEvaluator/validationUtils.ts index ad6117a90c..65a0d968cb 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/validationUtils.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/validationUtils.ts @@ -5,10 +5,8 @@ import type { WidgetEntityConfig, } from "@appsmith/entities/DataTree/types"; import type { ConfigTree } from "entities/DataTree/dataTreeTypes"; -import { isObject, isUndefined, set } from "lodash"; import type { EvaluationError } from "utils/DynamicBindingUtils"; import { - getEvalValuePath, isPathDynamicTrigger, PropertyEvaluationErrorType, } from "utils/DynamicBindingUtils"; @@ -18,47 +16,11 @@ import { resetValidationErrorsForEntityProperty, } from "@appsmith/workers/Evaluation/evaluationUtils"; import { validate } from "workers/Evaluation/validations"; -import type { EvalPathsIdenticalToState, EvalProps } from "."; +import type { EvalProps } from "."; import type { ValidationResponse } from "constants/WidgetValidation"; -const LARGE_COLLECTION_SIZE = 100; - -const getIsLargeCollection = (val: any) => { - if (!Array.isArray(val)) return false; - const rowSize = !isObject(val[0]) ? 1 : Object.keys(val[0]).length; - - const size = val.length * rowSize; - - return size > LARGE_COLLECTION_SIZE; -}; -export function setToEvalPathsIdenticalToState({ - evalPath, - evalPathsIdenticalToState, - evalProps, - fullPropertyPath, - isParsedValueTheSame, - value, -}: { - evalPath: string; - evalPathsIdenticalToState: EvalPathsIdenticalToState; - evalProps: EvalProps; - isParsedValueTheSame: boolean; - fullPropertyPath: string; - value: unknown; -}) { - const isLargeCollection = getIsLargeCollection(value); - - if (isParsedValueTheSame && isLargeCollection) { - evalPathsIdenticalToState[evalPath] = fullPropertyPath; - } else { - delete evalPathsIdenticalToState[evalPath]; - - set(evalProps, evalPath, value); - } -} export function validateAndParseWidgetProperty({ configTree, - evalPathsIdenticalToState, evalPropertyValue, evalProps, fullPropertyPath, @@ -71,7 +33,6 @@ export function validateAndParseWidgetProperty({ evalPropertyValue: unknown; unEvalPropertyValue: string; evalProps: EvalProps; - evalPathsIdenticalToState: EvalPathsIdenticalToState; }): unknown { const { propertyPath } = getEntityNameAndPropertyPath(fullPropertyPath); @@ -82,26 +43,20 @@ export function validateAndParseWidgetProperty({ const widgetConfig = configTree[widget.widgetName] as WidgetEntityConfig; const validation = widgetConfig.validationPaths[propertyPath]; - const { isValid, messages, parsed, transformed } = validateWidgetProperty( + const { isValid, messages, parsed } = validateWidgetProperty( validation, evalPropertyValue, widget, propertyPath, ); - let evaluatedValue; - // remove already present validation errors resetValidationErrorsForEntityProperty({ evalProps, fullPropertyPath, }); - if (isValid) { - evaluatedValue = parsed; - } else { - evaluatedValue = isUndefined(transformed) ? evalPropertyValue : transformed; - + if (!isValid) { const evalErrors: EvaluationError[] = messages?.map((message) => { return { @@ -120,21 +75,6 @@ export function validateAndParseWidgetProperty({ }); } - const evalPath = getEvalValuePath(fullPropertyPath, { - isPopulated: false, - fullPath: true, - }); - const isParsedValueTheSame = parsed === evaluatedValue; - - setToEvalPathsIdenticalToState({ - evalPath, - evalPathsIdenticalToState, - evalProps, - isParsedValueTheSame, - fullPropertyPath, - value: evaluatedValue, - }); - return parsed; } From 024d225ee391710b5856d252380ef49882e90223 Mon Sep 17 00:00:00 2001 From: sneha122 Date: Wed, 27 Mar 2024 15:12:15 +0530 Subject: [PATCH 2/2] fix: property pane collapse expand icon issue fixed (#32094) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description The PR fixes: Issue with icon being reversed in property pane. In property pane, for each section if it is expanded currently, it should have collapse icon beside it, if section is in collapsed state, it should have expand icon beside it. Earlier it was reverse, this PR corrects it Screenshot 2024-03-27 at 2 02 57 PM Screenshot 2024-03-27 at 2 00 48 PM Note: This PR fixes only of the UI issues mentioned in #32069 Fixes #32069 _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.IDE" ### :mag: Cypress test results > [!IMPORTANT] > Workflow run: > Commit: `37457a290f6258fbeda84376d62327d5fbe1d78b` > Cypress dashboard url: Click here! > All cypress tests have passed πŸŽ‰πŸŽ‰πŸŽ‰ ## Summary by CodeRabbit - **Refactor** - Simplified the logic for displaying the chevron icon in the property pane sections, enhancing user interface consistency. - Updated icons for open and closed states in the property sections for clearer visual cues. Co-authored-by: β€œsneha122” <β€œsneha@appsmith.com”> --- .../IDE/Canvas_Context_Property_Pane_1_spec.js | 10 ++++------ .../IDE/Canvas_Context_Property_Pane_2_spec.js | 10 ++++------ .../src/pages/Editor/PropertyPane/PropertySection.tsx | 4 ++-- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_1_spec.js b/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_1_spec.js index dcbbafadf8..9b4fca2e6b 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_1_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_1_spec.js @@ -196,9 +196,8 @@ function setPropertyPaneSectionState(propertySectionState) { )) { cy.get("body").then(($body) => { if ( - $body.find( - `${propertySectionClass(sectionName)} .t--chevron-icon.rotate-180`, - ).length > + $body.find(`${propertySectionClass(sectionName)} .t--chevron-icon`) + .length > 0 !== shouldSectionOpen ) { @@ -214,9 +213,8 @@ function verifyPropertyPaneSectionState(propertySectionState) { )) { cy.get("body").then(($body) => { const isSectionOpen = - $body.find( - `${propertySectionClass(sectionName)} .t--chevron-icon.rotate-180`, - ).length > 0; + $body.find(`${propertySectionClass(sectionName)} .t--chevron-icon`) + .length > 0; expect(isSectionOpen).to.equal(shouldSectionOpen); }); } diff --git a/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_2_spec.js b/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_2_spec.js index c6607c3658..de6f555307 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_2_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_2_spec.js @@ -180,9 +180,8 @@ function setPropertyPaneSectionState(propertySectionState) { )) { cy.get("body").then(($body) => { if ( - $body.find( - `${propertySectionClass(sectionName)} .t--chevron-icon.rotate-180`, - ).length > + $body.find(`${propertySectionClass(sectionName)} .t--chevron-icon`) + .length > 0 !== shouldSectionOpen ) { @@ -198,9 +197,8 @@ function verifyPropertyPaneSectionState(propertySectionState) { )) { cy.get("body").then(($body) => { const isSectionOpen = - $body.find( - `${propertySectionClass(sectionName)} .t--chevron-icon.rotate-180`, - ).length > 0; + $body.find(`${propertySectionClass(sectionName)} .t--chevron-icon`) + .length > 0; expect(isSectionOpen).to.equal(shouldSectionOpen); }); } diff --git a/app/client/src/pages/Editor/PropertyPane/PropertySection.tsx b/app/client/src/pages/Editor/PropertyPane/PropertySection.tsx index 1b79d926a6..c36611b876 100644 --- a/app/client/src/pages/Editor/PropertyPane/PropertySection.tsx +++ b/app/client/src/pages/Editor/PropertyPane/PropertySection.tsx @@ -194,8 +194,8 @@ export const PropertySection = memo((props: PropertySectionProps) => { )} {props.collapsible && ( )}