fix: preserve state if component disconnects then immediately reconnects (#313)

Fixes #312

src/database/databaseLifecycle.js

@@ -1,7 +1,7 @@
 import { initialMigration } from './migrations'
 import { DB_VERSION_INITIAL, DB_VERSION_CURRENT } from './constants'
 
-const openReqs = {}
+export const openIndexedDBRequests = {}
 const databaseCache = {}
 const onCloseListeners = {}
 
@@ -18,7 +18,7 @@ async function createDatabase (dbName) {
   performance.mark('createDatabase')
   const db = await new Promise((resolve, reject) => {
     const req = indexedDB.open(dbName, DB_VERSION_CURRENT)
-    openReqs[dbName] = req
+    openIndexedDBRequests[dbName] = req
     req.onupgradeneeded = e => {
       // Technically there is only one version, so we don't need this `if` check
       // But if an old version of the JS is in another browser tab
@@ -69,7 +69,7 @@ export function dbPromise (db, storeName, readOnlyOrReadWrite, cb) {
 
 export function closeDatabase (dbName) {
   // close any open requests
-  const req = openReqs[dbName]
+  const req = openIndexedDBRequests[dbName]
   const db = req && req.result
   if (db) {
     db.close()
@@ -81,7 +81,7 @@ export function closeDatabase (dbName) {
       }
     }
   }
-  delete openReqs[dbName]
+  delete openIndexedDBRequests[dbName]
   delete databaseCache[dbName]
   delete onCloseListeners[dbName]
 }

src/picker/PickerElement.js

@@ -48,22 +48,31 @@ export default class PickerElement extends HTMLElement {
   }
 
   connectedCallback () {
-    this._cmp = new SveltePicker({
-      target: this.shadowRoot,
-      props: this._ctx
-    })
+    // The _cmp may be defined if the component was immediately disconnected and then reconnected. In that case,
+    // do nothing (preserve the state)
+    if (!this._cmp) {
+      this._cmp = new SveltePicker({
+        target: this.shadowRoot,
+        props: this._ctx
+      })
+    }
   }
 
   disconnectedCallback () {
-    this._cmp.$destroy()
-    this._cmp = undefined
+    // Check in a microtask if the element is still connected. If so, treat this as a "move" rather than a disconnect
+    // Inspired by Vue: https://vuejs.org/guide/extras/web-components.html#building-custom-elements-with-vue
+    Promise.resolve().then(() => {
+      // this._cmp may be defined if connect-disconnect-connect-disconnect occurs synchronously
+      if (!this.isConnected && this._cmp) {
+        this._cmp.$destroy()
+        this._cmp = undefined
 
-    const { database } = this._ctx
-    if (database) {
-      database.close()
-        // only happens if the database failed to load in the first place, so we don't care)
-        .catch(err => console.error(err))
-    }
+        const { database } = this._ctx
+        database.close()
+          // only happens if the database failed to load in the first place, so we don't care
+          .catch(err => console.error(err))
+      }
+    })
   }
 
   static get observedAttributes () {

test/spec/picker/lifecycle.test.js

@@ -2,6 +2,7 @@ import { basicAfterEach, basicBeforeEach, tick } from '../shared'
 import Picker from '../../../src/picker/PickerElement'
 import { getByRole, waitFor } from '@testing-library/dom'
 import { DEFAULT_DATA_SOURCE } from '../../../src/database/constants'
+import { openIndexedDBRequests } from '../../../src/database/databaseLifecycle.js'
 
 describe('lifecycle', () => {
   beforeEach(basicBeforeEach)
@@ -29,7 +30,8 @@ describe('lifecycle', () => {
     expect(fetch).toHaveBeenLastCalledWith(DEFAULT_DATA_SOURCE, { method: 'HEAD' })
 
     document.body.removeChild(picker)
-    await tick(20)
+    await tick(60)
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
   })
 
   test('database.close() is called when disconnected', async () => {
@@ -42,12 +44,13 @@ describe('lifecycle', () => {
     const spy = jest.spyOn(picker.database, 'close')
 
     document.body.removeChild(picker)
-    await tick(20)
+    await tick(60)
 
     expect(spy).toHaveBeenCalled()
     expect(spy).toHaveBeenCalledTimes(1)
 
     spy.mockRestore()
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
   })
 
   test('connect and immediately disconnect', async () => {
@@ -55,10 +58,25 @@ describe('lifecycle', () => {
     document.body.appendChild(picker)
     document.body.removeChild(picker)
 
-    await tick(20)
+    await tick(60)
 
     expect(fetch).toHaveBeenCalledTimes(1)
     expect(fetch).toHaveBeenLastCalledWith(DEFAULT_DATA_SOURCE, undefined)
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
+  })
+
+  test('connect and immediately disconnect twice', async () => {
+    const picker = new Picker()
+    document.body.appendChild(picker)
+    document.body.removeChild(picker)
+    document.body.appendChild(picker)
+    document.body.removeChild(picker)
+
+    await tick(60)
+
+    expect(fetch).toHaveBeenCalledTimes(1)
+    expect(fetch).toHaveBeenLastCalledWith(DEFAULT_DATA_SOURCE, undefined)
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
   })
 
   test('connect, disconnect, and reconnect with a particular timing (#225)', async () => {
@@ -74,6 +92,85 @@ describe('lifecycle', () => {
 
     await tick(20)
     document.body.removeChild(picker)
+    await tick(60)
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
+  })
+
+  test('preserves state if component is disconnected and reconnected synchronously', async () => {
+    const picker = new Picker()
+    document.body.appendChild(picker)
     await tick(20)
+
+    expect(fetch).toHaveBeenCalledTimes(1)
+    expect(fetch).toHaveBeenLastCalledWith(DEFAULT_DATA_SOURCE, undefined)
+    await expect(() => (
+      expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
+    ))
+
+    document.body.removeChild(picker)
+    document.body.appendChild(picker)
+
+    await tick(20)
+
+    expect(fetch).toHaveBeenCalledTimes(1) // fetch is not called again because no re-render
+    await expect(() => (
+      expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
+    ))
+
+    await tick(20)
+    document.body.removeChild(picker)
+    await tick(60)
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
+  })
+
+  test('does not preserve state if component is disconnected and reconnected in separate microtasks', async () => {
+    const picker = new Picker()
+    document.body.appendChild(picker)
+    await tick(20)
+
+    expect(fetch).toHaveBeenCalledTimes(1)
+    expect(fetch).toHaveBeenLastCalledWith(DEFAULT_DATA_SOURCE, undefined)
+    await expect(() => (
+      expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
+    ))
+
+    document.body.removeChild(picker)
+    await Promise.resolve()
+    document.body.appendChild(picker)
+
+    await tick(20)
+
+    expect(fetch).toHaveBeenCalledTimes(2) // fetch is called again due to re-render
+    expect(fetch).toHaveBeenLastCalledWith(DEFAULT_DATA_SOURCE, { method: 'HEAD' }) // cached, so does a HEAD
+    await expect(() => (
+      expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
+    ))
+
+    await tick(20)
+    document.body.removeChild(picker)
+    await tick(60)
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
+  })
+
+  test('connect and immediately disconnect twice, then immediately reconnect', async () => {
+    const picker = new Picker()
+    document.body.appendChild(picker)
+    document.body.removeChild(picker)
+    document.body.appendChild(picker)
+    document.body.removeChild(picker)
+    document.body.appendChild(picker)
+
+    await tick(20)
+
+    expect(fetch).toHaveBeenCalledTimes(1)
+    expect(fetch).toHaveBeenLastCalledWith(DEFAULT_DATA_SOURCE, undefined)
+    await expect(() => (
+      expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
+    ))
+
+    await tick(20)
+    document.body.removeChild(picker)
+    await tick(60)
+    expect(Object.keys(openIndexedDBRequests).length).toBe(0) // no open IDB connections
   })
 })