solid icon indicating copy to clipboard operation
solid copied to clipboard

When updating store, setting a previous non-existent property with value undefined does not create the property, "in" operator does not find it.

Open IKoshelev opened this issue 2 years ago • 1 comments

Describe the bug

When you try to create a new property on an object during store update, and you set it to value undefined - a property descriptor is not added to the object, the in operator does not see it. The contradicts expected JS behavior:

import { createStore, produce } from "solid-js/store";

let normalObj = {} as any;
normalObj.foo = undefined;

// returns true
console.log("foo" in normalObj);
console.log(Object.getOwnPropertyDescriptors(normalObj));

let [val, setVal] = createStore({} as any);

setVal(
  produce((x: any) => {
    x.foo = undefined;
    // returns false
    console.log("foo" in x);
    console.log(Object.getOwnPropertyDescriptors(x));
  })
);

Playground with above code: https://playground.solidjs.com/anonymous/d71e1157-190b-4eca-94d1-cfab4e70c3c1

Your Example Website or App

https://playground.solidjs.com/anonymous/d71e1157-190b-4eca-94d1-cfab4e70c3c1

Steps to Reproduce the Bug or Issue

  1. Go to playground: https://playground.solidjs.com/anonymous/d71e1157-190b-4eca-94d1-cfab4e70c3c1
  2. Observe the console.

Expected behavior

A property descriptor should be created when a new property is assigned, even if assigned value is undefined.

Screenshots or Videos

No response

Platform

  • OS: Windows 11
  • Browser: Microsoft Edge
  • Version: 108.0.1462.54 (Official build) (64-bit)

Additional context

No response

IKoshelev avatar Jan 08 '23 19:01 IKoshelev

Yeah this is an extension of the original design. Related to issues brought up here: https://github.com/solidjs/solid/issues/1392

Stores were originally created with the path-based syntax in mind. Mutation based APIs came later but the path APIs had used undefined for deletion so there is this quirk. I tried a couple of things here to be able to flag supporting both but it always ended up with one scenario breaking existing behavior without needing to do extensive changes and checks throughout.

This will be addressed in Solid 2.0, but whether it is beforehand we will have to see. For now, I suggest using null for things that are present but have no value.

ryansolid avatar Jan 08 '23 20:01 ryansolid