solid icon indicating copy to clipboard operation
solid copied to clipboard

createMutable - delete key with undefined value does not trigger reactivity

Open LiQuidProQuo opened this issue 3 years ago • 3 comments

Describe the bug

delete key with undefined value does not trigger reactivity

let mutable = createMutable({a:'somevalue', b:undefined})

// reactive
delete mutable.a

// non reactive
delete mutable.b

minimal patch direction ( mutable proxy handler)

- function setProperty(state, property, value) {
- if (state[property] === value)
+ function setProperty(state, property, value, deleted=false) {
+ if (state[property] === value && !deleted)


    deleteProperty(target, property) {
-      setProperty(target, property, void 0);
+      setProperty(target, property, void 0, true);
+

Your Example Website or App

https://playground.solidjs.com/?hash=-641634001&version=1.4.1

Steps to Reproduce the Bug or Issue

see playground

  1. click "delete B" expect 2nd item to be removed from the list , and effect to log

  2. click "delete A" expect the 1st item to be removed and effect to log

Expected behavior

it is expected that when deleting B, it will be removed from the list and trigger the effects

Screenshots or Videos

No response

Platform

playground

Additional context

No response

LiQuidProQuo avatar Jul 12 '22 20:07 LiQuidProQuo

The reason because it's not reactive is because the value does not change: a deleted property is simply undefined so the Proxy does not rerun its dependencies.

paoloricciuti avatar Jul 15 '22 07:07 paoloricciuti

there is a difference between a defined property with an undefined value. ({k:undefined}) and an object without that property.{}

Object.keys({k:undefined}).length //  1
Object.keys({}).length //  0

deleting a key with an undefined value should trigger reactivity like with any other value.

LiQuidProQuo avatar Jul 15 '22 08:07 LiQuidProQuo

Yeah but your reactive object is not Object.keys (which effectively changes)...with createMutable you are creating a Proxy in which every field is a reactive variable. Each field will trigger its dependencies when the value change.

So the check Solid is actually doing is

Object.is(target[field], newValue);

and given that both values are undefined it will not trigger the rerender.

What we could do is tap into the deleteProperty trap of the Proxy to rerun the effects but this could lead to double running the effects when the value is not undefined. I'll take a look into this.

paoloricciuti avatar Jul 15 '22 08:07 paoloricciuti

fixed in 1.5

ryansolid avatar Aug 26 '22 08:08 ryansolid