underive does not delete the derived property unless there is a dependency defined

[dlindahl]

Summary

Calling underive with delete: true does not delete the derived property value unless there is a dependency.

1. Calling derive subscribes to the dependencies defined within the derive function.
2. Calling underive with delete: true loops through all of the dependencies.
3. After removing each dependency subscription, the key of the subscription is added to a Set to delete later.
4. Once each subscription has been removed, the derived functions that matched a subscription key are deleted from the proxyObject.

The problem arises from a situation where the derived function never calls get (for whatever reason) which prevents a subscription from being created. The lack of a subscription prevents the property from being deleted which then prevents new calls to derive because the property is already defined.

Is this silly? Yes.
Was this hard to debug. Also yes. 😄

Property deletion should probably be decoupled from the existence of a subscription, especially since I see some async subscriptions being handled in the derive code which could possibly introduce a situation where underive is called before the async value resolves which would result in a similar error.

Link to reproduction

derive({
  myProperty(get) {
    return 123; // NOTE: Not using `get` which does not create a dependency.
  }
}, {
  proxy: myProxy
})

underive(myProxy, { delete: true, keys: ['myProperty'])
// Throws Error: object property already defined

Pardon the lack of a codesandbox repro as the problem seemed pretty straight forward. I can make one if you really need one though.

Check List

Please do not ask questions in issues.

• I've already opened a discussion before opening this issue, or already discussed in other media.

Please include a minimal reproduction.

• I've added a link to a typescript playground or codesandbox with a minimal reproduction.

[dai-shi]

Nice catch. Thanks for reporting.

Would you or anyone like to fix it? derive is already complicated, and hope to simplify the implementation.

[dai-shi]

Here's a new proposition, which might be controversial: #792

[dai-shi]

Transferred to: valtiojs/derive-valtio#1