From 5acbfb423436b94e58af0e532e567bdc3783a622 Mon Sep 17 00:00:00 2001
From: Elias Meire <elias@meire.dev>
Date: Mon, 22 Apr 2024 14:57:41 +0200
Subject: [PATCH] fix(editor): Prevent duplicate values in preview for SQL
 editor (#9129)

---
 .../src/components/ExpressionEdit.vue         |  15 ++-
 .../ExpressionEditorModalOutput.vue           | 101 ---------------
 .../ExpressionOutput.vue                      | 117 ++++++++++++++++++
 .../InlineExpressionEditorOutput.vue          |  99 ++-------------
 .../InlineExpressionEditorOutput.test.ts      | 100 +++++++++++++++
 .../src/components/SqlEditor/SqlEditor.vue    |   2 +-
 6 files changed, 236 insertions(+), 198 deletions(-)
 delete mode 100644 packages/editor-ui/src/components/ExpressionEditorModal/ExpressionEditorModalOutput.vue
 create mode 100644 packages/editor-ui/src/components/InlineExpressionEditor/ExpressionOutput.vue
 create mode 100644 packages/editor-ui/src/components/InlineExpressionEditor/__tests__/InlineExpressionEditorOutput.test.ts

diff --git a/packages/editor-ui/src/components/ExpressionEdit.vue b/packages/editor-ui/src/components/ExpressionEdit.vue
index 7540d020c9..7e4deea9a2 100644
--- a/packages/editor-ui/src/components/ExpressionEdit.vue
+++ b/packages/editor-ui/src/components/ExpressionEdit.vue
@@ -65,9 +65,10 @@
 							{{ $locale.baseText('expressionEdit.resultOfItem1') }}
 						</div>
 						<div :class="{ 'ph-no-capture': redactValues }">
-							<ExpressionEditorModalOutput
+							<ExpressionOutput
 								ref="expressionResult"
 								:segments="segments"
+								:extensions="theme"
 								data-test-id="expression-modal-output"
 							/>
 						</div>
@@ -82,7 +83,6 @@
 import { defineComponent } from 'vue';
 import { mapStores } from 'pinia';
 import ExpressionEditorModalInput from '@/components/ExpressionEditorModal/ExpressionEditorModalInput.vue';
-import ExpressionEditorModalOutput from '@/components/ExpressionEditorModal/ExpressionEditorModalOutput.vue';
 import VariableSelector from '@/components/VariableSelector.vue';
 
 import type { IVariableItemSelected } from '@/Interface';
@@ -96,12 +96,14 @@ import { createExpressionTelemetryPayload } from '@/utils/telemetryUtils';
 import { useDebounce } from '@/composables/useDebounce';
 
 import type { Segment } from '@/types/expressions';
+import ExpressionOutput from './InlineExpressionEditor/ExpressionOutput.vue';
+import { outputTheme } from './ExpressionEditorModal/theme';
 
 export default defineComponent({
 	name: 'ExpressionEdit',
 	components: {
 		ExpressionEditorModalInput,
-		ExpressionEditorModalOutput,
+		ExpressionOutput,
 		VariableSelector,
 	},
 	props: {
@@ -149,6 +151,7 @@ export default defineComponent({
 			latestValue: '',
 			segments: [] as Segment[],
 			expressionsDocsUrl: EXPRESSIONS_DOCS_URL,
+			theme: outputTheme(),
 		};
 	},
 	computed: {
@@ -160,11 +163,7 @@ export default defineComponent({
 			this.latestValue = this.modelValue;
 
 			const resolvedExpressionValue =
-				(
-					this.$refs.expressionResult as {
-						getValue: () => string;
-					}
-				)?.getValue() || '';
+				(this.$refs.expressionResult as InstanceType<typeof ExpressionOutput>)?.getValue() || '';
 			void this.externalHooks.run('expressionEdit.dialogVisibleChanged', {
 				dialogVisible: newValue,
 				parameter: this.parameter,
diff --git a/packages/editor-ui/src/components/ExpressionEditorModal/ExpressionEditorModalOutput.vue b/packages/editor-ui/src/components/ExpressionEditorModal/ExpressionEditorModalOutput.vue
deleted file mode 100644
index 3e1d84524c..0000000000
--- a/packages/editor-ui/src/components/ExpressionEditorModal/ExpressionEditorModalOutput.vue
+++ /dev/null
@@ -1,101 +0,0 @@
-<template>
-	<div ref="root"></div>
-</template>
-
-<script lang="ts">
-import { EditorView } from '@codemirror/view';
-import { EditorState } from '@codemirror/state';
-import { defineComponent } from 'vue';
-import type { PropType } from 'vue';
-
-import { highlighter } from '@/plugins/codemirror/resolvableHighlighter';
-import { outputTheme } from './theme';
-
-import type { Plaintext, Resolved, Segment } from '@/types/expressions';
-import { forceParse } from '@/utils/forceParse';
-
-export default defineComponent({
-	name: 'ExpressionEditorModalOutput',
-	props: {
-		segments: {
-			type: Array as PropType<Segment[]>,
-			required: true,
-		},
-	},
-	data() {
-		return {
-			editor: null as EditorView | null,
-		};
-	},
-	computed: {
-		resolvedExpression(): string {
-			return this.segments.reduce((acc, segment) => {
-				acc += segment.kind === 'resolvable' ? segment.resolved : segment.plaintext;
-				return acc;
-			}, '');
-		},
-		plaintextSegments(): Plaintext[] {
-			return this.segments.filter((s): s is Plaintext => s.kind === 'plaintext');
-		},
-		resolvedSegments(): Resolved[] {
-			let cursor = 0;
-
-			return this.segments
-				.map((segment) => {
-					segment.from = cursor;
-
-					cursor +=
-						segment.kind === 'plaintext'
-							? segment.plaintext.length
-							: segment.resolved
-								? segment.resolved.toString().length
-								: 0;
-
-					segment.to = cursor;
-
-					return segment;
-				})
-				.filter((segment): segment is Resolved => segment.kind === 'resolvable');
-		},
-	},
-	watch: {
-		segments() {
-			if (!this.editor) return;
-
-			this.editor.dispatch({
-				changes: { from: 0, to: this.editor.state.doc.length, insert: this.resolvedExpression },
-			});
-
-			highlighter.addColor(this.editor, this.resolvedSegments);
-			highlighter.removeColor(this.editor, this.plaintextSegments);
-		},
-	},
-	mounted() {
-		const extensions = [
-			outputTheme(),
-			EditorState.readOnly.of(true),
-			EditorView.lineWrapping,
-			EditorView.editable.of(false),
-			EditorView.domEventHandlers({ scroll: forceParse }),
-		];
-
-		this.editor = new EditorView({
-			parent: this.$refs.root as HTMLDivElement,
-			state: EditorState.create({
-				doc: this.resolvedExpression,
-				extensions,
-			}),
-		});
-	},
-	beforeUnmount() {
-		this.editor?.destroy();
-	},
-	methods: {
-		getValue() {
-			return '=' + this.resolvedExpression;
-		},
-	},
-});
-</script>
-
-<style lang="scss"></style>
diff --git a/packages/editor-ui/src/components/InlineExpressionEditor/ExpressionOutput.vue b/packages/editor-ui/src/components/InlineExpressionEditor/ExpressionOutput.vue
new file mode 100644
index 0000000000..ab545be0ab
--- /dev/null
+++ b/packages/editor-ui/src/components/InlineExpressionEditor/ExpressionOutput.vue
@@ -0,0 +1,117 @@
+<script setup lang="ts">
+import { EditorState, type Extension } from '@codemirror/state';
+import { EditorView } from '@codemirror/view';
+
+import { useI18n } from '@/composables/useI18n';
+import { highlighter } from '@/plugins/codemirror/resolvableHighlighter';
+import type { Plaintext, Resolved, Segment } from '@/types/expressions';
+import { computed, onBeforeUnmount, onMounted, ref, watch } from 'vue';
+import { forceParse } from '@/utils/forceParse';
+
+interface ExpressionOutputProps {
+	segments: Segment[];
+	extensions?: Extension[];
+}
+
+const props = withDefaults(defineProps<ExpressionOutputProps>(), { extensions: () => [] });
+
+const i18n = useI18n();
+
+const editor = ref<EditorView | null>(null);
+const root = ref<HTMLElement | null>(null);
+
+const resolvedExpression = computed(() => {
+	if (props.segments.length === 0) {
+		return i18n.baseText('parameterInput.emptyString');
+	}
+
+	return props.segments.reduce(
+		(acc, segment) => {
+			// skip duplicate segments
+			if (acc.cursor >= segment.to) return acc;
+
+			acc.resolved += segment.kind === 'resolvable' ? String(segment.resolved) : segment.plaintext;
+			acc.cursor = segment.to;
+
+			return acc;
+		},
+		{ resolved: '', cursor: 0 },
+	).resolved;
+});
+
+const plaintextSegments = computed<Plaintext[]>(() => {
+	return props.segments.filter((s): s is Plaintext => s.kind === 'plaintext');
+});
+
+const resolvedSegments = computed<Resolved[]>(() => {
+	if (props.segments.length === 0) {
+		const emptyExpression = resolvedExpression.value;
+		const emptySegment: Resolved = {
+			from: 0,
+			to: emptyExpression.length,
+			kind: 'resolvable',
+			error: null,
+			resolvable: '',
+			resolved: emptyExpression,
+			state: 'pending',
+		};
+		return [emptySegment];
+	}
+
+	let cursor = 0;
+
+	return props.segments
+		.map((segment) => {
+			segment.from = cursor;
+			cursor +=
+				segment.kind === 'plaintext'
+					? segment.plaintext.length
+					: segment.resolved
+						? (segment.resolved as string | number | boolean).toString().length
+						: 0;
+			segment.to = cursor;
+			return segment;
+		})
+		.filter((segment): segment is Resolved => segment.kind === 'resolvable');
+});
+
+watch(
+	() => props.segments,
+	() => {
+		if (!editor.value) return;
+
+		editor.value.dispatch({
+			changes: { from: 0, to: editor.value.state.doc.length, insert: resolvedExpression.value },
+		});
+
+		highlighter.addColor(editor.value as EditorView, resolvedSegments.value);
+		highlighter.removeColor(editor.value as EditorView, plaintextSegments.value);
+	},
+);
+
+onMounted(() => {
+	editor.value = new EditorView({
+		parent: root.value as HTMLElement,
+		state: EditorState.create({
+			doc: resolvedExpression.value,
+			extensions: [
+				EditorState.readOnly.of(true),
+				EditorView.lineWrapping,
+				EditorView.editable.of(false),
+				EditorView.domEventHandlers({ scroll: forceParse }),
+				...props.extensions,
+			],
+		}),
+	});
+});
+
+onBeforeUnmount(() => {
+	editor.value?.destroy();
+});
+
+defineExpose({ getValue: () => '=' + resolvedExpression.value });
+</script>
+
+<template>
+	<div ref="root" data-test-id="expression-output"></div>
+</template>
diff --git a/packages/editor-ui/src/components/InlineExpressionEditor/InlineExpressionEditorOutput.vue b/packages/editor-ui/src/components/InlineExpressionEditor/InlineExpressionEditorOutput.vue
index ba45d6d1bf..3c160848d1 100644
--- a/packages/editor-ui/src/components/InlineExpressionEditor/InlineExpressionEditorOutput.vue
+++ b/packages/editor-ui/src/components/InlineExpressionEditor/InlineExpressionEditorOutput.vue
@@ -1,13 +1,11 @@
 <script setup lang="ts">
-import { EditorView } from '@codemirror/view';
-import { EditorState, type SelectionRange } from '@codemirror/state';
+import type { EditorState, SelectionRange } from '@codemirror/state';
 
 import { useI18n } from '@/composables/useI18n';
-import type { Plaintext, Resolved, Segment } from '@/types/expressions';
-import { highlighter } from '@/plugins/codemirror/resolvableHighlighter';
-import { computed, onMounted, onBeforeUnmount, ref, watch } from 'vue';
-import { outputTheme } from './theme';
+import type { Segment } from '@/types/expressions';
+import ExpressionOutput from './ExpressionOutput.vue';
 import InlineExpressionTip from './InlineExpressionTip.vue';
+import { outputTheme } from './theme';
 
 interface InlineExpressionEditorOutputProps {
 	segments: Segment[];
@@ -15,13 +13,11 @@ interface InlineExpressionEditorOutputProps {
 	unresolvedExpression?: string;
 	editorState?: EditorState;
 	selection?: SelectionRange;
-	isReadOnly?: boolean;
 	visible?: boolean;
 	noInputData?: boolean;
 }
 
-const props = withDefaults(defineProps<InlineExpressionEditorOutputProps>(), {
-	readOnly: false,
+withDefaults(defineProps<InlineExpressionEditorOutputProps>(), {
 	visible: false,
 	noInputData: false,
 	editorState: undefined,
@@ -30,84 +26,7 @@ const props = withDefaults(defineProps<InlineExpressionEditorOutputProps>(), {
 });
 
 const i18n = useI18n();
-
-const editor = ref<EditorView | null>(null);
-const root = ref<HTMLElement | null>(null);
-
-const resolvedExpression = computed(() => {
-	if (props.segments.length === 0) {
-		return i18n.baseText('parameterInput.emptyString');
-	}
-
-	return props.segments.reduce((acc, segment) => {
-		acc += segment.kind === 'resolvable' ? (segment.resolved as string) : segment.plaintext;
-		return acc;
-	}, '');
-});
-
-const plaintextSegments = computed<Plaintext[]>(() => {
-	return props.segments.filter((s): s is Plaintext => s.kind === 'plaintext');
-});
-
-const resolvedSegments = computed<Resolved[]>(() => {
-	if (props.segments.length === 0) {
-		const emptyExpression = resolvedExpression.value;
-		const emptySegment: Resolved = {
-			from: 0,
-			to: emptyExpression.length,
-			kind: 'resolvable',
-			error: null,
-			resolvable: '',
-			resolved: emptyExpression,
-			state: 'pending',
-		};
-		return [emptySegment];
-	}
-
-	let cursor = 0;
-
-	return props.segments
-		.map((segment) => {
-			segment.from = cursor;
-			cursor +=
-				segment.kind === 'plaintext'
-					? segment.plaintext.length
-					: segment.resolved
-						? (segment.resolved as string | number | boolean).toString().length
-						: 0;
-			segment.to = cursor;
-			return segment;
-		})
-		.filter((segment): segment is Resolved => segment.kind === 'resolvable');
-});
-
-watch(
-	() => props.segments,
-	() => {
-		if (!editor.value) return;
-
-		editor.value.dispatch({
-			changes: { from: 0, to: editor.value.state.doc.length, insert: resolvedExpression.value },
-		});
-
-		highlighter.addColor(editor.value as EditorView, resolvedSegments.value);
-		highlighter.removeColor(editor.value as EditorView, plaintextSegments.value);
-	},
-);
-
-onMounted(() => {
-	editor.value = new EditorView({
-		parent: root.value as HTMLElement,
-		state: EditorState.create({
-			doc: resolvedExpression.value,
-			extensions: [outputTheme(), EditorState.readOnly.of(true), EditorView.lineWrapping],
-		}),
-	});
-});
-
-onBeforeUnmount(() => {
-	editor.value?.destroy();
-});
+const theme = outputTheme();
 </script>
 
 <template>
@@ -116,7 +35,11 @@ onBeforeUnmount(() => {
 			{{ i18n.baseText('parameterInput.resultForItem') }} {{ hoveringItemNumber }}
 		</n8n-text>
 		<n8n-text :class="$style.body">
-			<div ref="root" data-test-id="inline-expression-editor-output"></div>
+			<ExpressionOutput
+				data-test-id="inline-expression-editor-output"
+				:segments="segments"
+				:extensions="theme"
+			></ExpressionOutput>
 		</n8n-text>
 		<div :class="$style.footer">
 			<InlineExpressionTip
diff --git a/packages/editor-ui/src/components/InlineExpressionEditor/__tests__/InlineExpressionEditorOutput.test.ts b/packages/editor-ui/src/components/InlineExpressionEditor/__tests__/InlineExpressionEditorOutput.test.ts
new file mode 100644
index 0000000000..b88a453d20
--- /dev/null
+++ b/packages/editor-ui/src/components/InlineExpressionEditor/__tests__/InlineExpressionEditorOutput.test.ts
@@ -0,0 +1,100 @@
+import { renderComponent } from '@/__tests__/render';
+import { createTestingPinia } from '@pinia/testing';
+import InlineExpressionEditorOutput from '../InlineExpressionEditorOutput.vue';
+
+describe('InlineExpressionEditorOutput.vue', () => {
+	test('should render duplicate segments correctly', async () => {
+		const { getByTestId } = renderComponent(InlineExpressionEditorOutput, {
+			pinia: createTestingPinia(),
+			props: {
+				hoveringItemNumber: 0,
+				segments: [
+					{
+						from: 0,
+						to: 5,
+						plaintext: '[1,2]',
+						kind: 'plaintext',
+					},
+					{
+						from: 0,
+						to: 1,
+						plaintext: '[',
+						kind: 'plaintext',
+					},
+					{
+						from: 1,
+						to: 2,
+						plaintext: '1',
+						kind: 'plaintext',
+					},
+					{
+						from: 2,
+						to: 3,
+						plaintext: ',',
+						kind: 'plaintext',
+					},
+					{
+						from: 3,
+						to: 4,
+						plaintext: '2',
+						kind: 'plaintext',
+					},
+					{
+						from: 4,
+						to: 5,
+						plaintext: ']',
+						kind: 'plaintext',
+					},
+				],
+			},
+		});
+		expect(getByTestId('inline-expression-editor-output')).toHaveTextContent('[1,2]');
+	});
+
+	test('should render segments with resolved expressions', () => {
+		const { getByTestId } = renderComponent(InlineExpressionEditorOutput, {
+			pinia: createTestingPinia(),
+			props: {
+				hoveringItemNumber: 0,
+				segments: [
+					{
+						kind: 'plaintext',
+						from: 0,
+						to: 6,
+						plaintext: 'before>',
+					},
+					{
+						kind: 'plaintext',
+						from: 6,
+						to: 7,
+						plaintext: ' ',
+					},
+					{
+						kind: 'resolvable',
+						from: 7,
+						to: 17,
+						resolvable: '{{ $now }}',
+						resolved: '[Object: "2024-04-18T09:03:26.651-04:00"]',
+						state: 'valid',
+						error: null,
+					},
+					{
+						kind: 'plaintext',
+						from: 17,
+						to: 18,
+						plaintext: ' ',
+					},
+					{
+						kind: 'plaintext',
+						from: 18,
+						to: 24,
+						plaintext: '<after',
+					},
+				],
+			},
+		});
+		expect(getByTestId('inline-expression-editor-output')).toHaveTextContent(
+			'before> [Object: "2024-04-18T09:03:26.651-04:00"] <after',
+		);
+	});
+});
diff --git a/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue b/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue
index c67688b9e2..daa2b33cd0 100644
--- a/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue
+++ b/packages/editor-ui/src/components/SqlEditor/SqlEditor.vue
@@ -140,7 +140,7 @@ const {
 	editorRef: sqlEditor,
 	editorValue,
 	extensions,
-	skipSegments: ['Statement', 'CompositeIdentifier', 'Parens'],
+	skipSegments: ['Statement', 'CompositeIdentifier', 'Parens', 'Brackets'],
 	isReadOnly: props.isReadOnly,
 });
 const ndvStore = useNDVStore();