Fixed #117 returned proxies not being finalized when only a subset wa…

…s returned

__tests__/base.js

@@ -858,6 +858,53 @@ function runBaseTest(name, useProxies, freeze) {
             expect(nextState).toEqual([1, 2, 100])
         })
 
+        it("should fix #117", () => {
+            const reducer = (state, action) =>
+                produce(state, draft => {
+                    switch (action.type) {
+                        case "SET_STARTING_DOTS":
+                            return draft.availableStartingDots.map(a => a)
+                        default:
+                            break
+                    }
+                })
+            const base = {
+                availableStartingDots: [
+                    {dots: 4, count: 1},
+                    {dots: 3, count: 2},
+                    {dots: 2, count: 3},
+                    {dots: 1, count: 4}
+                ]
+            }
+            const next = reducer(base, {type: "SET_STARTING_DOTS"})
+            expect(next).toEqual(base.availableStartingDots)
+            expect(next).not.toBe(base.availableStartingDots)
+        })
+
+        it("should fix #117 - 2", () => {
+            const reducer = (state, action) =>
+                produce(state, draft => {
+                    switch (action.type) {
+                        case "SET_STARTING_DOTS":
+                            return {
+                                dots: draft.availableStartingDots.map(a => a)
+                            }
+                        default:
+                            break
+                    }
+                })
+            const base = {
+                availableStartingDots: [
+                    {dots: 4, count: 1},
+                    {dots: 3, count: 2},
+                    {dots: 2, count: 3},
+                    {dots: 1, count: 4}
+                ]
+            }
+            const next = reducer(base, {type: "SET_STARTING_DOTS"})
+            expect(next).toEqual({dots: base.availableStartingDots})
+        })
+
         afterEach(() => {
             expect(baseState).toBe(origBaseState)
             expect(baseState).toEqual(createBaseState())

changelog.md

@@ -2,6 +2,7 @@
 
 # Next
 
+* Fixed [#117](https://github.com/mweststrate/immer/issues/117), proxy was not finalized when returning a subset of the state
 * Fixed [#116](https://github.com/mweststrate/immer/issues/116), changes to arrays that ended up with the same length were not properly detected.
 
 ### 1.1.1

src/es5.js

@@ -134,7 +134,10 @@ function hasArrayChanges(state) {
     // If after that new items are added, result in the same original length,
     // those last items will have no intercepting property.
     // So if there is no own descriptor on the last position, we know that items were removed and added
+    // N.B.: splice, unshift, etc only shift values around, but not prop descriptors, so we only have to check
+    // the last one
     const descriptor = Object.getOwnPropertyDescriptor(proxy, proxy.length - 1)
+    // descriptor can be null, but only for newly created sparse arrays, eg. new Array(10)
     if (descriptor && !descriptor.get) return true
     // For all other cases, we don't have to compare, as they would have been picked up by the index setters
     return false
@@ -155,14 +158,14 @@ export function produceEs5(baseState, producer) {
         // find and mark all changes (for parts not done yet)
         // TODO: store states by depth, to be able guarantee processing leaves first
         markChanges()
-        let result = finalize(rootProxy)
+        let result
         // check whether the draft was modified and/or a value was returned
         if (returnValue !== undefined && returnValue !== rootProxy) {
             // something was returned, and it wasn't the proxy itself
             if (rootProxy[PROXY_STATE].modified)
                 throw new Error(RETURNED_AND_MODIFIED_ERROR)
-            result = returnValue
-        }
+            result = finalize(returnValue)
+        } else result = finalize(rootProxy)
         // make sure all proxies become unusable
         each(states, (_, state) => {
             state.finished = true

src/proxy.js

@@ -137,13 +137,19 @@ export function produceProxy(baseState, producer) {
         // execute the thunk
         const returnValue = producer.call(rootProxy, rootProxy)
         // and finalize the modified proxy
-        let result = finalize(rootProxy)
+        let result
         // check whether the draft was modified and/or a value was returned
         if (returnValue !== undefined && returnValue !== rootProxy) {
             // something was returned, and it wasn't the proxy itself
             if (rootProxy[PROXY_STATE].modified)
                 throw new Error(RETURNED_AND_MODIFIED_ERROR)
-            result = returnValue
+
+            // See #117
+            // Should we just throw when returning a proxy which is not the root, but a subset of the original state?
+            // Looks like a wrongly modeled reducer
+            result = finalize(returnValue)
+        } else {
+            result = finalize(rootProxy)
         }
         // revoke all proxies
         each(proxies, (_, p) => p.revoke())