diff --git a/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts b/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts index 3488d7a6e5..52b60ccbf0 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts +++ b/app/client/cypress/e2e/Regression/ClientSide/JSObject/JSObjectMutation_spec.ts @@ -90,4 +90,52 @@ describe("JSObject testing", () => { expect($label).contains("[]"); }); }); + + it("6. Bug 27978 Check assignment should not get overridden by evaluation", () => { + _.entityExplorer.DragNDropWidget(_.draggableWidgets.TEXT, 400, 400); + _.propPane.TypeTextIntoField( + "Text", + `{{JSObject1.data.length ? 'id-' + JSObject1.data[0].id : 'Not Set' }}`, + true, + false, + ); + _.entityExplorer.NavigateToSwitcher("Explorer"); + _.apiPage.CreateAndFillApi( + _.dataManager.dsValues[_.dataManager.defaultEnviorment].mockApiUrl, + ); + const JS_OBJECT_BODY = `export default { + data: [], + async getData() { + await Api1.run() + return Api1.data + }, + async myFun1() { + this.data = await this.getData(); + console.log(this.data); + }, + async myFun2() { + const data = await this.getData(); + data.push({ name: "test123" }) + this.data = data; + console.log(this.data); + }, + }`; + _.jsEditor.CreateJSObject(JS_OBJECT_BODY, { + paste: true, + completeReplace: true, + toRun: false, + shouldCreateNewJSObj: true, + }); + _.jsEditor.SelectFunctionDropdown("myFun1"); + _.jsEditor.RunJSObj(); + _.entityExplorer.SelectEntityByName("Text2", "Widgets"); + _.agHelper.AssertContains("id-1"); + cy.reload(); + _.agHelper.AssertContains("Not Set"); + _.entityExplorer.SelectEntityByName("JSObject1", "Queries/JS"); + _.jsEditor.SelectFunctionDropdown("myFun2"); + _.jsEditor.RunJSObj(); + _.entityExplorer.SelectEntityByName("Text2", "Widgets"); + _.agHelper.AssertContains("id-1"); + }); }); diff --git a/app/client/src/ce/actions/evaluationActions.ts b/app/client/src/ce/actions/evaluationActions.ts index 2504d7b1be..8bb299de54 100644 --- a/app/client/src/ce/actions/evaluationActions.ts +++ b/app/client/src/ce/actions/evaluationActions.ts @@ -66,8 +66,6 @@ export const EVALUATE_REDUX_ACTIONS = [ ReduxActionTypes.MOVE_ACTION_SUCCESS, ReduxActionTypes.RUN_ACTION_SUCCESS, ReduxActionErrorTypes.RUN_ACTION_ERROR, - ReduxActionTypes.EXECUTE_PLUGIN_ACTION_SUCCESS, - ReduxActionErrorTypes.EXECUTE_PLUGIN_ACTION_ERROR, ReduxActionTypes.CLEAR_ACTION_RESPONSE, // JS Actions ReduxActionTypes.CREATE_JS_ACTION_SUCCESS, diff --git a/app/client/src/workers/Evaluation/JSObject/Collection.ts b/app/client/src/workers/Evaluation/JSObject/Collection.ts index df62aedeb5..d2874a2f0b 100644 --- a/app/client/src/workers/Evaluation/JSObject/Collection.ts +++ b/app/client/src/workers/Evaluation/JSObject/Collection.ts @@ -1,6 +1,20 @@ import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils"; import { klona } from "klona/full"; import { get, set } from "lodash"; +import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; +import ExecutionMetaData from "../fns/utils/ExecutionMetaData"; +import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; + +export enum PatchType { + "SET" = "SET", + "GET" = "GET", +} + +export interface Patch { + path: string; + method: PatchType; + value?: unknown; +} export type VariableState = Record>; @@ -61,6 +75,7 @@ export default class JSObjectCollection { const newVarState = { ...this.variableState[entityName] }; newVarState[propertyPath] = variableValue; this.variableState[entityName] = newVarState; + JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName); } static getVariableState( @@ -77,6 +92,53 @@ export default class JSObjectCollection { delete jsObject[propertyPath]; } + /**Map */ + static cachedJSVariablesByEntityName: Record = {}; + + /**Computes Map with getters & setters to track JS mutations + * We cache and reuse this map. We recreate only when the JSObject's content changes or when any of the variables + * gets evaluated + */ + static getVariablesForEvaluationContext(entityName: string) { + if (JSObjectCollection.cachedJSVariablesByEntityName[entityName]) + return JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + const varState = JSObjectCollection.getVariableState(entityName); + const variables = Object.entries(varState); + const newJSObject = {} as JSActionEntity; + + for (const [varName, varValue] of variables) { + let variable = varValue; + Object.defineProperty(newJSObject, varName, { + enumerable: true, + configurable: true, + get() { + TriggerEmitter.emit(BatchKey.process_js_variable_updates, { + path: `${entityName}.${varName}`, + method: PatchType.GET, + }); + return variable; + }, + set(value) { + TriggerEmitter.emit(BatchKey.process_js_variable_updates, { + path: `${entityName}.${varName}`, + method: PatchType.SET, + value, + }); + variable = value; + }, + }); + } + ExecutionMetaData.setExecutionMetaData({ + enableJSVarUpdateTracking: true, + }); + JSObjectCollection.cachedJSVariablesByEntityName[entityName] = newJSObject; + return JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + } + + static clearCachedVariablesForEvaluationContext(entityName: string) { + delete JSObjectCollection.cachedJSVariablesByEntityName[entityName]; + } + static clear() { this.variableState = {}; this.unEvalState = {}; diff --git a/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts b/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts deleted file mode 100644 index 1922f98a63..0000000000 --- a/app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts +++ /dev/null @@ -1,52 +0,0 @@ -import { PatchType } from "./JSVariableUpdates"; -import ExecutionMetaData from "../fns/utils/ExecutionMetaData"; -import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; -import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; - -class JSFactory { - static create( - jsObjectName: string, - varState: Record = {}, - ): JSActionEntity { - const newJSObject: any = {}; - - const variables = Object.entries(varState); - - for (const [varName, varValue] of variables) { - let variable = varValue; - Object.defineProperty(newJSObject, varName, { - enumerable: true, - configurable: true, - get() { - TriggerEmitter.emit(BatchKey.process_js_variable_updates, { - path: `${jsObjectName}.${varName}`, - method: PatchType.GET, - }); - return variable; - }, - set(value) { - TriggerEmitter.emit(BatchKey.process_js_variable_updates, { - path: `${jsObjectName}.${varName}`, - method: PatchType.SET, - value, - }); - variable = value; - }, - }); - - ExecutionMetaData.setExecutionMetaData({ - enableJSVarUpdateTracking: false, - }); - - newJSObject[varName] = varValue; - - ExecutionMetaData.setExecutionMetaData({ - enableJSVarUpdateTracking: true, - }); - } - - return newJSObject; - } -} - -export default JSFactory; diff --git a/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts b/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts index d15265aeaf..3d2638abba 100644 --- a/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts +++ b/app/client/src/workers/Evaluation/JSObject/JSVariableUpdates.ts @@ -5,17 +5,8 @@ import { evalTreeWithChanges } from "../evalTreeWithChanges"; import { get } from "lodash"; import { isJSObjectVariable } from "./utils"; import isDeepEqualES6 from "fast-deep-equal/es6"; - -export enum PatchType { - "SET" = "SET", - "GET" = "GET", -} - -export interface Patch { - path: string; - method: PatchType; - value?: unknown; -} +import type { Patch } from "./Collection"; +import { PatchType } from "./Collection"; export type UpdatedPathsMap = Record; diff --git a/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts b/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts index f7e0355b70..80191e1c6d 100644 --- a/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts +++ b/app/client/src/workers/Evaluation/JSObject/__test__/JSVariableFactory.test.ts @@ -1,9 +1,9 @@ -import JSFactory from "../JSVariableFactory"; import ExecutionMetaData from "workers/Evaluation/fns/utils/ExecutionMetaData"; import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; import TriggerEmitter, { jsVariableUpdatesHandlerWrapper, } from "workers/Evaluation/fns/utils/TriggerEmitter"; +import JSObjectCollection from "../Collection"; const applyJSVariableUpdatesToEvalTreeMock = jest.fn(); jest.mock("../JSVariableUpdates.ts", () => ({ @@ -36,7 +36,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: true, @@ -96,7 +101,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: false, @@ -125,7 +135,12 @@ describe("JSVariableFactory", () => { weakSet: new WeakSet(), } as unknown as JSActionEntity; - const proxiedJSObject = JSFactory.create("JSObject1", jsObject); + Object.entries(jsObject).forEach(([k, v]) => + JSObjectCollection.setVariableValue(v, `JSObject1.${k}`), + ); + + const proxiedJSObject = + JSObjectCollection.getVariablesForEvaluationContext("JSObject1"); ExecutionMetaData.setExecutionMetaData({ enableJSVarUpdateTracking: true, diff --git a/app/client/src/workers/Evaluation/JSObject/index.ts b/app/client/src/workers/Evaluation/JSObject/index.ts index a1af115b2a..56ac11224f 100644 --- a/app/client/src/workers/Evaluation/JSObject/index.ts +++ b/app/client/src/workers/Evaluation/JSObject/index.ts @@ -92,6 +92,7 @@ export function saveResolvedFunctionsAndJSUpdates( try { JSObjectCollection.deleteResolvedFunction(entityName); JSObjectCollection.deleteUnEvalState(entityName); + JSObjectCollection.clearCachedVariablesForEvaluationContext(entityName); const parseStartTime = performance.now(); const { parsedObject, success } = parseJSObject(entity.body); diff --git a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts index 7d50754db0..95ba119c36 100644 --- a/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts +++ b/app/client/src/workers/Evaluation/fns/utils/TriggerEmitter.ts @@ -1,10 +1,7 @@ import { EventEmitter } from "events"; import { MAIN_THREAD_ACTION } from "@appsmith/workers/Evaluation/evalWorkerActions"; import { WorkerMessenger } from "workers/Evaluation/fns/utils/Messenger"; -import type { - Patch, - UpdatedPathsMap, -} from "workers/Evaluation/JSObject/JSVariableUpdates"; +import type { UpdatedPathsMap } from "workers/Evaluation/JSObject/JSVariableUpdates"; import { applyJSVariableUpdatesToEvalTree } from "workers/Evaluation/JSObject/JSVariableUpdates"; import ExecutionMetaData from "./ExecutionMetaData"; import { get } from "lodash"; @@ -18,6 +15,7 @@ import type { import type { UpdateActionProps } from "workers/Evaluation/handlers/updateActionData"; import { handleActionsDataUpdate } from "workers/Evaluation/handlers/updateActionData"; import { getEntityNameAndPropertyPath } from "@appsmith/workers/Evaluation/evaluationUtils"; +import type { Patch } from "workers/Evaluation/JSObject/Collection"; const _internalSetTimeout = self.setTimeout; const _internalClearTimeout = self.clearTimeout; diff --git a/app/client/src/workers/Evaluation/getEntityForContext.ts b/app/client/src/workers/Evaluation/getEntityForContext.ts index 35b5bc46db..8037878d76 100644 --- a/app/client/src/workers/Evaluation/getEntityForContext.ts +++ b/app/client/src/workers/Evaluation/getEntityForContext.ts @@ -2,7 +2,6 @@ import type { JSActionEntity } from "@appsmith/entities/DataTree/types"; import type { DataTreeEntity } from "entities/DataTree/dataTreeTypes"; import { ENTITY_TYPE_VALUE } from "entities/DataTree/dataTreeFactory"; import JSObjectCollection from "./JSObject/Collection"; -import JSFactory from "./JSObject/JSVariableFactory"; import { jsObjectFunctionFactory } from "./fns/utils/jsObjectFnFactory"; import { isObject } from "lodash"; @@ -53,7 +52,8 @@ export function getEntityForEvalContext( return Object.assign({}, jsObject, fns); } - jsObjectForEval = JSFactory.create(entityName, jsObjectForEval); + jsObjectForEval = + JSObjectCollection.getVariablesForEvaluationContext(entityName); return Object.assign(jsObjectForEval, fns); } }