feat: removeItem when storing null or undefined (#93)

Author: cyrilletuzi

CHANGELOG.md

@@ -28,6 +28,7 @@ It was time to do a full review and refactoring, which results in:
 - `indexedDB` database and object store names default values are exported and can be changed
 (see the [interoperability guide](./docs/INTEROPERABILITY.md))
 - `indexedDB` storage will now works in web workers too
+- When trying to store `null` or `undefined`, `removeItem()` instead of just bypassing (meaning the old value was kept)
 
 ### Breaking changes
 

README.md

@@ -87,6 +87,8 @@ this.localStorage.setItem('user', user).subscribe(() => {});
 
 You can store any value, without worrying about stringifying.
 
+Storing `null` or `undefined` can cause issues in some browsers, so the item will be removed instead.
+
 ### Deleting data
 
 To delete one item:

projects/ngx-pwa/local-storage/src/lib/databases/indexeddb-database.ts

@@ -142,13 +142,9 @@ export class IndexedDBDatabase implements LocalDatabase {
    */
   setItem(key: string, data: any): Observable<boolean> {
 
-    /* Storing `null` or `undefined` is known to cause issues in some browsers.
-     * So it's useless, not storing anything in this case */
+    /* Storing `undefined` or `null` in `localStorage` can cause issues in some browsers so removing item instead */
     if ((data === undefined) || (data === null)) {
-
-      /* Trigger success */
-      return of(true);
-
+      return this.removeItem(key);
     }
 
     /* Open a transaction in write mode */

projects/ngx-pwa/local-storage/src/lib/databases/localstorage-database.ts

@@ -77,25 +77,25 @@ export class LocalStorageDatabase implements LocalDatabase {
    */
   setItem(key: string, data: any): Observable<boolean> {
 
-    /* Storing `undefined` or `null` in `localStorage` can cause issues in some browsers and has no sense */
-    if ((data !== undefined) && (data !== null)) {
-
-      let serializedData: string | null = null;
+    /* Storing `undefined` or `null` in `localStorage` can cause issues in some browsers so removing item instead */
+    if ((data === undefined) || (data === null)) {
+      return this.removeItem(key);
+    }
 
-      /* Try to stringify (can fail on circular references) */
-      try {
-        serializedData = JSON.stringify(data);
-      } catch (error) {
-        return throwError(error as TypeError);
-      }
+    let serializedData: string | null = null;
 
-      /* Can fail if storage quota is exceeded */
-      try {
-        localStorage.setItem(this.prefixKey(key), serializedData);
-      } catch (error) {
-        return throwError(error as DOMException);
-      }
+    /* Try to stringify (can fail on circular references) */
+    try {
+      serializedData = JSON.stringify(data);
+    } catch (error) {
+      return throwError(error as TypeError);
+    }
 
+    /* Can fail if storage quota is exceeded */
+    try {
+      localStorage.setItem(this.prefixKey(key), serializedData);
+    } catch (error) {
+      return throwError(error as DOMException);
     }
 
     /* Wrap in a RxJS `Observable` to be consistent with other storages */

projects/ngx-pwa/local-storage/src/lib/databases/memory-database.ts

@@ -46,13 +46,13 @@ export class MemoryDatabase implements LocalDatabase {
    */
    setItem(key: string, data: any): Observable<boolean> {
 
-    /* Storing `undefined` or `null` in `localStorage` is useless */
-    if ((data !== undefined) && (data !== null)) {
-
-      this.memoryStorage.set(key, data);
-
+    /* Storing `undefined` or `null` in `localStorage` is useless, so removing item instead */
+    if ((data === undefined) || (data === null)) {
+      return this.removeItem(key);
     }
 
+    this.memoryStorage.set(key, data);
+
     /* Wrap in a RxJS `Observable` to be consistent with other storages */
     return of(true);
 

projects/ngx-pwa/local-storage/src/lib/lib.service.spec.ts

@@ -165,10 +165,9 @@ function tests(description: string, localStorageServiceFactory: () => LocalStora
 
       it('null', (done) => {
 
-        const value = null;
-
-        localStorageService.setItem(key, value).pipe(
-          mergeMap(() => localStorageService.getItem(key))
+        localStorageService.setItem(key, 'test').pipe(
+          mergeMap(() => localStorageService.setItem(key, null)),
+          mergeMap(() => localStorageService.getItem(key)),
         ).subscribe((result) => {
 
           expect(result).toBeNull();
@@ -181,10 +180,9 @@ function tests(description: string, localStorageServiceFactory: () => LocalStora
 
       it('undefined', (done) => {
 
-        const value = undefined;
-
-        localStorageService.setItem(key, value).pipe(
-          mergeMap(() => localStorageService.getItem(key))
+        localStorageService.setItem(key, 'test').pipe(
+          mergeMap(() => localStorageService.setItem(key, undefined)),
+          mergeMap(() => localStorageService.getItem(key)),
         ).subscribe((result) => {
 
           expect(result).toBeNull();