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/cypress_ci_custom.config.ts b/app/client/cypress_ci_custom.config.ts index 025ee81137..9ffd3a80b2 100644 --- a/app/client/cypress_ci_custom.config.ts +++ b/app/client/cypress_ci_custom.config.ts @@ -58,6 +58,8 @@ export default defineConfig({ "cypress/e2e/EE/Enterprise/MultipleEnv/ME_airtable_spec.ts", "cypress/e2e/Regression/ServerSide/Datasources/ElasticSearch_Basic_Spec.ts", "cypress/e2e/Regression/ServerSide/Datasources/Oracle_Spec.ts", + "cypress/e2e/Regression/ClientSide/Widgets/Others/MapWidget_Spec.ts", + "cypress/e2e/Sanity/Datasources/MsSQL_Basic_Spec.ts", ], }, }); diff --git a/app/client/src/ce/AppRouter.tsx b/app/client/src/ce/AppRouter.tsx index cbfd14f1ef..1d95ef88c6 100644 --- a/app/client/src/ce/AppRouter.tsx +++ b/app/client/src/ce/AppRouter.tsx @@ -45,7 +45,10 @@ import * as Sentry from "@sentry/react"; import { getSafeCrash, getSafeCrashCode } from "selectors/errorSelectors"; import UserProfile from "pages/UserProfile"; import { getCurrentUser } from "actions/authActions"; -import { getCurrentUserLoading } from "selectors/usersSelectors"; +import { + getCurrentUserLoading, + getFeatureFlagsFetching, +} from "selectors/usersSelectors"; import Setup from "pages/setup"; import SettingsLoader from "pages/AdminSettings/loader"; import SignupSuccess from "pages/setup/SignupSuccess"; @@ -153,6 +156,7 @@ function AppRouter(props: { } = props; const tenantIsLoading = useSelector(isTenantLoading); const currentUserIsLoading = useSelector(getCurrentUserLoading); + const featuresIsLoading = useSelector(getFeatureFlagsFetching); useEffect(() => { getCurrentUser(); @@ -166,7 +170,11 @@ function AppRouter(props: { // hide the top loader once the tenant is loaded useEffect(() => { - if (tenantIsLoading === false && currentUserIsLoading === false) { + if ( + tenantIsLoading === false && + currentUserIsLoading === false && + featuresIsLoading === false + ) { const loader = document.getElementById("loader") as HTMLDivElement; if (loader) { @@ -177,9 +185,9 @@ function AppRouter(props: { }); } } - }, [tenantIsLoading, currentUserIsLoading]); + }, [tenantIsLoading, currentUserIsLoading, featuresIsLoading]); - if (tenantIsLoading || currentUserIsLoading) return null; + if (tenantIsLoading || currentUserIsLoading || featuresIsLoading) return null; return ( 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/ce/sagas/tenantSagas.tsx b/app/client/src/ce/sagas/tenantSagas.tsx index c197e3df13..4abb084117 100644 --- a/app/client/src/ce/sagas/tenantSagas.tsx +++ b/app/client/src/ce/sagas/tenantSagas.tsx @@ -22,12 +22,22 @@ export function* fetchCurrentTenantConfigSaga() { ); const isValidResponse: boolean = yield validateResponse(response); if (isValidResponse) { - const data: any = response.data; + const payload: any = response.data; yield put({ type: ReduxActionTypes.FETCH_CURRENT_TENANT_CONFIG_SUCCESS, - payload: data, + payload: { + ...payload, + tenantConfiguration: { + ...CE_defaultBrandingConfig, + ...payload.tenantConfiguration, + brandColors: { + ...CE_defaultBrandingConfig.brandColors, + ...payload.tenantConfiguration.brandColors, + }, + }, + }, }); - AnalyticsUtil.initInstanceId(data.instanceId); + AnalyticsUtil.initInstanceId(payload.instanceId); } } catch (error) { yield put({ diff --git a/app/client/src/reducers/uiReducers/usersReducer.ts b/app/client/src/reducers/uiReducers/usersReducer.ts index cfef428966..52c0b5f96c 100644 --- a/app/client/src/reducers/uiReducers/usersReducer.ts +++ b/app/client/src/reducers/uiReducers/usersReducer.ts @@ -24,6 +24,7 @@ const initialState: UsersReduxState = { featureFlag: { data: DEFAULT_FEATURE_FLAG_VALUE, isFetched: false, + isFetching: true, }, productAlert: { config: { @@ -173,6 +174,14 @@ const usersReducer = createReducer(initialState, { photoId: action.payload.photoId, }, }), + [ReduxActionTypes.FETCH_FEATURE_FLAGS_INIT]: (state: UsersReduxState) => ({ + ...state, + featureFlag: { + ...state.featureFlag, + isFetched: false, + isFetching: true, + }, + }), [ReduxActionTypes.FETCH_FEATURE_FLAGS_SUCCESS]: ( state: UsersReduxState, action: ReduxAction, @@ -181,6 +190,7 @@ const usersReducer = createReducer(initialState, { featureFlag: { data: action.payload, isFetched: true, + isFetching: false, }, }), [ReduxActionErrorTypes.FETCH_FEATURE_FLAGS_ERROR]: ( @@ -190,6 +200,7 @@ const usersReducer = createReducer(initialState, { featureFlag: { data: {}, isFetched: true, + isFetching: false, }, }), [ReduxActionTypes.FETCH_PRODUCT_ALERT_SUCCESS]: ( @@ -252,6 +263,7 @@ export interface UsersReduxState { featureFlag: { isFetched: boolean; data: FeatureFlags; + isFetching: boolean; }; productAlert: ProductAlertState; } diff --git a/app/client/src/selectors/usersSelectors.tsx b/app/client/src/selectors/usersSelectors.tsx index 53fcc78c02..e693ec0aa1 100644 --- a/app/client/src/selectors/usersSelectors.tsx +++ b/app/client/src/selectors/usersSelectors.tsx @@ -5,16 +5,24 @@ import { ANONYMOUS_USERNAME } from "constants/userConstants"; export const getCurrentUser = (state: AppState): User | undefined => state.ui?.users?.currentUser; + export const getCurrentUserLoading = (state: AppState): boolean => state.ui.users.loadingStates.fetchingUser; + export const getUserAuthError = (state: AppState): string => state.ui.users.error; + export const getUsers = (state: AppState): User[] => state.ui.users.users; + export const getProppanePreference = ( state: AppState, ): PropertyPanePositionConfig | undefined => state.ui.users.propPanePreferences; + export const getFeatureFlagsFetched = (state: AppState) => state.ui.users.featureFlag.isFetched; +export const getFeatureFlagsFetching = (state: AppState) => + state.ui.users.featureFlag.isFetching; + export const getIsUserLoggedIn = (state: AppState): boolean => state.ui.users.currentUser?.email !== ANONYMOUS_USERNAME; diff --git a/app/client/src/widgets/WidgetUtils.test.ts b/app/client/src/widgets/WidgetUtils.test.ts index d6a90203a8..aeec7138e3 100644 --- a/app/client/src/widgets/WidgetUtils.test.ts +++ b/app/client/src/widgets/WidgetUtils.test.ts @@ -26,6 +26,7 @@ import { isAutoHeightEnabledForWidgetWithLimits, getWidgetMaxAutoHeight, getWidgetMinAutoHeight, + isCompactMode, } from "./WidgetUtils"; import { getCustomTextColor, @@ -696,4 +697,10 @@ describe("Should Update Widget Height Automatically?", () => { const result = shouldUpdateWidgetHeightAutomatically(input, props); expect(result).toStrictEqual(expected); }); + it("should return correct value for isCompactMode", () => { + const compactHeight = 40; + const unCompactHeight = 41; + expect(isCompactMode(compactHeight)).toBeTruthy(); + expect(isCompactMode(unCompactHeight)).toBeFalsy(); + }); }); diff --git a/app/client/src/widgets/WidgetUtils.ts b/app/client/src/widgets/WidgetUtils.ts index 31761b438a..8f60f05e20 100644 --- a/app/client/src/widgets/WidgetUtils.ts +++ b/app/client/src/widgets/WidgetUtils.ts @@ -921,7 +921,7 @@ const findReactInstanceProps = (domElement: any) => { export function isCompactMode(componentHeight: number) { return ( - componentHeight < + componentHeight <= COMPACT_MODE_MIN_ROWS * GridDefaults.DEFAULT_GRID_ROW_HEIGHT ); } 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 a7b117d1f0..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 type 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 304b28009f..b4d5bceb4c 100644 --- a/app/client/src/workers/Evaluation/JSObject/index.ts +++ b/app/client/src/workers/Evaluation/JSObject/index.ts @@ -95,6 +95,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 06dd01c605..592d098b5a 100644 --- a/app/client/src/workers/Evaluation/getEntityForContext.ts +++ b/app/client/src/workers/Evaluation/getEntityForContext.ts @@ -4,7 +4,6 @@ import type { } from "@appsmith/entities/DataTree/types"; 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"; @@ -55,7 +54,8 @@ export function getEntityForEvalContext( return Object.assign({}, jsObject, fns); } - jsObjectForEval = JSFactory.create(entityName, jsObjectForEval); + jsObjectForEval = + JSObjectCollection.getVariablesForEvaluationContext(entityName); return Object.assign(jsObjectForEval, fns); } }