fix bug in providable where it should only dispose of older disposers

This was causing values to be discarded after the first change which is
a mistake. The disposer should be called only if it recieves a new one
(meaning a different provider has entered the ring).

Author: keithamus

src/providable.ts

@@ -58,7 +58,10 @@ const [consume, getConsume, initConsume] = createMark<CustomElement>(
         (value: unknown, dispose?: () => void) => {
           if (!disposes.has(instance)) disposes.set(instance, new Map())
           const instanceDisposes = disposes.get(instance)!
-          if (instanceDisposes.has(name)) instanceDisposes.get(name)!()
+          if (instanceDisposes.has(name)) {
+            const oldDispose = instanceDisposes.get(name)!
+            if (oldDispose !== dispose) oldDispose()
+          }
           if (dispose) instanceDisposes.set(name, dispose)
           currentValue = value
           access.set?.call(instance, currentValue)
@@ -84,18 +87,20 @@ export const providable = createAbility(
         super(...args)
         initProvide(this)
         if (getProvide(this).size) {
+          if (!contexts.has(this)) contexts.set(this, new Map())
+          const instanceContexts = contexts.get(this)!
           this.addEventListener('context-request', event => {
             if (!isContextEvent(event)) return
             const name = event.context.name
             const value = this[name]
-            const callback = event.callback
+            const dispose = () => instanceContexts.get(name)?.delete(callback)
+            const eventCallback = event.callback
+            const callback = (newValue: unknown) => eventCallback(newValue, dispose)
             if (event.multiple) {
-              if (!contexts.has(this)) contexts.set(this, new Map())
-              const instanceContexts = contexts.get(this)!
               if (!instanceContexts.has(name)) instanceContexts.set(name, new Set())
               instanceContexts.get(name)!.add(callback)
             }
-            callback(value, () => contexts.get(this)?.get(name)?.delete(callback))
+            callback(value)
           })
         }
       }

test/providable.ts

@@ -39,9 +39,15 @@ describe('Providable', () => {
 
   describe('consumer without provider', () => {
     let instance: ProvidableConsumerTest
+    let events = fake()
     beforeEach(async () => {
+      events = fake()
+      document.body.addEventListener('context-request', events)
       instance = await fixture(html`<providable-consumer-test />`)
     })
+    afterEach(() => {
+      document.body.removeEventListener('context-request', events)
+    })
 
     it('uses the given values', () => {
       expect(instance).to.have.property('foo', 'goodbye')
@@ -59,11 +65,6 @@ describe('Providable', () => {
     })
 
     it('emits the `context-request` event when connected, for each field', async () => {
-      instance = document.createElement('providable-consumer-test') as ProvidableConsumerTest
-      const events = fake()
-      instance.addEventListener('context-request', events)
-      await fixture(instance)
-
       expect(events).to.have.callCount(5)
       const fooEvent = events.getCall(0).args[0]
       expect(fooEvent).to.be.instanceof(ContextEvent)
@@ -100,6 +101,34 @@ describe('Providable', () => {
       expect(quxEvent).to.have.property('multiple', true)
       expect(quxEvent).to.have.property('bubbles', true)
     })
+
+    it('changes value based on callback new value', async () => {
+      expect(events).to.have.callCount(5)
+      const fooCallback = events.getCall(0).args[0].callback
+      fooCallback('hello')
+      expect(instance).to.have.property('foo', 'hello')
+      fooCallback('goodbye')
+      expect(instance).to.have.property('foo', 'goodbye')
+    })
+
+    it('disposes of past callbacks when given new ones', async () => {
+      const dispose1 = fake()
+      const dispose2 = fake()
+      expect(events).to.have.callCount(5)
+      const fooCallback = events.getCall(0).args[0].callback
+      fooCallback('hello', dispose1)
+      expect(dispose1).to.have.callCount(0)
+      expect(dispose2).to.have.callCount(0)
+      fooCallback('goodbye', dispose1)
+      expect(dispose1).to.have.callCount(0)
+      expect(dispose2).to.have.callCount(0)
+      fooCallback('greetings', dispose2)
+      expect(dispose1).to.have.callCount(1)
+      expect(dispose2).to.have.callCount(0)
+      fooCallback('hola', dispose1)
+      expect(dispose1).to.have.callCount(1)
+      expect(dispose2).to.have.callCount(1)
+    })
   })
 
   describe('provider', () => {
@@ -190,6 +219,9 @@ describe('Providable', () => {
       provider.qux = 17
       expect(consumer).to.have.property('qux', 17)
       expect(consumer).to.have.property('count', 2)
+      provider.qux = 18
+      expect(consumer).to.have.property('qux', 18)
+      expect(consumer).to.have.property('count', 3)
     })
   })