fix: remove custom storage option and interface

.changeset/smooth-seahorses-unite.md

@@ -2,4 +2,4 @@
 '@segment/analytics-next': minor
 ---
 
-Adds storage option in analytics client to specify priority of storage (e.g use cookies over localstorage) or use a custom implementation
+Adds storage option in analytics client to specify priority of storage (e.g use cookies over localstorage)

packages/browser/src/core/analytics/__tests__/integration.test.ts

@@ -288,7 +288,13 @@ describe('Analytics', () => {
       const analytics = new Analytics(
         { writeKey: '' },
         {
-          storage: [StoreType.Cookie, StoreType.LocalStorage, StoreType.Memory],
+          storage: {
+            stores: [
+              StoreType.Cookie,
+              StoreType.LocalStorage,
+              StoreType.Memory,
+            ],
+          },
         }
       )
 
@@ -308,7 +314,9 @@ describe('Analytics', () => {
       const analytics = new Analytics(
         { writeKey: '' },
         {
-          storage: [StoreType.Cookie, StoreType.Memory],
+          storage: {
+            stores: [StoreType.Cookie, StoreType.Memory],
+          },
         }
       )
 

packages/browser/src/core/analytics/index.ts

@@ -46,13 +46,11 @@ import {
   CookieOptions,
   MemoryStorage,
   UniversalStorage,
-  Storage,
   StorageSettings,
   StoreType,
   applyCookieOptions,
   initializeStorages,
   isArrayOfStoreType,
-  isStorageObject,
 } from '../storage'
 
 const deprecationWarning =
@@ -138,7 +136,7 @@ export class Analytics
   private _group: Group
   private eventFactory: EventFactory
   private _debug = false
-  private _universalStorage: Storage
+  private _universalStorage: UniversalStorage
 
   initialized = false
   integrations: Integrations
@@ -213,22 +211,19 @@ export class Analytics
     disablePersistance: boolean,
     storageSetting: InitOptions['storage'],
     cookieOptions?: CookieOptions | undefined
-  ): Storage {
+  ): UniversalStorage {
     // DisablePersistance option overrides all, no storage will be used outside of memory even if specified
     if (disablePersistance) {
-      return new MemoryStorage()
+      return new UniversalStorage([new MemoryStorage()])
     } else {
       if (storageSetting) {
         if (isArrayOfStoreType(storageSetting)) {
           // We will create the store with the priority for customer settings
           return new UniversalStorage(
             initializeStorages(
-              applyCookieOptions(storageSetting, cookieOptions)
+              applyCookieOptions(storageSetting.stores, cookieOptions)
             )
           )
-        } else if (isStorageObject(storageSetting)) {
-          // If it is an object we will use the customer provided storage
-          return storageSetting
         }
       }
     }
@@ -245,7 +240,7 @@ export class Analytics
     )
   }
 
-  get storage(): Storage {
+  get storage(): UniversalStorage {
     return this._universalStorage
   }
 

packages/browser/src/core/storage/__tests__/cookieStorage.test.ts

@@ -1,6 +1,5 @@
 import { CookieStorage } from '../cookieStorage'
 import jar from 'js-cookie'
-import { disableCookies } from './test-helpers'
 
 describe('cookieStorage', () => {
   function clearCookies() {
@@ -15,23 +14,6 @@ describe('cookieStorage', () => {
     clearCookies()
   })
 
-  describe('#available', () => {
-    afterEach(() => {
-      jest.restoreAllMocks()
-    })
-
-    it('is available', () => {
-      const cookie = new CookieStorage()
-      expect(cookie.available).toBe(true)
-    })
-
-    it("is unavailable if can't write cookies", () => {
-      disableCookies()
-      const cookie = new CookieStorage()
-      expect(cookie.available).toBe(false)
-    })
-  })
-
   describe('cookie options', () => {
     it('should have default cookie options', () => {
       const cookie = new CookieStorage()

packages/browser/src/core/storage/__tests__/localStorage.test.ts

@@ -63,7 +63,7 @@ describe('LocalStorage', function () {
     it('should be able to remove a record', function () {
       store.set('x', { a: 'b' })
       expect(store.get('x')).toStrictEqual({ a: 'b' })
-      store.clear('x')
+      store.remove('x')
       expect(store.get('x')).toBe(null)
     })
   })

packages/browser/src/core/storage/__tests__/test-helpers.ts

@@ -1,19 +1,27 @@
+import jar from 'js-cookie'
 /**
  * Disables Cookies
  * @returns jest spy
  */
-export function disableCookies(): jest.SpyInstance {
-  return jest
-    .spyOn(window.navigator, 'cookieEnabled', 'get')
-    .mockReturnValue(false)
+export function disableCookies(): void {
+  jest.spyOn(window.navigator, 'cookieEnabled', 'get').mockReturnValue(false)
+  jest.spyOn(jar, 'set').mockImplementation(() => {
+    throw new Error()
+  })
+  jest.spyOn(jar, 'get').mockImplementation(() => {
+    throw new Error()
+  })
 }
 
 /**
  * Disables LocalStorage
  * @returns jest spy
  */
-export function disableLocalStorage(): jest.SpyInstance {
-  return jest.spyOn(Storage.prototype, 'setItem').mockImplementation(() => {
+export function disableLocalStorage(): void {
+  jest.spyOn(Storage.prototype, 'setItem').mockImplementation(() => {
+    throw new Error()
+  })
+  jest.spyOn(Storage.prototype, 'getItem').mockImplementation(() => {
     throw new Error()
   })
 }

packages/browser/src/core/storage/__tests__/universalStorage.test.ts

@@ -103,29 +103,27 @@ describe('UniversalStorage', function () {
       expect(us.get('ajs_test_key')).toEqual('💰')
     })
 
-    it('does not write to cookies when cookies are not available', function () {
-      disableCookies()
+    it('handles cookie errors gracefully', function () {
+      disableCookies() // Cookies is going to throw exceptions now
       const us = new UniversalStorage([
         new LocalStorage(),
         new CookieStorage(),
         new MemoryStorage(),
       ])
       us.set('ajs_test_key', '💰')
-      expect(jar.get('ajs_test_key')).toEqual(undefined)
       expect(getFromLS('ajs_test_key')).toEqual('💰')
       expect(us.get('ajs_test_key')).toEqual('💰')
     })
 
     it('does not write to LS when LS is not available', function () {
-      disableLocalStorage()
+      disableLocalStorage() // Localstorage will throw exceptions
       const us = new UniversalStorage([
         new LocalStorage(),
         new CookieStorage(),
         new MemoryStorage(),
       ])
       us.set('ajs_test_key', '💰')
       expect(jar.get('ajs_test_key')).toEqual('💰')
-      expect(localStorage.getItem('ajs_test_key')).toEqual(null)
       expect(us.get('ajs_test_key')).toEqual('💰')
     })
   })

packages/browser/src/core/storage/cookieStorage.ts

@@ -1,4 +1,4 @@
-import { BaseStorage, StorageObject, StoreType } from './types'
+import { Store, StorageObject } from './types'
 import jar from 'js-cookie'
 import { tld } from '../user/tld'
 
@@ -15,25 +15,9 @@ export interface CookieOptions {
 /**
  * Data storage using browser cookies
  */
-export class CookieStorage<
-  Data extends StorageObject = StorageObject
-> extends BaseStorage {
-  get available(): boolean {
-    let cookieEnabled = window.navigator.cookieEnabled
-
-    if (!cookieEnabled) {
-      jar.set('ajs:cookies', 'test')
-      cookieEnabled = document.cookie.includes('ajs:cookies')
-      jar.remove('ajs:cookies')
-    }
-
-    return cookieEnabled
-  }
-
-  get type() {
-    return StoreType.Cookie
-  }
-
+export class CookieStorage<Data extends StorageObject = StorageObject>
+  implements Store<Data>
+{
   static get defaults(): CookieOptions {
     return {
       maxage: ONE_YEAR,
@@ -46,7 +30,6 @@ export class CookieStorage<
   private options: Required<CookieOptions>
 
   constructor(options: CookieOptions = CookieStorage.defaults) {
-    super()
     this.options = {
       ...CookieStorage.defaults,
       ...options,
@@ -91,7 +74,7 @@ export class CookieStorage<
     }
   }
 
-  clear<K extends keyof Data>(key: K): void {
+  remove<K extends keyof Data>(key: K): void {
     return jar.remove(key, this.opts())
   }
 }

packages/browser/src/core/storage/index.ts

@@ -2,7 +2,7 @@ import { CookieOptions, CookieStorage } from './cookieStorage'
 import { LocalStorage } from './localStorage'
 import { MemoryStorage } from './memoryStorage'
 import { isStoreTypeWithSettings } from './settings'
-import { StoreType, Storage, InitializeStorageArgs } from './types'
+import { StoreType, Store, InitializeStorageArgs } from './types'
 
 export * from './types'
 export * from './localStorage'
@@ -16,7 +16,7 @@ export * from './settings'
  * @param args StoreType and options
  * @returns Storage array
  */
-export function initializeStorages(args: InitializeStorageArgs): Storage[] {
+export function initializeStorages(args: InitializeStorageArgs): Store[] {
   const storages = args.map((s) => {
     let type: StoreType
     let settings

packages/browser/src/core/storage/localStorage.ts

@@ -1,30 +1,15 @@
-import { BaseStorage, StorageObject, StoreType } from './types'
+import { StorageObject, Store } from './types'
 
 /**
  * Data storage using browser's localStorage
  */
-export class LocalStorage<
-  Data extends StorageObject = StorageObject
-> extends BaseStorage {
+export class LocalStorage<Data extends StorageObject = StorageObject>
+  implements Store<Data>
+{
   private localStorageWarning(key: keyof Data, state: 'full' | 'unavailable') {
     console.warn(`Unable to access ${key}, localStorage may be ${state}`)
   }
 
-  get type() {
-    return StoreType.LocalStorage
-  }
-
-  get available(): boolean {
-    const test = 'test'
-    try {
-      localStorage.setItem(test, test)
-      localStorage.removeItem(test)
-      return true
-    } catch (e) {
-      return false
-    }
-  }
-
   get<K extends keyof Data>(key: K): Data[K] | null {
     try {
       const val = localStorage.getItem(key)
@@ -50,7 +35,7 @@ export class LocalStorage<
     }
   }
 
-  clear<K extends keyof Data>(key: K): void {
+  remove<K extends keyof Data>(key: K): void {
     try {
       return localStorage.removeItem(key)
     } catch (err) {

packages/browser/src/core/storage/memoryStorage.ts

@@ -1,21 +1,13 @@
-import { BaseStorage, StorageObject, StoreType } from './types'
+import { Store, StorageObject } from './types'
 
 /**
  * Data Storage using in memory object
  */
-export class MemoryStorage<
-  Data extends StorageObject = StorageObject
-> extends BaseStorage<Data> {
+export class MemoryStorage<Data extends StorageObject = StorageObject>
+  implements Store<Data>
+{
   private cache: Record<string, unknown> = {}
 
-  get type() {
-    return StoreType.Memory
-  }
-
-  get available(): boolean {
-    return true
-  }
-
   get<K extends keyof Data>(key: K): Data[K] | null {
     return (this.cache[key] ?? null) as Data[K] | null
   }
@@ -24,7 +16,7 @@ export class MemoryStorage<
     this.cache[key] = value
   }
 
-  clear<K extends keyof Data>(key: K): void {
+  remove<K extends keyof Data>(key: K): void {
     delete this.cache[key]
   }
 }

packages/browser/src/core/storage/settings.ts

@@ -1,19 +1,21 @@
-import { Storage, StoreType, StoreTypeWithSettings } from './types'
+import { StoreType, StoreTypeWithSettings } from './types'
 
-export type StorageSettings = Storage | StoreType[]
+export type UniversalStorageSettings = { stores: StoreType[] }
 
-export function isArrayOfStoreType(s: StorageSettings): s is StoreType[] {
+// This is setup this way to permit eventually a different set of settings for custom storage
+export type StorageSettings = UniversalStorageSettings
+
+export function isArrayOfStoreType(
+  s: StorageSettings
+): s is UniversalStorageSettings {
   return (
     s &&
-    Array.isArray(s) &&
-    s.every((e) => Object.values(StoreType).includes(e))
+    s.stores &&
+    Array.isArray(s.stores) &&
+    s.stores.every((e) => Object.values(StoreType).includes(e))
   )
 }
 
-export function isStorageObject(s: StorageSettings): s is Storage {
-  return s && !Array.isArray(s) && typeof s === 'object' && s.get !== undefined
-}
-
 export function isStoreTypeWithSettings(
   s: StoreTypeWithSettings | StoreType
 ): s is StoreTypeWithSettings {