plugins/graphiql: more tests for StorageBucket and fix removeItem

plugins/graphiql/src/lib/storage/StorageBucket.test.ts

@@ -28,6 +28,13 @@ describe('StorageBucket', () => {
     }).toThrow('Direct property access is not allowed for StorageBuckets');
   });
 
+  it('should not implement all methods', () => {
+    const bucket = StorageBucket.forLocalStorage('hello');
+
+    expect(() => bucket.length).toThrow('Method not implemented.');
+    expect(() => bucket.key()).toThrow('Method not implemented.');
+  });
+
   describe('with mocked underlying storage', () => {
     const mockStorage = {
       getItem: jest.fn(),
@@ -88,5 +95,49 @@ describe('StorageBucket', () => {
       expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
       expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
     });
+
+    it('should remove an item', () => {
+      mockStorage.getItem.mockReturnValueOnce(
+        JSON.stringify({ x: 'X', y: 'Y' }),
+      );
+      bucket.removeItem('x');
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.setItem).toHaveBeenLastCalledWith(
+        'my-bucket',
+        JSON.stringify({ y: 'Y' }),
+      );
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+    });
+
+    it('should not bother to write when deleting a missing key', () => {
+      mockStorage.getItem.mockReturnValueOnce(JSON.stringify({ y: 'Y' }));
+      bucket.removeItem('x');
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+    });
+
+    it('should ignore bad data', () => {
+      mockStorage.getItem.mockReturnValue('derp');
+
+      expect(bucket.getItem('x')).toBe(null);
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(1);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+
+      bucket.removeItem('x');
+
+      expect(mockStorage.getItem).toHaveBeenCalledTimes(2);
+      expect(mockStorage.getItem).toHaveBeenLastCalledWith('my-bucket');
+      expect(mockStorage.setItem).toHaveBeenCalledTimes(0);
+      expect(mockStorage.removeItem).toHaveBeenCalledTimes(0);
+    });
   });
 });

plugins/graphiql/src/lib/storage/StorageBucket.ts

@@ -71,8 +71,8 @@ export class StorageBucket implements Storage {
       return;
     }
 
-    const deleted = delete data[key];
-    if (deleted) {
+    if (key in data) {
+      delete data[key];
       this.write(data);
     }
   }