Array flat fix

index.js

@@ -214,7 +214,7 @@ class Reactor {
 
     // Early rejection for non-objects
     // Could be handled later by proxy creation, but cleaner to have the logic here
-    // Can use the same function as Signal does for it's wrapping
+    // Can use the same function as Signal does for its wrapping
     if (!isObject(initializedSource)) {
       throw new TypeError('Reactor source must be an Object')
     }
@@ -244,7 +244,24 @@ class Reactor {
           // `proxiedMap.keys()` will work because keys gets wrapped by this handler
           // `Map.prototype.keys.call(proxiedMap)` won't work because it doesnt get wrapped
           try {
-            return Reflect.apply(this.source, thisArg, argumentsList)
+            const result = Reflect.apply(this.source, thisArg, argumentsList)
+            // flat() reads elements through the proxy to build dependencies correctly,
+            // but sub-arrays at the un-flattened cut-off depth end up reactor-wrapped
+            // in the result because they were read from inner reactor proxies.
+            // Calling flat() on the raw source instead would avoid this, but it
+            // bypasses the proxy entirely so no dependencies are built.
+            // Instead we call on the proxy and then unwrap any reactor-wrapped arrays
+            // left in the result.
+            if (this.source === Array.prototype.flat && Array.isArray(result)) {
+              const unwrapReactorArrays = (el) => {
+                if (!Reactors.has(el)) return el
+                const source = reactorCoreExtractor.get(el).source
+                if (!Array.isArray(source)) return el
+                return source.map(unwrapReactorArrays)
+              }
+              return result.map(unwrapReactorArrays)
+            }
+            return result
           } catch (error) {
             if (error.name === 'TypeError' && error.message.includes('called on incompatible receiver #')) {
               const core = reactorCoreExtractor.get(thisArg)
@@ -270,7 +287,7 @@ class Reactor {
       get (property, receiver) {
         // Disable unnecessary wrapping for unmodifiable properties
         // Needed because Array prototype checking fails if wrapped
-        // Specificaly [].map()
+        // Specifically [].map()
         const descriptor = Object.getOwnPropertyDescriptor(
           this.source, property
         )
@@ -285,7 +302,7 @@ class Reactor {
           Object.prototype.hasOwnProperty.call(this.getSignals, property)
             ? this.getSignals[property]
             : new Signal()
-        // User accessor signals to give the actual output
+        // Use accessor signals to give the actual output
         // This enables automatic dependency tracking
         const signalCore = signalCoreExtractor.get(this.getSignals[property])
         signalCore.removeSelf = () => delete this.getSignals[property]
@@ -337,7 +354,7 @@ class Reactor {
       },
 
       // Have a map of dummy Signals to keep track of dependents on has
-      // We don't resuse the get Signals to avoid triggering getters
+      // We don't reuse the get Signals to avoid triggering getters
       hasSignals: {},
       has (property) {
         // Lazily instantiate has signals
@@ -348,7 +365,7 @@ class Reactor {
           Object.prototype.hasOwnProperty.call(this.hasSignals, property)
             ? this.hasSignals[property]
             : new Signal(null)
-        // User accessor signals to give the actual output
+        // Use accessor signals to give the actual output
         // This enables automatic dependency tracking
         const signalCore = signalCoreExtractor.get(this.hasSignals[property])
         signalCore.removeSelf = () => delete this.hasSignals[property]
@@ -498,13 +515,10 @@ class Observer extends Function {
       // Stored return value of the last successful execute
       // Stored in a Signal which makes it observable itself
       value: new Signal(),
-      // Flag on whether this is a unobserve block
-      // Avoids creating dependencies in that case
 
       // Symmetrically removes dependencies
       clearDependencies () {
         // Go upstream to break the connection
-        if (this.dependencies === null) return
         this.dependencies.forEach(dependency => {
           dependency.removeDependent(this)
         })
@@ -568,7 +582,7 @@ class Observer extends Function {
 
     }
 
-    // Public interace to hide the ugliness of how observers work
+    // Public interface to hide the ugliness of how observers work
     // An empty call force triggers the block and turns it on
     // A call with arguments gets those arguments passed as a context
     // for that and future retriggers
@@ -673,7 +687,7 @@ const batch = function (execute) {
     }
 
     return result
-  // No need to do anything if batching is already taking place }
+  // No need to do anything if batching is already taking place
   } else {
     return execute()
   }

test/batching.test.js

@@ -276,6 +276,11 @@ describe('Batching', () => {
     assert.strictEqual(runCount, 2)
   })
 
+  it('returns the return value of the batch function', () => {
+    const result = batch(() => 'foo')
+    assert.strictEqual(result, 'foo')
+  })
+
   it('throws an error if the batch function it is called with no arguments', () => {
     assert.throws(() => batch(), {
       name: 'Error',

test/hiding.test.js

@@ -1,7 +1,7 @@
 /* eslint-env mocha */
 import assert from 'assert'
 import {
-  // Signal,
+  Signal,
   Reactor,
   Observer,
   // Signals,
@@ -97,6 +97,12 @@ describe('Hiding', () => {
     assert.strictEqual(runValue, 'c')
   })
 
+  it('can read reactive values outside an observer without error', () => {
+    const signal = new Signal('foo')
+    const result = hide(() => signal())
+    assert.strictEqual(result, 'foo')
+  })
+
   it('throws an error if the hide function is not called with a function', () => {
     assert.throws(() => hide(), {
       name: 'Error',

test/observer.test.js

@@ -117,7 +117,7 @@ describe('Observer', () => {
           message: 'Cannot create observer with a non-function'
         })
       })
-      it('fails to initialize with an array', () => {
+      it('throws an error when initialized with an array', () => {
         assert.throws(() => new Observer([]), {
           name: 'TypeError',
           message: 'Cannot create observer with a non-function'
@@ -222,7 +222,7 @@ describe('Observer', () => {
   })
 
   describe('returns object values wrapped in a reactor', () => {
-    it('', () => {
+    it('returns object values wrapped in a reactor', () => {
       const object = {}
       const observer = new Observer(() => object)
       const result = observer()
@@ -232,7 +232,7 @@ describe('Observer', () => {
     })
   })
 
-  describe('exposes the last return value throught the value property', () => {
+  describe('exposes the last return value through the value property', () => {
     it('exposes the last derived value for primitive values', () => {
       let counter = 0
       const dummyFunction = () => (counter += 1)

test/reactivity.test.js

@@ -136,6 +136,21 @@ describe('Reactivity', () => {
       assert.deepEqual(runValue, ['foo', 'baz'])
     })
 
+    it('builds a dependency when using Object.keys and triggers on property deletion', () => {
+      let runCount = 0
+      let runValue
+      const reactor = new Reactor({ foo: 'bar', baz: 'qux' })
+      new Observer(() => {
+        runCount += 1
+        runValue = Object.keys(reactor)
+      })()
+      assert.strictEqual(runCount, 1)
+      assert.deepEqual(runValue, ['foo', 'baz'])
+      delete reactor.foo
+      assert.strictEqual(runCount, 2)
+      assert.deepEqual(runValue, ['baz'])
+    })
+
     it('builds a dependency when using the in operator', () => {
       let runCount = 0
       let runValue

test/reactor.test.js

@@ -196,14 +196,14 @@ describe('Reactor', () => {
       const object = { foo: 'bar' }
       const reactor = new Reactor(object)
       delete reactor.foo
-      assert.equal(object.foo, undefined)
+      assert.strictEqual(object.foo, undefined)
     })
 
     it('deletes properties from itself', () => {
       const object = { foo: 'bar' }
       const reactor = new Reactor(object)
       delete reactor.foo
-      assert.equal(reactor.foo, undefined)
+      assert.strictEqual(reactor.foo, undefined)
     })
 
     it('works with Object.defineProperty', () => {
@@ -334,28 +334,10 @@ describe('Reactor', () => {
       assert.deepStrictEqual(shuck(result), [5, 4, 3, 1, 1])
     })
 
-    it.skip('works with Array.prototype.flat()', () => {
+    it('works with Array.prototype.flat()', () => {
       const reactor = new Reactor([1, [2, 3], [4, [5, 6]]])
-      const result = reactor.flat()
-      // Interesting edge case here
-      // Flat returns a new array with the subarrays being reactor wrapped too
-      // But the subarrays are reactor wrapped even in the internal object
-      // This is because flat is constructing them from reactor reads
-      // Making the apply call use the internal object as `this` solves this
-      // but it breaks a bunch of other stuff
-      // TODO find a way to fix this
-      assert.deepStrictEqual(shuck(result), [1, 2, 3, 4, [5, 6]])
-    })
-
-    // it('test flat', () => {
-    //   const array = [1, [2, 3], [4, [5, 6]]]
-    //   const result = array.flat()
-    //   const original56 = array[2][1]
-    //   const result56 = result[4]
-    //   console.log(original56, result56)
-    //   assert(original56 === result56)
-    //   assert.strictEqual(array[2][1], result[4])
-    // })
+      assert.deepStrictEqual(reactor.flat(), [1, 2, 3, 4, [5, 6]])
+    })
 
     it('works with Array.prototype.flatMap()', () => {
       const reactor = new Reactor([1, 2, 3])