From 00e3ebc9e2e0b8cc2d88b678c3a2a21602dac010 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Wed, 5 Feb 2025 17:07:32 +0100 Subject: [PATCH] fix(editor): Code node overwrites code when switching nodes after edits (#13078) --- cypress/e2e/6-code-node.cy.ts | 28 +++++++++++++++---- cypress/pages/ndv.ts | 6 ++++ .../src/components/ParameterInput.vue | 9 +++--- .../src/composables/useCodeEditor.ts | 10 ++----- .../editor-ui/src/composables/useDebounce.ts | 21 ++++++-------- 5 files changed, 43 insertions(+), 31 deletions(-) diff --git a/cypress/e2e/6-code-node.cy.ts b/cypress/e2e/6-code-node.cy.ts index 674d91af18..709b6c2c91 100644 --- a/cypress/e2e/6-code-node.cy.ts +++ b/cypress/e2e/6-code-node.cy.ts @@ -7,6 +7,9 @@ import { WorkflowPage as WorkflowPageClass } from '../pages/workflow'; const WorkflowPage = new WorkflowPageClass(); const ndv = new NDV(); +const getParameter = () => ndv.getters.parameterInput('jsCode').should('be.visible'); +const getEditor = () => getParameter().find('.cm-content').should('exist'); + describe('Code node', () => { describe('Code editor', () => { beforeEach(() => { @@ -40,10 +43,26 @@ describe('Code node', () => { successToast().contains('Node executed successfully'); }); - it('should show lint errors in `runOnceForAllItems` mode', () => { - const getParameter = () => ndv.getters.parameterInput('jsCode').should('be.visible'); - const getEditor = () => getParameter().find('.cm-content').should('exist'); + it('should allow switching between sibling code nodes', () => { + // Setup + getEditor().type('{selectall}').paste("console.log('code node 1')"); + ndv.actions.close(); + WorkflowPage.actions.addNodeToCanvas('Code', true, true); + getEditor().type('{selectall}').paste("console.log('code node 2')"); + ndv.actions.close(); + WorkflowPage.actions.openNode('Code'); + ndv.actions.clickFloatingNode('Code1'); + getEditor().should('have.text', "console.log('code node 2')"); + getEditor().type('{selectall}').type("console.log('code node 2 edited')"); + // wait for debounce + cy.wait(200); + + ndv.actions.clickFloatingNode('Code'); + getEditor().should('have.text', "console.log('code node 1')"); + }); + + it('should show lint errors in `runOnceForAllItems` mode', () => { getEditor() .type('{selectall}') .paste(`$input.itemMatching() @@ -66,9 +85,6 @@ return }); it('should show lint errors in `runOnceForEachItem` mode', () => { - const getParameter = () => ndv.getters.parameterInput('jsCode').should('be.visible'); - const getEditor = () => getParameter().find('.cm-content').should('exist'); - ndv.getters.parameterInput('mode').click(); ndv.actions.selectOptionInParameterDropdown('mode', 'Run Once for Each Item'); getEditor() diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index 91ece23122..54ae067e72 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -151,6 +151,9 @@ export class NDV extends BasePage { schemaViewNodeName: () => cy.getByTestId('run-data-schema-node-name'), expressionExpanders: () => cy.getByTestId('expander'), expressionModalOutput: () => cy.getByTestId('expression-modal-output'), + floatingNodes: () => cy.getByTestId('floating-node'), + floatingNodeByName: (name: string) => + cy.getByTestId('floating-node').filter(`[data-node-name="${name}"]`), }; actions = { @@ -339,6 +342,9 @@ export class NDV extends BasePage { dragMainPanelToRight: () => { cy.drag('[data-test-id=panel-drag-button]', [1000, 0], { moveTwice: true }); }, + clickFloatingNode: (name: string) => { + this.getters.floatingNodeByName(name).realHover().click({ force: true }); + }, }; } diff --git a/packages/editor-ui/src/components/ParameterInput.vue b/packages/editor-ui/src/components/ParameterInput.vue index 661580bb6b..809b1edfe4 100644 --- a/packages/editor-ui/src/components/ParameterInput.vue +++ b/packages/editor-ui/src/components/ParameterInput.vue @@ -118,7 +118,7 @@ const emit = defineEmits<{ const externalHooks = useExternalHooks(); const i18n = useI18n(); const nodeHelpers = useNodeHelpers(); -const { callDebounced } = useDebounce(); +const { debounce } = useDebounce(); const router = useRouter(); const workflowHelpers = useWorkflowHelpers({ router }); const telemetry = useTelemetry(); @@ -801,9 +801,9 @@ function onTextInputChange(value: string) { emit('textInput', parameterData); } -function valueChangedDebounced(value: NodeParameterValueType | {} | Date) { - void callDebounced(valueChanged, { debounceTime: 100 }, value); -} + +const valueChangedDebounced = debounce(valueChanged, { debounceTime: 100 }); + function onUpdateTextInput(value: string) { valueChanged(value); onTextInputChange(value); @@ -1032,6 +1032,7 @@ defineExpose({ }); onBeforeUnmount(() => { + valueChangedDebounced.cancel(); props.eventBus.off('optionSelected', optionSelected); }); diff --git a/packages/editor-ui/src/composables/useCodeEditor.ts b/packages/editor-ui/src/composables/useCodeEditor.ts index 05afc3b2d9..a470b72f77 100644 --- a/packages/editor-ui/src/composables/useCodeEditor.ts +++ b/packages/editor-ui/src/composables/useCodeEditor.ts @@ -154,10 +154,7 @@ export const useCodeEditor = ({ } } - const emitChanges = debounce((update: ViewUpdate) => { - onChange(update); - }, 300); - const lastChange = ref(); + const emitChanges = debounce(onChange, 300); function onEditorUpdate(update: ViewUpdate) { autocompleteStatus.value = completionStatus(update.view.state); @@ -168,7 +165,6 @@ export const useCodeEditor = ({ ); if (update.docChanged && !shouldIgnoreUpdate) { - lastChange.value = update; hasChanges.value = true; emitChanges(update); } @@ -375,9 +371,7 @@ export const useCodeEditor = ({ localStorage.removeItem(storedStateId.value); } - if (lastChange.value) { - onChange(lastChange.value); - } + emitChanges.flush(); editor.value.destroy(); } }); diff --git a/packages/editor-ui/src/composables/useDebounce.ts b/packages/editor-ui/src/composables/useDebounce.ts index 5ed9f221f0..d71b9b5c7d 100644 --- a/packages/editor-ui/src/composables/useDebounce.ts +++ b/packages/editor-ui/src/composables/useDebounce.ts @@ -1,24 +1,19 @@ import { ref } from 'vue'; -import { debounce as _debounce } from 'lodash-es'; +import { debounce as _debounce, type DebouncedFunc } from 'lodash-es'; export interface DebounceOptions { debounceTime: number; trailing?: boolean; } -export type DebouncedFunction = (...args: Args) => R; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type AnyFunction = (...args: any) => any; export function useDebounce() { // Create a ref for the WeakMap to store debounced functions. - const debounceCache = ref( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - new WeakMap, DebouncedFunction>(), - ); + const debounceCache = ref(new WeakMap>()); - const debounce = , ReturnType>>( - fn: T, - options: DebounceOptions, - ): T => { + const debounce = (fn: T, options: DebounceOptions): DebouncedFunc => { const { trailing, debounceTime } = options; // Check if a debounced version of the function is already stored in the WeakMap. @@ -37,14 +32,14 @@ export function useDebounce() { debounceCache.value.set(fn, debouncedFn); } - return debouncedFn as T; + return debouncedFn; }; - const callDebounced = , ReturnType>>( + const callDebounced = ( fn: T, options: DebounceOptions, ...inputParameters: Parameters - ): ReturnType => { + ): ReturnType | undefined => { const debouncedFn = debounce(fn, options); return debouncedFn(...inputParameters);