From bff76ca1bf05ad998bc05c3f9011102bd0d8a5fb Mon Sep 17 00:00:00 2001 From: albinAppsmith <87797149+albinAppsmith@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:46:04 +0530 Subject: [PATCH 1/5] fix: Branding hover color issue (#27964) ## Description Primary buttons when hovered, turns the background to white. This was happening due to some recent changes in the branding related APIs and API calls. #### PR fixes following issue(s) Fixes https://github.com/appsmithorg/appsmith/issues/27940 #### Media #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --------- Co-authored-by: Pawan Kumar (cherry picked from commit c218f9228fb18e0e866ae2447cefc0ef27d02ed3) --- app/client/src/ce/sagas/tenantSagas.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) 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({ From 83970b9770baabad8cbad832907350ec25b34160 Mon Sep 17 00:00:00 2001 From: Dipyaman Biswas Date: Wed, 11 Oct 2023 23:19:19 +0530 Subject: [PATCH 2/5] feat: make features call a blocking API call for page load (#27974) ## Description Make features call a blocking API call for page load #### PR fixes following issue(s) Fixes #27973 #### Media #### Type of change > Please delete options that are not relevant. - Chore (housekeeping or task changes that don't impact user perception) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed (cherry picked from commit be7f6674f755b10a780e64280d4da1526482a021) --- app/client/src/ce/AppRouter.tsx | 16 ++++++++++++---- .../src/reducers/uiReducers/usersReducer.ts | 12 ++++++++++++ app/client/src/selectors/usersSelectors.tsx | 8 ++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) 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/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; From 6abd5e2c0a8c57cfa4e8a001b3e8ba26472f6f47 Mon Sep 17 00:00:00 2001 From: Valera Melnikov Date: Fri, 13 Oct 2023 12:08:01 +0300 Subject: [PATCH 3/5] fix: auto label position (#28022) ## Description Fix auto label position Fixes #27975 #### Media https://github.com/appsmithorg/appsmith/assets/11555074/bf6e4c8c-97db-4eab-8986-b4cd2de5bd0a #### Type of change - Bug fix (non-breaking change which fixes an issue) ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed (cherry picked from commit b42caa030823f47625098305565b3c68d4725895) --- app/client/src/widgets/WidgetUtils.test.ts | 7 +++++++ app/client/src/widgets/WidgetUtils.ts | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) 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 ); } From ee37b5272ad1e46b1dc9d77033e2cf450b35bcae Mon Sep 17 00:00:00 2001 From: arunvjn <32433245+arunvjn@users.noreply.github.com> Date: Mon, 16 Oct 2023 13:32:27 +0530 Subject: [PATCH 4/5] fix: Race condition in JS object mutation (#28083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes race condition in JS Object mutation where evaluation overrides the variables state. Root Cause: Execution context is shared between every evaluation request and trigger execution request. A trigger execution request could be paused when an asynchronous task is awaited. In the mean time, worker might pick up the task to perform and evaluation. Evaluation would end up replacing the entities in the execution context, there by resetting the actions performed by the trigger execution before it was paused. What the fix does? We are now caching the JS object for reuse which means that every execution/evaluation request reuses the same JS Object as long the JS Object isn't modified by a user action. Fixes #27978 > > > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > - Bug fix (non-breaking change which fixes an issue) > > > - [x] Manual - [x] Jest - [x] Cypress > > > Add Testsmith test cases links that relate to this PR > > > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag - [x] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [x] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [x] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed (cherry picked from commit 1afd223e10a61db916c3c91ddd73daf10ada02d8) --- .../JSObject/JSObjectMutation_spec.ts | 48 ++++++++++++++ .../src/ce/actions/evaluationActions.ts | 2 - .../workers/Evaluation/JSObject/Collection.ts | 62 +++++++++++++++++++ .../Evaluation/JSObject/JSVariableFactory.ts | 52 ---------------- .../Evaluation/JSObject/JSVariableUpdates.ts | 13 +--- .../__test__/JSVariableFactory.test.ts | 23 +++++-- .../src/workers/Evaluation/JSObject/index.ts | 1 + .../Evaluation/fns/utils/TriggerEmitter.ts | 6 +- .../workers/Evaluation/getEntityForContext.ts | 4 +- 9 files changed, 136 insertions(+), 75 deletions(-) delete mode 100644 app/client/src/workers/Evaluation/JSObject/JSVariableFactory.ts 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 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); } } From ab3ebc94f53ef1b824f690b7d009f66cb01ff731 Mon Sep 17 00:00:00 2001 From: Aishwarya-U-R <91450662+Aishwarya-U-R@users.noreply.github.com> Date: Fri, 13 Oct 2023 20:35:05 +0530 Subject: [PATCH 5/5] test: Cypress | Skip MsSQL_Basic_Spec.ts spec in CI (#28074) - This PR skips MsSQL_Basic_Spe.ts from CI runs since its runing out of memory in multiple CI runs - Until this is added to TED or we find other ways - will be run locally during regression - Script fix (non-breaking change which fixes an issue) - [X] Added `Test Plan Approved` label after changes were reviewed (cherry picked from commit 2a302e234dfecb5751b3217878f6caf0f7a7c47a) --- app/client/cypress_ci_custom.config.ts | 2 ++ 1 file changed, 2 insertions(+) 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", ], }, });