Merge pull request #336 from knockout/fix/proxy-delete-property

Fix ko.proxy deleteProperty trap dropping the property key

Author: brianmhunt

.changeset/fix-proxy-delete-property.md

@@ -0,0 +1,18 @@
+---
+"@tko/computed": patch
+---
+
+Fix `ko.proxy` `deleteProperty` trap silently doing nothing
+
+The `deleteProperty` handler on proxies built by `ko.proxy` declared only one
+parameter, named `property`. Per the Proxy spec, the trap is invoked with
+`(target, property)` — so the handler was receiving `target` (the internal
+`function(){}`) in place of the property key, stringifying it, and attempting
+to delete that bogus key. The actual property remained on both the mirror
+observable store and the underlying object, and its tracked observable stayed
+alive.
+
+`delete proxied.foo` now correctly removes `foo` from both the proxy and the
+underlying object and returns `true`. Added a regression test to
+`proxyBehavior.ts` — the bug had been present since `ko.proxy` was introduced
+in 2017 and was untested.

packages/computed/spec/proxyBehavior.ts

@@ -106,4 +106,19 @@ describe('Proxy', function () {
     }
     expect(p.x2).to.equal(16)
   })
+
+  it('deletes properties from both the proxy and the underlying object', function () {
+    const x: { a; b? } = { a: 1, b: 2 }
+    const p = proxy(x)
+    expect('b' in p).to.equal(true)
+    expect(x.b).to.equal(2)
+
+    const result = delete p.b
+    expect(result).to.equal(true)
+    expect('b' in p).to.equal(false)
+    expect('b' in x).to.equal(false)
+    // Remaining properties are untouched.
+    expect(p.a).to.equal(1)
+    expect(x.a).to.equal(1)
+  })
 })

packages/computed/src/proxy.ts

@@ -54,7 +54,7 @@ export function proxy(object) {
       object[prop] = value
       return true
     },
-    deleteProperty(property) {
+    deleteProperty(_target, property) {
       delete mirror[property as any]
       return delete object[property as any]
     },