From 99db0a1fae2c801ed06949f73d861c722e61b21b Mon Sep 17 00:00:00 2001 From: Vemparala Surya Vamsi <121419957+vsvamsi1@users.noreply.github.com> Date: Mon, 9 Jun 2025 11:21:27 +0530 Subject: [PATCH] fix: code editor changes (#40239) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description We've encountered a bug in the REST API plugin where rapidly changing the URL input causes the save status to get stuck in the loading state. This happens because the evaluation is debounced, and by the time it's ready to run, the inputs may have changed in a way that prevents the evaluation from being triggered. However, we still initiate a saga that tracks the terminal state and controls the loading status. Since the evaluation never actually occurs, the terminal state is never reached, causing the loading status to remain stuck. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.JS" ### :mag: Cypress test results > [!TIP] > ๐ŸŸข ๐ŸŸข ๐ŸŸข All cypress tests have passed! ๐ŸŽ‰ ๐ŸŽ‰ ๐ŸŽ‰ > Workflow run: > Commit: f993e21dd7a3bc0414d5c2cb35e96f3831b625bc > Cypress dashboard. > Tags: `@tag.JS` > Spec: >
Fri, 06 Jun 2025 16:05:30 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **Refactor** - Streamlined change handling in the code editor for more consistent updates using a standardized debounce timer. - **Bug Fixes** - Improved auto-save reliability by adding a delay before save status checks to prevent premature assertions. - **New Features** - Introduced a configurable save delay constant to unify save operation timing across the editor. --------- Co-authored-by: โ€œsneha122โ€ <โ€œsneha@appsmith.comโ€> --- .../cypress/support/Pages/AggregateHelper.ts | 8 +++++ .../CodeEditor/debounceConstants.ts | 1 + .../editorComponents/CodeEditor/index.tsx | 35 +++++-------------- 3 files changed, 17 insertions(+), 27 deletions(-) create mode 100644 app/client/src/components/editorComponents/CodeEditor/debounceConstants.ts diff --git a/app/client/cypress/support/Pages/AggregateHelper.ts b/app/client/cypress/support/Pages/AggregateHelper.ts index de301009b4..12aab06c45 100644 --- a/app/client/cypress/support/Pages/AggregateHelper.ts +++ b/app/client/cypress/support/Pages/AggregateHelper.ts @@ -8,6 +8,9 @@ import EditorNavigator from "./EditorNavigation"; import { EntityType } from "./EditorNavigation"; import ClickOptions = Cypress.ClickOptions; import { DEBOUNCE_WAIT_TIME_ON_INPUT_CHANGE } from "../../../src/constants/WidgetConstants"; +const { + SAVE_TRIGGER_DELAY_MS, +} = require("../../../src/components/editorComponents/CodeEditor/debounceConstants"); type ElementType = string | JQuery; @@ -296,6 +299,11 @@ export class AggregateHelper { } public AssertAutoSave() { + // After fix made in https://github.com/appsmithorg/appsmith/pull/40239 to make save state and nw call in sync + // We will need to wait for the debounced time before we make any assertions on save state, otherwise + // we might run into situation where absence of save is asserted but save actually hasn't happened + // Additional 100ms added to avoid flaky issues that might be caused by race condition. + this.Sleep(SAVE_TRIGGER_DELAY_MS + 100); let saveStatus = this.CheckForPageSaveError(); // wait for save query to trigger & n/w call to finish occuring if (!saveStatus) diff --git a/app/client/src/components/editorComponents/CodeEditor/debounceConstants.ts b/app/client/src/components/editorComponents/CodeEditor/debounceConstants.ts new file mode 100644 index 0000000000..609dd6e0a2 --- /dev/null +++ b/app/client/src/components/editorComponents/CodeEditor/debounceConstants.ts @@ -0,0 +1 @@ +export const SAVE_TRIGGER_DELAY_MS = 600; diff --git a/app/client/src/components/editorComponents/CodeEditor/index.tsx b/app/client/src/components/editorComponents/CodeEditor/index.tsx index c2b261eb12..c621d571a0 100644 --- a/app/client/src/components/editorComponents/CodeEditor/index.tsx +++ b/app/client/src/components/editorComponents/CodeEditor/index.tsx @@ -161,6 +161,7 @@ import { getEachEntityInformation } from "ee/utils/autocomplete/EntityDefinition import { getCurrentPageId } from "selectors/editorSelectors"; import { executeCommandAction } from "actions/pluginActionActions"; import { PEEK_OVERLAY_DELAY } from "./PeekOverlayPopup/constants"; +import { SAVE_TRIGGER_DELAY_MS } from "./debounceConstants"; type ReduxStateProps = ReturnType; type ReduxDispatchProps = ReturnType; @@ -269,8 +270,6 @@ interface State { isOpened: boolean; autoCompleteVisible: boolean; hinterOpen: boolean; - // Flag for determining whether the entity change has been started or not so that even if the initial and final value remains the same, the status should be changed to not loading - changeStarted: boolean; ctrlPressed: boolean; peekOverlayProps: | (PeekOverlayStateProps & { @@ -314,7 +313,6 @@ class CodeEditor extends Component { isOpened: false, autoCompleteVisible: false, hinterOpen: false, - changeStarted: false, ctrlPressed: false, peekOverlayProps: undefined, showAIWindow: false, @@ -1309,17 +1307,17 @@ class CodeEditor extends Component { instance?: CodeMirror.Editor, changeObj?: CodeMirror.EditorChangeLinkedList, ) => { - const value = this.editor?.getValue() || ""; + const value = this.editor.getValue() || ""; const inputValue = this.props.input.value || ""; if ( this.props.input.onChange && - ((value !== inputValue && this.state.isFocused) || - this.state.changeStarted) + value !== inputValue && + this.state.isFocused ) { - this.setState({ - changeStarted: false, - }); + /* This action updates the status of the savingEntity to true so that any + shortcut commands do not execute before updating the entity in the store */ + this.props.startingEntityUpdate(); this.props.input.onChange(value); } @@ -1364,29 +1362,12 @@ class CodeEditor extends Component { } }; - handleDebouncedChange = _.debounce(this.handleChange, 600); + handleDebouncedChange = _.debounce(this.handleChange, SAVE_TRIGGER_DELAY_MS); startChange = ( instance: CodeMirror.Editor, changeObj: CodeMirror.EditorChangeLinkedList, ) => { - /* This action updates the status of the savingEntity to true so that any - shortcut commands do not execute before updating the entity in the store */ - const value = this.editor.getValue() || ""; - const inputValue = this.props.input.value || ""; - - if ( - this.props.input.onChange && - value !== inputValue && - this.state.isFocused && - !this.state.changeStarted - ) { - this.setState({ - changeStarted: true, - }); - this.props.startingEntityUpdate(); - } - this.hidePeekOverlay(); this.handleDebouncedChange(instance, changeObj); };