From ff3018ccda166c8b9814d029581447e145f9d1e2 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Tue, 10 Dec 2024 11:42:06 +0530 Subject: [PATCH 01/10] chore: optimise first evaluation using scope cache --- .../ce/entities/DataTree/dataTreeJSAction.ts | 1 + app/client/src/ce/entities/DataTree/types.ts | 1 + .../src/ce/workers/Evaluation/Actions.ts | 14 ++-- .../ce/workers/Evaluation/evaluationUtils.ts | 13 ++++ .../src/entities/Engine/AppViewerEngine.ts | 9 --- app/client/src/index.tsx | 2 + .../sagas/ActionExecution/PluginActionSaga.ts | 3 +- .../sagas/ActionExecution/geolocationSaga.ts | 2 +- app/client/src/sagas/EvalWorkerActionSagas.ts | 2 +- app/client/src/sagas/EvaluationsSaga.test.ts | 2 +- app/client/src/sagas/EvaluationsSaga.ts | 19 +---- app/client/src/sagas/JSLibrarySaga.ts | 2 +- app/client/src/utils/workerInstances.ts | 13 ++++ .../src/workers/Evaluation/JSObject/test.ts | 4 +- .../Evaluation/__tests__/Actions.test.ts | 7 +- .../Evaluation/__tests__/evaluate.test.ts | 4 +- app/client/src/workers/Evaluation/evaluate.ts | 48 ++++++++---- .../workers/Evaluation/handlers/jsLibrary.ts | 5 +- .../workers/common/DataTreeEvaluator/index.ts | 77 ++++++++++++++++++- .../workers/common/DataTreeEvaluator/test.ts | 2 + 20 files changed, 170 insertions(+), 60 deletions(-) create mode 100644 app/client/src/utils/workerInstances.ts diff --git a/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts b/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts index e7bc3ec36bdd..9e9d49c187ee 100644 --- a/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts +++ b/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts @@ -89,6 +89,7 @@ export const generateDataTreeJSAction = ( dynamicBindingPathList: dynamicBindingPathList, variables: listVariables, dependencyMap: dependencyMap, + actionNames: actions.map((action) => action.name), }, }; }; diff --git a/app/client/src/ce/entities/DataTree/types.ts b/app/client/src/ce/entities/DataTree/types.ts index 04a5877c4a1d..8ed6728a417c 100644 --- a/app/client/src/ce/entities/DataTree/types.ts +++ b/app/client/src/ce/entities/DataTree/types.ts @@ -96,6 +96,7 @@ export interface JSActionEntityConfig extends EntityConfig { moduleId?: string; moduleInstanceId?: string; isPublic?: boolean; + actionNames: string[]; } export interface JSActionEntity { diff --git a/app/client/src/ce/workers/Evaluation/Actions.ts b/app/client/src/ce/workers/Evaluation/Actions.ts index a27bb7bfe4af..a2a524a073a3 100644 --- a/app/client/src/ce/workers/Evaluation/Actions.ts +++ b/app/client/src/ce/workers/Evaluation/Actions.ts @@ -40,8 +40,7 @@ export enum ExecutionType { /** * This method returns new dataTree with entity function and platform function */ -export const addDataTreeToContext = (args: { - EVAL_CONTEXT: EvalContext; +export const getDataTreeContext = (args: { dataTree: Readonly; removeEntityFunctions?: boolean; isTriggerBased: boolean; @@ -50,10 +49,11 @@ export const addDataTreeToContext = (args: { const { configTree, dataTree, - EVAL_CONTEXT, isTriggerBased, removeEntityFunctions = false, } = args; + const EVAL_CONTEXT: EvalContext = {}; + const dataTreeEntries = Object.entries(dataTree); const entityFunctionCollection: Record> = {}; @@ -96,15 +96,19 @@ export const addDataTreeToContext = (args: { } if (removeEntityFunctions) - return removeEntityFunctionsFromEvalContext( + removeEntityFunctionsFromEvalContext( entityFunctionCollection, EVAL_CONTEXT, ); - if (!isTriggerBased) return; + if (!isTriggerBased) { + return EVAL_CONTEXT; + } // if eval is not trigger based i.e., sync eval then we skip adding entity function to evalContext addEntityFunctionsToEvalContext(EVAL_CONTEXT, entityFunctionCollection); + + return EVAL_CONTEXT; }; export const addEntityFunctionsToEvalContext = ( diff --git a/app/client/src/ce/workers/Evaluation/evaluationUtils.ts b/app/client/src/ce/workers/Evaluation/evaluationUtils.ts index 17835767c031..5b9c0458b63b 100644 --- a/app/client/src/ce/workers/Evaluation/evaluationUtils.ts +++ b/app/client/src/ce/workers/Evaluation/evaluationUtils.ts @@ -1104,6 +1104,19 @@ export const isNotEntity = (entity: DataTreeEntity) => { export const isEntityAction = (entity: DataTreeEntity) => { return isAction(entity); }; + +export const isPropertyAnEntityAction = ( + entity: DataTreeEntity, + propertyPath: string, + entityConfig: DataTreeEntityConfig, +) => { + if (!isJSAction(entity)) return false; + + const { actionNames } = entityConfig as JSActionEntityConfig; + + return actionNames.includes(propertyPath); +}; + export const convertMicroDiffToDeepDiff = ( microDiffDifferences: Difference[], ): Diff[] => diff --git a/app/client/src/entities/Engine/AppViewerEngine.ts b/app/client/src/entities/Engine/AppViewerEngine.ts index e4cca1b7db6e..f088f615812a 100644 --- a/app/client/src/entities/Engine/AppViewerEngine.ts +++ b/app/client/src/entities/Engine/AppViewerEngine.ts @@ -25,7 +25,6 @@ import { waitForSegmentInit, waitForFetchUserSuccess, } from "ee/sagas/userSagas"; -import { waitForFetchEnvironments } from "ee/sagas/EnvironmentSagas"; import { fetchJSCollectionsForView } from "actions/jsActionActions"; import { fetchAppThemesAction, @@ -154,14 +153,6 @@ export default class AppViewerEngine extends AppEngine { yield call(waitForSegmentInit, true); endSpan(waitForSegmentSpan); - const waitForEnvironmentsSpan = startNestedSpan( - "AppViewerEngine.waitForFetchEnvironments", - rootSpan, - ); - - yield call(waitForFetchEnvironments); - endSpan(waitForEnvironmentsSpan); - yield put(fetchAllPageEntityCompletion([executePageLoadActions()])); endSpan(loadAppEntitiesSpan); diff --git a/app/client/src/index.tsx b/app/client/src/index.tsx index 1b21d5f0f738..6688d1f2071a 100755 --- a/app/client/src/index.tsx +++ b/app/client/src/index.tsx @@ -1,5 +1,7 @@ // This file must be executed as early as possible to ensure the preloads are triggered ASAP import "./preload-route-chunks"; +// Initialise eval worker instance +import "utils/workerInstances"; import React from "react"; import "./wdyr"; diff --git a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts index 59c312ba893d..c378edd52f2e 100644 --- a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts +++ b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts @@ -102,7 +102,8 @@ import log from "loglevel"; import { EMPTY_RESPONSE } from "components/editorComponents/emptyResponse"; import type { AppState } from "ee/reducers"; import { DEFAULT_EXECUTE_ACTION_TIMEOUT_MS } from "ee/constants/ApiConstants"; -import { evaluateActionBindings, evalWorker } from "sagas/EvaluationsSaga"; +import { evaluateActionBindings } from "sagas/EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import { isBlobUrl, parseBlobUrl } from "utils/AppsmithUtils"; import { getType, Types } from "utils/TypeHelpers"; import { matchPath } from "react-router"; diff --git a/app/client/src/sagas/ActionExecution/geolocationSaga.ts b/app/client/src/sagas/ActionExecution/geolocationSaga.ts index 92873a25ebc8..c2391f1f80dd 100644 --- a/app/client/src/sagas/ActionExecution/geolocationSaga.ts +++ b/app/client/src/sagas/ActionExecution/geolocationSaga.ts @@ -5,7 +5,7 @@ import { showToastOnExecutionError } from "sagas/ActionExecution/errorUtils"; import { setUserCurrentGeoLocation } from "actions/browserRequestActions"; import type { Channel } from "redux-saga"; import { channel } from "redux-saga"; -import { evalWorker } from "sagas/EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import type { TGetGeoLocationDescription, TWatchGeoLocationDescription, diff --git a/app/client/src/sagas/EvalWorkerActionSagas.ts b/app/client/src/sagas/EvalWorkerActionSagas.ts index 4485b67a36bd..1d12806fe858 100644 --- a/app/client/src/sagas/EvalWorkerActionSagas.ts +++ b/app/client/src/sagas/EvalWorkerActionSagas.ts @@ -12,10 +12,10 @@ import type { TMessage } from "utils/MessageUtil"; import { MessageType } from "utils/MessageUtil"; import type { ResponsePayload } from "../sagas/EvaluationsSaga"; import { - evalWorker, executeTriggerRequestSaga, updateDataTreeHandler, } from "../sagas/EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import { handleStoreOperations } from "./ActionExecution/StoreActionSaga"; import type { EvalTreeResponseData } from "workers/Evaluation/types"; import isEmpty from "lodash/isEmpty"; diff --git a/app/client/src/sagas/EvaluationsSaga.test.ts b/app/client/src/sagas/EvaluationsSaga.test.ts index 2cad962369ad..9e97aab028d2 100644 --- a/app/client/src/sagas/EvaluationsSaga.test.ts +++ b/app/client/src/sagas/EvaluationsSaga.test.ts @@ -2,8 +2,8 @@ import { defaultAffectedJSObjects, evalQueueBuffer, evaluateTreeSaga, - evalWorker, } from "./EvaluationsSaga"; +import { evalWorker } from "utils/workerInstances"; import { expectSaga } from "redux-saga-test-plan"; import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; import { select } from "redux-saga/effects"; diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index c7b0115ae0eb..03ab5ab82952 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -25,7 +25,7 @@ import { import { getMetaWidgets, getWidgets, getWidgetsMeta } from "sagas/selectors"; import type { WidgetTypeConfigMap } from "WidgetProvider/factory"; import WidgetFactory from "WidgetProvider/factory"; -import { GracefulWorkerService } from "utils/WorkerUtil"; +import { evalWorker } from "utils/workerInstances"; import type { EvalError, EvaluationError } from "utils/DynamicBindingUtils"; import { PropertyEvaluationErrorType } from "utils/DynamicBindingUtils"; import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; @@ -117,21 +117,10 @@ import { getCurrentPageId, } from "selectors/editorSelectors"; import { getInstanceId } from "ee/selectors/tenantSelectors"; +import { waitForFetchEnvironments } from "ee/sagas/EnvironmentSagas"; const APPSMITH_CONFIGS = getAppsmithConfigs(); -export const evalWorker = new GracefulWorkerService( - new Worker( - new URL("../workers/Evaluation/evaluation.worker.ts", import.meta.url), - { - type: "module", - // Note: the `Worker` part of the name is slightly important – LinkRelPreload_spec.js - // relies on it to find workers in the list of all requests. - name: "evalWorker", - }, - ), -); - let widgetTypeConfigMap: WidgetTypeConfigMap; export function* updateDataTreeHandler( @@ -305,6 +294,8 @@ export function* evaluateTreeSaga( evalTreeRequestData, ); + yield call(waitForFetchEnvironments); + yield call( updateDataTreeHandler, { @@ -901,5 +892,3 @@ export default function* evaluationSagaListeners() { } } } - -export { evalWorker as EvalWorker }; diff --git a/app/client/src/sagas/JSLibrarySaga.ts b/app/client/src/sagas/JSLibrarySaga.ts index 5a880cb92564..52fc076cca54 100644 --- a/app/client/src/sagas/JSLibrarySaga.ts +++ b/app/client/src/sagas/JSLibrarySaga.ts @@ -21,7 +21,7 @@ import { getCurrentApplicationId } from "selectors/editorSelectors"; import CodemirrorTernService from "utils/autocomplete/CodemirrorTernService"; import { EVAL_WORKER_ACTIONS } from "ee/workers/Evaluation/evalWorkerActions"; import { validateResponse } from "./ErrorSagas"; -import { EvalWorker } from "./EvaluationsSaga"; +import { evalWorker as EvalWorker } from "utils/workerInstances"; import log from "loglevel"; import { APP_MODE } from "entities/App"; import { getAppMode } from "ee/selectors/applicationSelectors"; diff --git a/app/client/src/utils/workerInstances.ts b/app/client/src/utils/workerInstances.ts new file mode 100644 index 000000000000..e3edd6625017 --- /dev/null +++ b/app/client/src/utils/workerInstances.ts @@ -0,0 +1,13 @@ +import { GracefulWorkerService } from "./WorkerUtil"; + +export const evalWorker = new GracefulWorkerService( + new Worker( + new URL("../workers/Evaluation/evaluation.worker.ts", import.meta.url), + { + type: "module", + // Note: the `Worker` part of the name is slightly important – LinkRelPreload_spec.js + // relies on it to find workers in the list of all requests. + name: "evalWorker", + }, + ), +); diff --git a/app/client/src/workers/Evaluation/JSObject/test.ts b/app/client/src/workers/Evaluation/JSObject/test.ts index ccfb1c6c969c..30e7e7975ce7 100644 --- a/app/client/src/workers/Evaluation/JSObject/test.ts +++ b/app/client/src/workers/Evaluation/JSObject/test.ts @@ -195,9 +195,6 @@ describe("saveResolvedFunctionsAndJSUpdates", function () { { key: "myFun2", }, - { - key: "myFun2", - }, ], bindingPaths: { body: "SMART_SUBSTITUTE", @@ -216,6 +213,7 @@ describe("saveResolvedFunctionsAndJSUpdates", function () { pluginType: "JS", name: "JSObject1", actionId: "64013546b956c26882acc587", + actionNames: ["myFun1", "myFun2"], } as JSActionEntityConfig, }; const entityName = "JSObject1"; diff --git a/app/client/src/workers/Evaluation/__tests__/Actions.test.ts b/app/client/src/workers/Evaluation/__tests__/Actions.test.ts index 87dea7c35556..8300a7d5f43f 100644 --- a/app/client/src/workers/Evaluation/__tests__/Actions.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/Actions.test.ts @@ -6,7 +6,7 @@ import type { EvalContext } from "workers/Evaluation/evaluate"; import { createEvaluationContext } from "workers/Evaluation/evaluate"; import { MessageType } from "utils/MessageUtil"; import { - addDataTreeToContext, + getDataTreeContext, addPlatformFunctionsToEvalContext, } from "ee/workers/Evaluation/Actions"; import TriggerEmitter, { BatchKey } from "../fns/utils/TriggerEmitter"; @@ -548,12 +548,13 @@ describe("Test addDataTreeToContext method", () => { const evalContext: EvalContext = {}; beforeAll(() => { - addDataTreeToContext({ - EVAL_CONTEXT: evalContext, + const EVAL_CONTEXT = getDataTreeContext({ dataTree: dataTree as unknown as DataTree, configTree, isTriggerBased: true, }); + + Object.assign(evalContext, EVAL_CONTEXT); addPlatformFunctionsToEvalContext(evalContext); }); diff --git a/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts b/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts index 65da53a793fa..81b810add994 100644 --- a/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts @@ -143,8 +143,8 @@ describe("evaluateSync", () => { errors: [ { errorMessage: { - name: "ReferenceError", - message: "setImmediate is not defined", + name: "TypeError", + message: "setImmediate is not a function", }, errorType: "PARSE", kind: { diff --git a/app/client/src/workers/Evaluation/evaluate.ts b/app/client/src/workers/Evaluation/evaluate.ts index 96a851e89870..fec597a5721e 100644 --- a/app/client/src/workers/Evaluation/evaluate.ts +++ b/app/client/src/workers/Evaluation/evaluate.ts @@ -23,7 +23,7 @@ import { PrimitiveErrorModifier, TypeErrorModifier, } from "./errorModifier"; -import { addDataTreeToContext } from "ee/workers/Evaluation/Actions"; +import { getDataTreeContext } from "ee/workers/Evaluation/Actions"; import { set } from "lodash"; import { klona } from "klona"; import { getEntityNameAndPropertyPath } from "ee/workers/Evaluation/evaluationUtils"; @@ -103,7 +103,7 @@ const ignoreGlobalObjectKeys = new Set([ "location", ]); -function resetWorkerGlobalScope() { +export function resetWorkerGlobalScope() { const jsLibraryAccessorSet = JSLibraryAccessor.getSet(); for (const key of Object.keys(self)) { @@ -273,14 +273,15 @@ export const createEvaluationContext = (args: createEvaluationContextArgs) => { Object.assign(EVAL_CONTEXT, context.globalContext); } - addDataTreeToContext({ - EVAL_CONTEXT, + const dataTreeContext = getDataTreeContext({ dataTree, configTree, removeEntityFunctions: !!removeEntityFunctions, isTriggerBased, }); + Object.assign(EVAL_CONTEXT, dataTreeContext); + overrideEvalContext(EVAL_CONTEXT, context?.overrideContext); return EVAL_CONTEXT; @@ -373,7 +374,8 @@ export default function evaluateSync( // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any evalArguments?: Array, - configTree?: ConfigTree, + configTree: ConfigTree = {}, + scopeCache?: EvalContext, ): EvalResult { return (function () { const errors: EvaluationError[] = []; @@ -394,16 +396,34 @@ export default function evaluateSync( }; } - resetWorkerGlobalScope(); + self["$isDataField"] = true; + const EVAL_CONTEXT: EvalContext = {}; - setEvalContext({ - dataTree, - configTree, - isDataField: true, - isTriggerBased: isJSCollection, - context, - evalArguments, - }); + ///// Adding callback data + EVAL_CONTEXT.ARGUMENTS = evalArguments; + //// Adding contextual data not part of data tree + EVAL_CONTEXT.THIS_CONTEXT = context?.thisContext || {}; + + if (context?.globalContext) { + Object.assign(EVAL_CONTEXT, context.globalContext); + } + + if (scopeCache) { + Object.assign(EVAL_CONTEXT, scopeCache); + } else { + const dataTreeContext = getDataTreeContext({ + dataTree, + configTree, + removeEntityFunctions: false, + isTriggerBased: isJSCollection, + }); + + Object.assign(EVAL_CONTEXT, dataTreeContext); + } + + overrideEvalContext(EVAL_CONTEXT, context?.overrideContext); + + Object.assign(self, EVAL_CONTEXT); try { result = indirectEval(script); diff --git a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts index ce81c2d7bf5d..56821485f172 100644 --- a/app/client/src/workers/Evaluation/handlers/jsLibrary.ts +++ b/app/client/src/workers/Evaluation/handlers/jsLibrary.ts @@ -1,4 +1,3 @@ -import { createMessage, customJSLibraryMessages } from "ee/constants/messages"; import difference from "lodash/difference"; import type { Def } from "tern"; import { invalidEntityIdentifiers } from "workers/common/DependencyMap/utils"; @@ -34,7 +33,7 @@ enum LibraryInstallError { class ImportError extends Error { code = LibraryInstallError.ImportError; constructor(url: string) { - super(createMessage(customJSLibraryMessages.IMPORT_URL_ERROR, url)); + super(`The script at ${url} cannot be installed.`); this.name = "ImportError"; } } @@ -42,7 +41,7 @@ class ImportError extends Error { class TernDefinitionError extends Error { code = LibraryInstallError.TernDefinitionError; constructor(name: string) { - super(createMessage(customJSLibraryMessages.DEFS_FAILED_ERROR, name)); + super(`Failed to generate autocomplete definitions for ${name}.`); this.name = "TernDefinitionError"; } } diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 095d9bd25798..63eaaae760f2 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -38,6 +38,7 @@ import type { DataTreeDiff } from "ee/workers/Evaluation/evaluationUtils"; import { convertMicroDiffToDeepDiff, getAllPathsBasedOnDiffPaths, + isPropertyAnEntityAction, } from "ee/workers/Evaluation/evaluationUtils"; import { @@ -86,7 +87,11 @@ import { EXECUTION_PARAM_REFERENCE_REGEX, THIS_DOT_PARAMS_KEY, } from "constants/AppsmithActionConstants/ActionConstants"; -import type { EvalResult, EvaluateContext } from "workers/Evaluation/evaluate"; +import { + resetWorkerGlobalScope, + type EvalResult, + type EvaluateContext, +} from "workers/Evaluation/evaluate"; import evaluateSync, { evaluateAsync, setEvalContext, @@ -148,6 +153,7 @@ import { EComputationCacheName, type ICacheProps, } from "../AppComputationCache/types"; +import { getDataTreeContext } from "ee/workers/Evaluation/Actions"; type SortedDependencies = Array; export interface EvalProps { @@ -1059,6 +1065,8 @@ export default class DataTreeEvaluator { staleMetaIds: string[]; contextTree: DataTree; } { + resetWorkerGlobalScope(); + const safeTree = klonaJSON(unEvalTree); const dataStore = DataStore.getDataStore(); const dataStoreClone = klonaJSON(dataStore); @@ -1084,6 +1092,18 @@ export default class DataTreeEvaluator { const { isFirstTree, metaWidgets, unevalUpdates } = options; let staleMetaIds: string[] = []; + const triggerBasedDataTreeContext = getDataTreeContext({ + dataTree: contextTree, + configTree: oldConfigTree, + isTriggerBased: true, + }); + + const nonTriggerBasedDataTreeContext = getDataTreeContext({ + dataTree: contextTree, + configTree: oldConfigTree, + isTriggerBased: false, + }); + try { for (const fullPropertyPath of evaluationOrder) { const { entityName, propertyPath } = @@ -1093,6 +1113,10 @@ export default class DataTreeEvaluator { if (!isWidgetActionOrJsObject(entity)) continue; + if (isPropertyAnEntityAction(entity, propertyPath, entityConfig)) { + continue; + } + // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any let unEvalPropertyValue = get(contextTree as any, fullPropertyPath); @@ -1135,6 +1159,11 @@ export default class DataTreeEvaluator { unEvalPropertyValue = replaceThisDotParams(unEvalPropertyValue); } + const scopeCache = + !!entity && isAnyJSAction(entity) + ? triggerBasedDataTreeContext + : nonTriggerBasedDataTreeContext; + try { evalPropertyValue = this.getDynamicValue( unEvalPropertyValue, @@ -1144,6 +1173,7 @@ export default class DataTreeEvaluator { contextData, undefined, fullPropertyPath, + scopeCache, ); } catch (error) { this.errors.push({ @@ -1208,6 +1238,16 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, parsedValue); set(safeTree, fullPropertyPath, klona(parsedValue)); + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(parsedValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(parsedValue), + ); staleMetaIds = staleMetaIds.concat( getStaleMetaStateIds({ @@ -1254,6 +1294,16 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); break; } case ENTITY_TYPE.JSACTION: { @@ -1294,6 +1344,16 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalValue); set(safeTree, fullPropertyPath, valueForSafeTree); + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); JSObjectCollection.setVariableValue(evalValue, fullPropertyPath); JSObjectCollection.setPrevUnEvalState({ fullPath: fullPropertyPath, @@ -1306,6 +1366,16 @@ export default class DataTreeEvaluator { default: set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); } } } catch (error) { @@ -1417,6 +1487,7 @@ export default class DataTreeEvaluator { // eslint-disable-next-line @typescript-eslint/no-explicit-any callBackData?: Array, fullPropertyPath?: string, + scopeCache?: EvaluateContext, ) { // Get the {{binding}} bound values let entity: DataTreeEntity | undefined = undefined; @@ -1467,6 +1538,7 @@ export default class DataTreeEvaluator { !!entity && isAnyJSAction(entity), contextData, callBackData, + scopeCache, ); if (fullPropertyPath && evalErrors.length) { @@ -1560,6 +1632,7 @@ export default class DataTreeEvaluator { // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any callbackData?: Array, + scopeCache?: EvaluateContext, ): EvalResult { let evalResponse: EvalResult; @@ -1574,6 +1647,8 @@ export default class DataTreeEvaluator { isJSObject, contextData, callbackData, + {}, + scopeCache, ); } catch (error) { evalResponse = { diff --git a/app/client/src/workers/common/DataTreeEvaluator/test.ts b/app/client/src/workers/common/DataTreeEvaluator/test.ts index 8c77aeaea266..8225955c5b3f 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/test.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/test.ts @@ -768,6 +768,7 @@ describe("isDataField", () => { dependencyMap: { body: ["myFun2", "myFun1"], }, + actionNames: ["myFun1", "myFun2"], }, JSObject2: { actionId: "644242aeadc0936a9b0e71cc", @@ -821,6 +822,7 @@ describe("isDataField", () => { dependencyMap: { body: ["myFun2", "myFun1"], }, + actionNames: ["myFun1", "myFun2"], }, MainContainer: { defaultProps: {}, From 541a00fee352d4e2cf649ecbbe177aa2bb995f69 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Tue, 10 Dec 2024 11:52:45 +0530 Subject: [PATCH 02/10] add feature flag integration --- app/client/src/ce/entities/FeatureFlag.ts | 2 ++ .../workers/Evaluation/handlers/workerEnv.ts | 10 ++++++---- .../workers/common/DataTreeEvaluator/index.ts | 17 +++++++++++++---- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/app/client/src/ce/entities/FeatureFlag.ts b/app/client/src/ce/entities/FeatureFlag.ts index f07d6f65b627..c26f5ade6cdb 100644 --- a/app/client/src/ce/entities/FeatureFlag.ts +++ b/app/client/src/ce/entities/FeatureFlag.ts @@ -43,6 +43,7 @@ export const FEATURE_FLAG = { "release_table_custom_loading_state_enabled", release_custom_widget_ai_builder: "release_custom_widget_ai_builder", ab_request_new_integration_enabled: "ab_request_new_integration_enabled", + release_evaluation_scope_cache: "release_evaluation_scope_cache", } as const; export type FeatureFlag = keyof typeof FEATURE_FLAG; @@ -81,6 +82,7 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = { release_table_custom_loading_state_enabled: false, release_custom_widget_ai_builder: false, ab_request_new_integration_enabled: false, + release_evaluation_scope_cache: false, }; export const AB_TESTING_EVENT_KEYS = { diff --git a/app/client/src/workers/Evaluation/handlers/workerEnv.ts b/app/client/src/workers/Evaluation/handlers/workerEnv.ts index c9286a3b5d06..8c286dc55f6a 100644 --- a/app/client/src/workers/Evaluation/handlers/workerEnv.ts +++ b/app/client/src/workers/Evaluation/handlers/workerEnv.ts @@ -1,9 +1,7 @@ -import type { FeatureFlags } from "ee/entities/FeatureFlag"; +import type { FeatureFlags, FeatureFlag } from "ee/entities/FeatureFlag"; export class WorkerEnv { - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-explicit-any - static flags: any; + static flags: FeatureFlags; static cloudHosting: boolean; static setFeatureFlags(featureFlags: FeatureFlags) { @@ -21,4 +19,8 @@ export class WorkerEnv { static getCloudHosting() { return WorkerEnv.cloudHosting; } + + static isFFEnabled(flag: FeatureFlag) { + return WorkerEnv.flags[flag] === true; + } } diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 63eaaae760f2..2b9270f49927 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -154,6 +154,8 @@ import { type ICacheProps, } from "../AppComputationCache/types"; import { getDataTreeContext } from "ee/workers/Evaluation/Actions"; +import { WorkerEnv } from "workers/Evaluation/handlers/workerEnv"; +import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; type SortedDependencies = Array; export interface EvalProps { @@ -1159,10 +1161,17 @@ export default class DataTreeEvaluator { unEvalPropertyValue = replaceThisDotParams(unEvalPropertyValue); } - const scopeCache = - !!entity && isAnyJSAction(entity) - ? triggerBasedDataTreeContext - : nonTriggerBasedDataTreeContext; + let scopeCache; + + if ( + WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) + ) { + if (!!entity && isAnyJSAction(entity)) { + scopeCache = triggerBasedDataTreeContext; + } else { + scopeCache = nonTriggerBasedDataTreeContext; + } + } try { evalPropertyValue = this.getDynamicValue( From f7bdef48e8635ab59a7a2b035f03a48e01836837 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Tue, 10 Dec 2024 11:57:22 +0530 Subject: [PATCH 03/10] fix a return statement in getDataTree context --- app/client/src/ce/workers/Evaluation/Actions.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/client/src/ce/workers/Evaluation/Actions.ts b/app/client/src/ce/workers/Evaluation/Actions.ts index a2a524a073a3..c09e94c4356e 100644 --- a/app/client/src/ce/workers/Evaluation/Actions.ts +++ b/app/client/src/ce/workers/Evaluation/Actions.ts @@ -95,12 +95,15 @@ export const getDataTreeContext = (args: { ); } - if (removeEntityFunctions) + if (removeEntityFunctions) { removeEntityFunctionsFromEvalContext( entityFunctionCollection, EVAL_CONTEXT, ); + return EVAL_CONTEXT; + } + if (!isTriggerBased) { return EVAL_CONTEXT; } From 70955efd8e28a2200921036f6348e4b423b17780 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Tue, 10 Dec 2024 12:03:11 +0530 Subject: [PATCH 04/10] remove default export of evaluateSync fn --- .../src/workers/Evaluation/JSObject/index.ts | 2 +- .../Evaluation/__tests__/evaluate.test.ts | 39 ++++++++++--------- app/client/src/workers/Evaluation/evaluate.ts | 2 +- .../src/workers/Evaluation/fns/resetWidget.ts | 2 +- .../workers/common/DataTreeEvaluator/index.ts | 3 +- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/client/src/workers/Evaluation/JSObject/index.ts b/app/client/src/workers/Evaluation/JSObject/index.ts index 79dce10b39bb..315eacb6f10d 100644 --- a/app/client/src/workers/Evaluation/JSObject/index.ts +++ b/app/client/src/workers/Evaluation/JSObject/index.ts @@ -5,7 +5,7 @@ import { EvalErrorTypes, getEvalValuePath } from "utils/DynamicBindingUtils"; import type { JSUpdate, ParsedJSSubAction } from "utils/JSPaneUtils"; import { parseJSObject, isJSFunctionProperty } from "@shared/ast"; import type DataTreeEvaluator from "workers/common/DataTreeEvaluator"; -import evaluateSync from "workers/Evaluation/evaluate"; +import { evaluateSync } from "workers/Evaluation/evaluate"; import type { DataTreeDiff } from "ee/workers/Evaluation/evaluationUtils"; import { DataTreeDiffEvent, diff --git a/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts b/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts index 81b810add994..e905500327a8 100644 --- a/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/evaluate.test.ts @@ -1,4 +1,5 @@ -import evaluate, { +import { + evaluateSync, createEvaluationContext, evaluateAsync, } from "workers/Evaluation/evaluate"; @@ -54,29 +55,29 @@ describe("evaluateSync", () => { }); it("unescapes string before evaluation", () => { const js = '\\"Hello!\\"'; - const response = evaluate(js, {}, false); + const response = evaluateSync(js, {}, false); expect(response.result).toBe("Hello!"); }); it("evaluate string post unescape in v1", () => { const js = '[1, 2, 3].join("\\\\n")'; - const response = evaluate(js, {}, false); + const response = evaluateSync(js, {}, false); expect(response.result).toBe("1\n2\n3"); }); it("evaluate string without unescape in v2", () => { self.evaluationVersion = 2; const js = '[1, 2, 3].join("\\n")'; - const response = evaluate(js, {}, false); + const response = evaluateSync(js, {}, false); expect(response.result).toBe("1\n2\n3"); }); it("throws error for undefined js", () => { // @ts-expect-error: Types are not available - expect(() => evaluate(undefined, {})).toThrow(TypeError); + expect(() => evaluateSync(undefined, {})).toThrow(TypeError); }); it("Returns for syntax errors", () => { - const response1 = evaluate("wrongJS", {}, false); + const response1 = evaluateSync("wrongJS", {}, false); expect(response1).toStrictEqual({ result: undefined, @@ -100,7 +101,7 @@ describe("evaluateSync", () => { }, ], }); - const response2 = evaluate("{}.map()", {}, false); + const response2 = evaluateSync("{}.map()", {}, false); expect(response2).toStrictEqual({ result: undefined, @@ -130,13 +131,13 @@ describe("evaluateSync", () => { }); it("evaluates value from data tree", () => { const js = "Input1.text"; - const response = evaluate(js, dataTree, false); + const response = evaluateSync(js, dataTree, false); expect(response.result).toBe("value"); }); it("disallows unsafe function calls", () => { const js = "setImmediate(() => {}, 100)"; - const response = evaluate(js, dataTree, false); + const response = evaluateSync(js, dataTree, false); expect(response).toStrictEqual({ result: undefined, @@ -166,51 +167,51 @@ describe("evaluateSync", () => { }); it("has access to extra library functions", () => { const js = "_.add(1,2)"; - const response = evaluate(js, dataTree, false); + const response = evaluateSync(js, dataTree, false); expect(response.result).toBe(3); }); it("evaluates functions with callback data", () => { const js = "(arg1, arg2) => arg1.value + arg2"; const callbackData = [{ value: "test" }, "1"]; - const response = evaluate(js, dataTree, false, {}, callbackData); + const response = evaluateSync(js, dataTree, false, {}, callbackData); expect(response.result).toBe("test1"); }); it("handles EXPRESSIONS with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, false); + let response = evaluateSync(js, dataTree, false); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, false); + response = evaluateSync(js, dataTree, false); expect(response.errors.length).toBe(0); }); it("handles TRIGGERS with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, false, undefined, undefined); + let response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, false, undefined, undefined); + response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); }); it("handles ANONYMOUS_FUNCTION with new lines", () => { let js = "\n"; - let response = evaluate(js, dataTree, false, undefined, undefined); + let response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); js = "\n\n\n"; - response = evaluate(js, dataTree, false, undefined, undefined); + response = evaluateSync(js, dataTree, false, undefined, undefined); expect(response.errors.length).toBe(0); }); it("has access to this context", () => { const js = "this.contextVariable"; const thisContext = { contextVariable: "test" }; - const response = evaluate(js, dataTree, false, { thisContext }); + const response = evaluateSync(js, dataTree, false, { thisContext }); expect(response.result).toBe("test"); // there should not be any error when accessing "this" variables @@ -220,7 +221,7 @@ describe("evaluateSync", () => { it("has access to additional global context", () => { const js = "contextVariable"; const globalContext = { contextVariable: "test" }; - const response = evaluate(js, dataTree, false, { globalContext }); + const response = evaluateSync(js, dataTree, false, { globalContext }); expect(response.result).toBe("test"); expect(response.errors).toHaveLength(0); diff --git a/app/client/src/workers/Evaluation/evaluate.ts b/app/client/src/workers/Evaluation/evaluate.ts index fec597a5721e..21047d2e4076 100644 --- a/app/client/src/workers/Evaluation/evaluate.ts +++ b/app/client/src/workers/Evaluation/evaluate.ts @@ -366,7 +366,7 @@ export function setEvalContext({ Object.assign(self, evalContext); } -export default function evaluateSync( +export function evaluateSync( userScript: string, dataTree: DataTree, isJSCollection: boolean, diff --git a/app/client/src/workers/Evaluation/fns/resetWidget.ts b/app/client/src/workers/Evaluation/fns/resetWidget.ts index ffa8cfb3a0e9..cea91ca76a91 100644 --- a/app/client/src/workers/Evaluation/fns/resetWidget.ts +++ b/app/client/src/workers/Evaluation/fns/resetWidget.ts @@ -15,7 +15,7 @@ import type { import { isWidget } from "ee/workers/Evaluation/evaluationUtils"; import { klona } from "klona"; import { getDynamicBindings, isDynamicValue } from "utils/DynamicBindingUtils"; -import evaluateSync, { setEvalContext } from "../evaluate"; +import { evaluateSync, setEvalContext } from "../evaluate"; import type { DescendantWidgetMap } from "sagas/WidgetOperationUtils"; import type { MetaState } from "reducers/entityReducers/metaReducer"; import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer"; diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 2b9270f49927..b97b00934900 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -88,11 +88,10 @@ import { THIS_DOT_PARAMS_KEY, } from "constants/AppsmithActionConstants/ActionConstants"; import { + evaluateSync, resetWorkerGlobalScope, type EvalResult, type EvaluateContext, -} from "workers/Evaluation/evaluate"; -import evaluateSync, { evaluateAsync, setEvalContext, } from "workers/Evaluation/evaluate"; From a6b86deb578ed2a170094b89a512a6dd3f8a716d Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Tue, 10 Dec 2024 17:01:01 +0530 Subject: [PATCH 05/10] set cachedScope only if the ff is enabled --- .../workers/common/DataTreeEvaluator/index.ts | 104 +++++++++++------- 1 file changed, 64 insertions(+), 40 deletions(-) diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index b97b00934900..1a8bde0c99c0 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -1246,16 +1246,21 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, parsedValue); set(safeTree, fullPropertyPath, klona(parsedValue)); - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(parsedValue), - ); - set( - nonTriggerBasedDataTreeContext, - fullPropertyPath, - klona(parsedValue), - ); + + if ( + WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) + ) { + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(parsedValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(parsedValue), + ); + } staleMetaIds = staleMetaIds.concat( getStaleMetaStateIds({ @@ -1302,16 +1307,22 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); - set( - nonTriggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); + + if ( + WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) + ) { + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + } + break; } case ENTITY_TYPE.JSACTION: { @@ -1352,16 +1363,24 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalValue); set(safeTree, fullPropertyPath, valueForSafeTree); - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); - set( - nonTriggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); + + if ( + WorkerEnv.isFFEnabled( + FEATURE_FLAG.release_evaluation_scope_cache, + ) + ) { + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + } + JSObjectCollection.setVariableValue(evalValue, fullPropertyPath); JSObjectCollection.setPrevUnEvalState({ fullPath: fullPropertyPath, @@ -1374,16 +1393,21 @@ export default class DataTreeEvaluator { default: set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); - set( - nonTriggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); + + if ( + WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) + ) { + set( + triggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + set( + nonTriggerBasedDataTreeContext, + fullPropertyPath, + klona(evalPropertyValue), + ); + } } } } catch (error) { From 6203160fb9d967a7efdae47dcc848e879d0868ef Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Tue, 10 Dec 2024 18:20:26 +0530 Subject: [PATCH 06/10] Fix test cases --- .../DataTree/dataTreeJSAction.test.ts | 2 ++ .../Evaluation/__tests__/evaluation.test.ts | 1 - .../workers/Evaluation/handlers/workerEnv.ts | 8 ++----- .../workers/common/DataTreeEvaluator/index.ts | 23 ++++--------------- 4 files changed, 9 insertions(+), 25 deletions(-) diff --git a/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts b/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts index 44cd09558191..954457fe78a9 100644 --- a/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts +++ b/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts @@ -191,6 +191,7 @@ describe("generateDataTreeJSAction", () => { myVar1: "SMART_SUBSTITUTE", myVar2: "SMART_SUBSTITUTE", }, + actionNames: ["myFun2", "myFun1"], }; const resultData = generateDataTreeJSAction(jsCollection); @@ -389,6 +390,7 @@ describe("generateDataTreeJSAction", () => { myVar1: "SMART_SUBSTITUTE", myVar2: "SMART_SUBSTITUTE", }, + actionNames: ["myFun2", "myFun1"], }; const result = generateDataTreeJSAction(jsCollection); diff --git a/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts b/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts index ad1a2936e742..8cfd4cc02fb3 100644 --- a/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts +++ b/app/client/src/workers/Evaluation/__tests__/evaluation.test.ts @@ -18,7 +18,6 @@ import WidgetFactory from "WidgetProvider/factory"; import { generateDataTreeWidget } from "entities/DataTree/dataTreeWidget"; import { sortObjectWithArray } from "../../../utils/treeUtils"; import klona from "klona"; - import { APP_MODE } from "entities/App"; const klonaFullSpy = jest.fn(); diff --git a/app/client/src/workers/Evaluation/handlers/workerEnv.ts b/app/client/src/workers/Evaluation/handlers/workerEnv.ts index 8c286dc55f6a..0646334fab23 100644 --- a/app/client/src/workers/Evaluation/handlers/workerEnv.ts +++ b/app/client/src/workers/Evaluation/handlers/workerEnv.ts @@ -1,7 +1,7 @@ -import type { FeatureFlags, FeatureFlag } from "ee/entities/FeatureFlag"; +import type { FeatureFlags } from "ee/entities/FeatureFlag"; export class WorkerEnv { - static flags: FeatureFlags; + static flags: FeatureFlags = {} as FeatureFlags; static cloudHosting: boolean; static setFeatureFlags(featureFlags: FeatureFlags) { @@ -19,8 +19,4 @@ export class WorkerEnv { static getCloudHosting() { return WorkerEnv.cloudHosting; } - - static isFFEnabled(flag: FeatureFlag) { - return WorkerEnv.flags[flag] === true; - } } diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 1a8bde0c99c0..4c01ee92bccc 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -154,7 +154,6 @@ import { } from "../AppComputationCache/types"; import { getDataTreeContext } from "ee/workers/Evaluation/Actions"; import { WorkerEnv } from "workers/Evaluation/handlers/workerEnv"; -import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; type SortedDependencies = Array; export interface EvalProps { @@ -1162,9 +1161,7 @@ export default class DataTreeEvaluator { let scopeCache; - if ( - WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) - ) { + if (WorkerEnv.flags.release_evaluation_scope_cache) { if (!!entity && isAnyJSAction(entity)) { scopeCache = triggerBasedDataTreeContext; } else { @@ -1247,9 +1244,7 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, parsedValue); set(safeTree, fullPropertyPath, klona(parsedValue)); - if ( - WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) - ) { + if (WorkerEnv.flags.release_evaluation_scope_cache) { set( triggerBasedDataTreeContext, fullPropertyPath, @@ -1308,9 +1303,7 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); - if ( - WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) - ) { + if (WorkerEnv.flags.release_evaluation_scope_cache) { set( triggerBasedDataTreeContext, fullPropertyPath, @@ -1364,11 +1357,7 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalValue); set(safeTree, fullPropertyPath, valueForSafeTree); - if ( - WorkerEnv.isFFEnabled( - FEATURE_FLAG.release_evaluation_scope_cache, - ) - ) { + if (WorkerEnv.flags.release_evaluation_scope_cache) { set( triggerBasedDataTreeContext, fullPropertyPath, @@ -1394,9 +1383,7 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); - if ( - WorkerEnv.isFFEnabled(FEATURE_FLAG.release_evaluation_scope_cache) - ) { + if (WorkerEnv.flags.release_evaluation_scope_cache) { set( triggerBasedDataTreeContext, fullPropertyPath, From 5c933fbfd5b5fedc79f79d27d52d094ad6c231bf Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Wed, 11 Dec 2024 12:39:19 +0530 Subject: [PATCH 07/10] resolve pr comments --- app/client/src/sagas/EvaluationsSaga.ts | 3 --- app/client/src/sagas/PostEvaluationSagas.ts | 4 ++++ app/client/src/workers/Evaluation/evaluate.ts | 6 +++--- .../workers/common/DataTreeEvaluator/index.ts | 16 ++++++++-------- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index 03ab5ab82952..557f32970720 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -117,7 +117,6 @@ import { getCurrentPageId, } from "selectors/editorSelectors"; import { getInstanceId } from "ee/selectors/tenantSelectors"; -import { waitForFetchEnvironments } from "ee/sagas/EnvironmentSagas"; const APPSMITH_CONFIGS = getAppsmithConfigs(); @@ -294,8 +293,6 @@ export function* evaluateTreeSaga( evalTreeRequestData, ); - yield call(waitForFetchEnvironments); - yield call( updateDataTreeHandler, { diff --git a/app/client/src/sagas/PostEvaluationSagas.ts b/app/client/src/sagas/PostEvaluationSagas.ts index 1467bc7582f7..29c31c1fc8bd 100644 --- a/app/client/src/sagas/PostEvaluationSagas.ts +++ b/app/client/src/sagas/PostEvaluationSagas.ts @@ -41,6 +41,7 @@ import type { EvalTreeResponseData } from "workers/Evaluation/types"; import { endSpan, startRootSpan } from "UITelemetry/generateTraces"; import { getCollectionNameToDisplay } from "ee/utils/actionExecutionUtils"; import { showToastOnExecutionError } from "./ActionExecution/errorUtils"; +import { waitForFetchEnvironments } from "ee/sagas/EnvironmentSagas"; let successfulBindingsMap: SuccessfulBindingMap | undefined; @@ -190,6 +191,9 @@ export function* logSuccessfulBindings( } export function* postEvalActionDispatcher(actions: Array) { + // Wait for environments api fetch before dispatching actions + yield call(waitForFetchEnvironments); + for (const action of actions) { yield put(action); } diff --git a/app/client/src/workers/Evaluation/evaluate.ts b/app/client/src/workers/Evaluation/evaluate.ts index 21047d2e4076..b8ca7cfcdc0b 100644 --- a/app/client/src/workers/Evaluation/evaluate.ts +++ b/app/client/src/workers/Evaluation/evaluate.ts @@ -375,7 +375,7 @@ export function evaluateSync( // eslint-disable-next-line @typescript-eslint/no-explicit-any evalArguments?: Array, configTree: ConfigTree = {}, - scopeCache?: EvalContext, + evalContextCache?: EvalContext, ): EvalResult { return (function () { const errors: EvaluationError[] = []; @@ -408,8 +408,8 @@ export function evaluateSync( Object.assign(EVAL_CONTEXT, context.globalContext); } - if (scopeCache) { - Object.assign(EVAL_CONTEXT, scopeCache); + if (evalContextCache) { + Object.assign(EVAL_CONTEXT, evalContextCache); } else { const dataTreeContext = getDataTreeContext({ dataTree, diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 4c01ee92bccc..302ac3822e41 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -1159,13 +1159,13 @@ export default class DataTreeEvaluator { unEvalPropertyValue = replaceThisDotParams(unEvalPropertyValue); } - let scopeCache; + let evalContextCache; if (WorkerEnv.flags.release_evaluation_scope_cache) { if (!!entity && isAnyJSAction(entity)) { - scopeCache = triggerBasedDataTreeContext; + evalContextCache = triggerBasedDataTreeContext; } else { - scopeCache = nonTriggerBasedDataTreeContext; + evalContextCache = nonTriggerBasedDataTreeContext; } } @@ -1178,7 +1178,7 @@ export default class DataTreeEvaluator { contextData, undefined, fullPropertyPath, - scopeCache, + evalContextCache, ); } catch (error) { this.errors.push({ @@ -1506,7 +1506,7 @@ export default class DataTreeEvaluator { // eslint-disable-next-line @typescript-eslint/no-explicit-any callBackData?: Array, fullPropertyPath?: string, - scopeCache?: EvaluateContext, + evalContextCache?: EvaluateContext, ) { // Get the {{binding}} bound values let entity: DataTreeEntity | undefined = undefined; @@ -1557,7 +1557,7 @@ export default class DataTreeEvaluator { !!entity && isAnyJSAction(entity), contextData, callBackData, - scopeCache, + evalContextCache, ); if (fullPropertyPath && evalErrors.length) { @@ -1651,7 +1651,7 @@ export default class DataTreeEvaluator { // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any callbackData?: Array, - scopeCache?: EvaluateContext, + evalContextCache?: EvaluateContext, ): EvalResult { let evalResponse: EvalResult; @@ -1667,7 +1667,7 @@ export default class DataTreeEvaluator { contextData, callbackData, {}, - scopeCache, + evalContextCache, ); } catch (error) { evalResponse = { From 7ed4dc3206c611ba03c6ad3fae21043f71576f23 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Wed, 11 Dec 2024 13:45:14 +0530 Subject: [PATCH 08/10] use only one evalContextCache --- .../workers/common/DataTreeEvaluator/index.ts | 89 ++++++------------- 1 file changed, 29 insertions(+), 60 deletions(-) diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 302ac3822e41..11d38469f997 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -1092,17 +1092,15 @@ export default class DataTreeEvaluator { const { isFirstTree, metaWidgets, unevalUpdates } = options; let staleMetaIds: string[] = []; - const triggerBasedDataTreeContext = getDataTreeContext({ - dataTree: contextTree, - configTree: oldConfigTree, - isTriggerBased: true, - }); + let evalContextCache; - const nonTriggerBasedDataTreeContext = getDataTreeContext({ - dataTree: contextTree, - configTree: oldConfigTree, - isTriggerBased: false, - }); + if (WorkerEnv.flags.release_evaluation_scope_cache) { + evalContextCache = getDataTreeContext({ + dataTree: contextTree, + configTree: oldConfigTree, + isTriggerBased: false, + }); + } try { for (const fullPropertyPath of evaluationOrder) { @@ -1113,6 +1111,7 @@ export default class DataTreeEvaluator { if (!isWidgetActionOrJsObject(entity)) continue; + // Skip evaluations for actions in JSObjects if (isPropertyAnEntityAction(entity, propertyPath, entityConfig)) { continue; } @@ -1159,16 +1158,6 @@ export default class DataTreeEvaluator { unEvalPropertyValue = replaceThisDotParams(unEvalPropertyValue); } - let evalContextCache; - - if (WorkerEnv.flags.release_evaluation_scope_cache) { - if (!!entity && isAnyJSAction(entity)) { - evalContextCache = triggerBasedDataTreeContext; - } else { - evalContextCache = nonTriggerBasedDataTreeContext; - } - } - try { evalPropertyValue = this.getDynamicValue( unEvalPropertyValue, @@ -1244,17 +1233,11 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, parsedValue); set(safeTree, fullPropertyPath, klona(parsedValue)); - if (WorkerEnv.flags.release_evaluation_scope_cache) { - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(parsedValue), - ); - set( - nonTriggerBasedDataTreeContext, - fullPropertyPath, - klona(parsedValue), - ); + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { + set(evalContextCache, fullPropertyPath, klona(parsedValue)); } staleMetaIds = staleMetaIds.concat( @@ -1303,17 +1286,11 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); - if (WorkerEnv.flags.release_evaluation_scope_cache) { - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); - set( - nonTriggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { + set(evalContextCache, fullPropertyPath, klona(evalPropertyValue)); } break; @@ -1357,14 +1334,12 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalValue); set(safeTree, fullPropertyPath, valueForSafeTree); - if (WorkerEnv.flags.release_evaluation_scope_cache) { - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { set( - nonTriggerBasedDataTreeContext, + evalContextCache, fullPropertyPath, klona(evalPropertyValue), ); @@ -1383,17 +1358,11 @@ export default class DataTreeEvaluator { set(contextTree, fullPropertyPath, evalPropertyValue); set(safeTree, fullPropertyPath, klona(evalPropertyValue)); - if (WorkerEnv.flags.release_evaluation_scope_cache) { - set( - triggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); - set( - nonTriggerBasedDataTreeContext, - fullPropertyPath, - klona(evalPropertyValue), - ); + if ( + WorkerEnv.flags.release_evaluation_scope_cache && + evalContextCache + ) { + set(evalContextCache, fullPropertyPath, klona(evalPropertyValue)); } } } From 9d770d94cc62c2421c4fc740e3aa1a95b06f0bf8 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 12 Dec 2024 15:40:46 +0530 Subject: [PATCH 09/10] use klonaJSON instead of klona --- app/client/src/workers/common/DataTreeEvaluator/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/client/src/workers/common/DataTreeEvaluator/index.ts b/app/client/src/workers/common/DataTreeEvaluator/index.ts index 367a0cc79a29..56ac7c712e87 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/index.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/index.ts @@ -1237,7 +1237,7 @@ export default class DataTreeEvaluator { WorkerEnv.flags.release_evaluation_scope_cache && evalContextCache ) { - set(evalContextCache, fullPropertyPath, klona(parsedValue)); + set(evalContextCache, fullPropertyPath, klonaJSON(parsedValue)); } staleMetaIds = staleMetaIds.concat( From 04b1e859b02282ba9efa96aea25acc9f20098061 Mon Sep 17 00:00:00 2001 From: Diljit VJ Date: Thu, 12 Dec 2024 16:31:42 +0530 Subject: [PATCH 10/10] use set instead of array for actionNames --- app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts | 4 ++-- app/client/src/ce/entities/DataTree/dataTreeJSAction.ts | 4 ++-- app/client/src/ce/entities/DataTree/types.ts | 2 +- app/client/src/ce/workers/Evaluation/evaluationUtils.ts | 2 +- app/client/src/workers/Evaluation/JSObject/test.ts | 2 +- app/client/src/workers/common/DataTreeEvaluator/test.ts | 4 ++-- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts b/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts index 954457fe78a9..0978fa770b75 100644 --- a/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts +++ b/app/client/src/ce/entities/DataTree/dataTreeJSAction.test.ts @@ -191,7 +191,7 @@ describe("generateDataTreeJSAction", () => { myVar1: "SMART_SUBSTITUTE", myVar2: "SMART_SUBSTITUTE", }, - actionNames: ["myFun2", "myFun1"], + actionNames: new Set(["myFun2", "myFun1"]), }; const resultData = generateDataTreeJSAction(jsCollection); @@ -390,7 +390,7 @@ describe("generateDataTreeJSAction", () => { myVar1: "SMART_SUBSTITUTE", myVar2: "SMART_SUBSTITUTE", }, - actionNames: ["myFun2", "myFun1"], + actionNames: new Set(["myFun2", "myFun1"]), }; const result = generateDataTreeJSAction(jsCollection); diff --git a/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts b/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts index 9e9d49c187ee..1cf8fe1c92e5 100644 --- a/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts +++ b/app/client/src/ce/entities/DataTree/dataTreeJSAction.ts @@ -46,7 +46,7 @@ export const generateDataTreeJSAction = ( const dependencyMap: DependencyMap = {}; dependencyMap["body"] = []; - const actions = js.config.actions; + const actions = js.config.actions || []; // TODO: Fix this the next time the file is edited // eslint-disable-next-line @typescript-eslint/no-explicit-any const actionsData: Record = {}; @@ -89,7 +89,7 @@ export const generateDataTreeJSAction = ( dynamicBindingPathList: dynamicBindingPathList, variables: listVariables, dependencyMap: dependencyMap, - actionNames: actions.map((action) => action.name), + actionNames: new Set(actions.map((action) => action.name)), }, }; }; diff --git a/app/client/src/ce/entities/DataTree/types.ts b/app/client/src/ce/entities/DataTree/types.ts index 8ed6728a417c..a2fcbcbc9d2d 100644 --- a/app/client/src/ce/entities/DataTree/types.ts +++ b/app/client/src/ce/entities/DataTree/types.ts @@ -96,7 +96,7 @@ export interface JSActionEntityConfig extends EntityConfig { moduleId?: string; moduleInstanceId?: string; isPublic?: boolean; - actionNames: string[]; + actionNames: Set; } export interface JSActionEntity { diff --git a/app/client/src/ce/workers/Evaluation/evaluationUtils.ts b/app/client/src/ce/workers/Evaluation/evaluationUtils.ts index 5b9c0458b63b..9cb0a70e8694 100644 --- a/app/client/src/ce/workers/Evaluation/evaluationUtils.ts +++ b/app/client/src/ce/workers/Evaluation/evaluationUtils.ts @@ -1114,7 +1114,7 @@ export const isPropertyAnEntityAction = ( const { actionNames } = entityConfig as JSActionEntityConfig; - return actionNames.includes(propertyPath); + return actionNames.has(propertyPath); }; export const convertMicroDiffToDeepDiff = ( diff --git a/app/client/src/workers/Evaluation/JSObject/test.ts b/app/client/src/workers/Evaluation/JSObject/test.ts index 30e7e7975ce7..960e790a4b17 100644 --- a/app/client/src/workers/Evaluation/JSObject/test.ts +++ b/app/client/src/workers/Evaluation/JSObject/test.ts @@ -213,7 +213,7 @@ describe("saveResolvedFunctionsAndJSUpdates", function () { pluginType: "JS", name: "JSObject1", actionId: "64013546b956c26882acc587", - actionNames: ["myFun1", "myFun2"], + actionNames: new Set(["myFun1", "myFun2"]), } as JSActionEntityConfig, }; const entityName = "JSObject1"; diff --git a/app/client/src/workers/common/DataTreeEvaluator/test.ts b/app/client/src/workers/common/DataTreeEvaluator/test.ts index 8225955c5b3f..ee6a876ee267 100644 --- a/app/client/src/workers/common/DataTreeEvaluator/test.ts +++ b/app/client/src/workers/common/DataTreeEvaluator/test.ts @@ -768,7 +768,7 @@ describe("isDataField", () => { dependencyMap: { body: ["myFun2", "myFun1"], }, - actionNames: ["myFun1", "myFun2"], + actionNames: new Set(["myFun1", "myFun2"]), }, JSObject2: { actionId: "644242aeadc0936a9b0e71cc", @@ -822,7 +822,7 @@ describe("isDataField", () => { dependencyMap: { body: ["myFun2", "myFun1"], }, - actionNames: ["myFun1", "myFun2"], + actionNames: new Set(["myFun1", "myFun2"]), }, MainContainer: { defaultProps: {},