From 278681b5c4f887da29de123b0ad676f53cb94e32 Mon Sep 17 00:00:00 2001
From: Max Duval <max.duval@theguardian.com>
Date: Thu, 4 Aug 2022 13:52:03 +0100
Subject: [PATCH] fix(storage): return type `unkown`

`any` is too loose a value, and consumers
should make appropriate decision about
the data, so `unknown` is more appropriate.

The motivation comes from a bug that made its
way into production recently.
---
 src/storage/storage.ts | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/src/storage/storage.ts b/src/storage/storage.ts
index ba0c3c8e..04f0c8f8 100644
--- a/src/storage/storage.ts
+++ b/src/storage/storage.ts
@@ -1,3 +1,6 @@
+import { isObject } from '../isObject/isObject';
+import { isString } from '../isString/isString';
+
 class StorageFactory {
 	#storage: Storage | undefined; // https://mdn.io/Private_class_fields
 
@@ -22,24 +25,17 @@ class StorageFactory {
 		return Boolean(this.#storage);
 	}
 
-	/* eslint-disable
-		@typescript-eslint/no-unsafe-assignment,
-		@typescript-eslint/no-explicit-any,
-		@typescript-eslint/no-unsafe-argument
-		--
-		- we're using the `try` to handle anything bad happening
-		- JSON.parse returns an `any`, we really are with an `any`
-	*/
 	/**
 	 * Retrieve an item from storage.
 	 *
 	 * @param key - the name of the item
 	 */
-	get(key: string): any {
+	get(key: string): unknown {
 		try {
-			const { value, expires } = JSON.parse(
-				this.#storage?.getItem(key) ?? '',
-			);
+			const data: unknown = JSON.parse(this.#storage?.getItem(key) ?? '');
+			if (!isObject(data)) return null;
+			const { value, expires } = data;
+			if (!isString(expires)) return null;
 
 			// is this item has passed its sell-by-date, remove it
 			if (expires && new Date() > new Date(expires)) {
@@ -52,11 +48,6 @@ class StorageFactory {
 			return null;
 		}
 	}
-	/* eslint-enable
-		@typescript-eslint/no-unsafe-assignment,
-		@typescript-eslint/no-explicit-any,
-		@typescript-eslint/no-unsafe-argument
-	*/
 
 	/**
 	 * Save a value to storage.