From 1732324965cce2030d0198f7b7e7091edfad6727 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= <ivov.src@gmail.com>
Date: Mon, 24 Oct 2022 12:48:16 +0200
Subject: [PATCH] fix(core): amend typing for `jsonParse()` options (#4423)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* :blue_book: Amend typing for `jsonParse()` options

* :pencil2: Update rule message and description

* :twisted_rightwards_arrows: Cherrypick Adi's work

* :bug: Account for falsy fallback values

* :recycle: Use `else if`

* :zap: Add explicit error message as type

* :zap: Consolidate utils tests

* :recycle: Use optional chaining

* :fire: Remove patchy type error

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
---
 packages/@n8n_io/eslint-config/local-rules.js |  6 ++++--
 packages/workflow/src/utils.ts                | 19 ++++++++++---------
 packages/workflow/{src => test}/utils.test.ts | 19 ++++++++++++++++++-
 3 files changed, 32 insertions(+), 12 deletions(-)
 rename packages/workflow/{src => test}/utils.test.ts (56%)

diff --git a/packages/@n8n_io/eslint-config/local-rules.js b/packages/@n8n_io/eslint-config/local-rules.js
index e735290d44..75f307d1e3 100644
--- a/packages/@n8n_io/eslint-config/local-rules.js
+++ b/packages/@n8n_io/eslint-config/local-rules.js
@@ -30,12 +30,14 @@ module.exports = {
 		meta: {
 			type: 'problem',
 			docs: {
-				description: 'Calls to JSON.parse() must be surrounded with a try/catch block.',
+				description:
+					'Calls to `JSON.parse()` must be replaced with `jsonParse()` from `n8n-workflow` or surrounded with a try/catch block.',
 				recommended: 'error',
 			},
 			schema: [],
 			messages: {
-				noUncaughtJsonParse: 'Surround the JSON.parse() call with a try/catch block.',
+				noUncaughtJsonParse:
+					'Use `jsonParse()` from `n8n-workflow` or surround the `JSON.parse()` call with a try/catch block.',
 			},
 		},
 		defaultOptions: [],
diff --git a/packages/workflow/src/utils.ts b/packages/workflow/src/utils.ts
index 400cfc3523..c8694c801f 100644
--- a/packages/workflow/src/utils.ts
+++ b/packages/workflow/src/utils.ts
@@ -30,22 +30,23 @@ export const deepCopy = <T>(source: T): T => {
 	return clone;
 };
 // eslint-enable
-type ErrorMessage = { errorMessage: string };
-type FallbackValue<T> = { fallbackValue: T };
 
-export const jsonParse = <T>(
-	jsonString: string,
-	options: ErrorMessage | FallbackValue<T> | {} = {},
-): T => {
+type MutuallyExclusive<T, U> =
+	| (T & { [k in Exclude<keyof U, keyof T>]?: never })
+	| (U & { [k in Exclude<keyof T, keyof U>]?: never });
+
+type JSONParseOptions<T> = MutuallyExclusive<{ errorMessage: string }, { fallbackValue: T }>;
+
+export const jsonParse = <T>(jsonString: string, options?: JSONParseOptions<T>): T => {
 	try {
 		return JSON.parse(jsonString) as T;
 	} catch (error) {
-		if ('fallbackValue' in options) {
+		if (options?.fallbackValue !== undefined) {
 			return options.fallbackValue;
-		}
-		if ('errorMessage' in options) {
+		} else if (options?.errorMessage) {
 			throw new Error(options.errorMessage);
 		}
+
 		throw error;
 	}
 };
diff --git a/packages/workflow/src/utils.test.ts b/packages/workflow/test/utils.test.ts
similarity index 56%
rename from packages/workflow/src/utils.test.ts
rename to packages/workflow/test/utils.test.ts
index ad86753951..6ec35877b8 100644
--- a/packages/workflow/src/utils.test.ts
+++ b/packages/workflow/test/utils.test.ts
@@ -1,4 +1,21 @@
-import { deepCopy } from './utils';
+import { jsonParse, deepCopy } from '../src/utils';
+
+describe('jsonParse', () => {
+	it('parses JSON', () => {
+		expect(jsonParse('[1, 2, 3]')).toEqual([1, 2, 3]);
+		expect(jsonParse('{ "a": 1 }')).toEqual({ a: 1 });
+	});
+
+	it('optionally throws `errorMessage', () => {
+		expect(() => {
+			jsonParse('', { errorMessage: 'Invalid JSON' });
+		}).toThrow('Invalid JSON');
+	});
+
+	it('optionally returns a `fallbackValue`', () => {
+		expect(jsonParse('', { fallbackValue: { foo: 'bar' } })).toEqual({ foo: 'bar' });
+	});
+});
 
 describe('deepCopy', () => {
 	it('should deep copy an object', () => {