Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Commit

Permalink
fix(storage): return type of get is unknown
Browse files Browse the repository at this point in the history
The `any` is very loose, 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.
  • Loading branch information
mxdvl committed Aug 4, 2022
1 parent f80ad67 commit 0c9a0a6
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 20 deletions.
2 changes: 1 addition & 1 deletion src/storage/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ _n.b. the examples below use `storage.local`, but all methods are available for

## `get(key)`

Returns: `string` | `number` | `boolean` | `null` | `object` | `array`
Returns: `unknown`

Retrieves an item from storage.

Expand Down
6 changes: 5 additions & 1 deletion src/storage/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ describe.each([
// re-import now we've disabled native storage API
const { storage } = await import('./storage');
expect(() => storage[name].set('🚫', true)).not.toThrowError();
// eslint-disable-next-line @typescript-eslint/no-unsafe-return -- it's correct
expect(() => storage[name].get('🚫')).not.toThrowError();
expect(() => storage[name].remove('🚫')).not.toThrowError();
expect(() => storage[name].getRaw('🚫')).not.toThrowError();
Expand Down Expand Up @@ -87,6 +86,11 @@ describe.each([
expect(native.getItem('iAmExpired')).toBeNull();
});

it(`return null for a malformed item`, () => {
native.setItem('malformed', '[]');
expect(implementation.get('malformed')).toBeNull();
});

it(`returns a non-expired item`, () => {
implementation.set('iAmNotExpired', 'data', new Date('2040-01-01'));
expect(implementation.get('iAmNotExpired')).toBeTruthy();
Expand Down
26 changes: 8 additions & 18 deletions src/storage/storage.ts
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -22,27 +25,19 @@ 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;

// is this item has passed its sell-by-date, remove it
if (expires && new Date() > new Date(expires)) {
if (isString(expires) && new Date() > new Date(expires)) {
this.remove(key);
return null;
}
Expand All @@ -52,11 +47,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.
Expand Down

0 comments on commit 0c9a0a6

Please sign in to comment.