Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reconcile does nothing for top-level key change #1032

Closed
izreit opened this issue Jun 2, 2022 · 3 comments
Closed

reconcile does nothing for top-level key change #1032

izreit opened this issue Jun 2, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@izreit
Copy link

izreit commented Jun 2, 2022

Describe the bug

I found that the reconcile() changes nothing when

  • the key is given, and
  • the top-level key property is being modified.

For example, the following reconcile() does not change state:

const [state, setState] = createStore({ id: "id0", value: 0 });
setState(reconcile({ id: "id1", value: 42 }, { key: "id" }));
state.value // ==> 0

while the following which uses the same id works:

const [state, setState] = createStore({ id: "id0", value: 0 });
setState(reconcile({ id: "id0", value: 42 }, { key: "id" }));
state.value // ==> 42

It also works if the modified key is in an element of an array:

const [state, setState] = createStore([{ id: "id0", value: 0 }]);
setState(reconcile([{ id: "id1", value: 42 }], { key: "id" })); 
state.value // ==> 42

or in a nested object:

const [state, setState] = createStore({ id: "id0", nested: { id: "id100", value: 0 } });
setState(reconcile({ id: "id0", nested: { id: "id101", value: 42 } }, { key: "id" }));
state.nested.value // ==> 42

It seems there is no unit tests examining this behavior in modifiers.spec.ts.

Your Example Website or App

https://playground.solidjs.com/?version=1.4.1#NobwRAdghgtgpmAXGGUCWEwBowBcCeADgsrgM4Ae2YZA9gK4BOAxiWGjIbY7gAQi9GcCABM4jXgF9eAM0a0YvADo1aAGzQiAtACsyAegDucAEYqA3EogcuPfr2ZCouOAGVc3OFkFxmtCMxoanBSsvKKKnQa2nr6ZB5CFlZWMvQBuGj+vADCDBAujAAUAJT8VrwO-vG8wPHOXrxkcLju9QC6vAC8Dk4u7p6FIOUVvJqIymCEUEL5WpoADCpYwxXMABZBIjPjoKMi4yrrm3Mii9i8tCY64wIAblBq9HDj81KSbcOSxZYQw34Q1X82Q0zAA1l1eCUugA+RrNVouQorHz-QLBQbKCAjEZjCZTGa4E4ARiWyNWGzUW2EOwEuMOFO0CyWFyuN1490ez14RPmr0k72Rkm8AlBcHwB3YIhUUmKw2+ySxPlwTCxhQAPNDkWqTPRcB4sQRiJ0VDq9f5pUCQaDOiBLWgwZJoQAFehkNa8eBq-Sm-WaxUVNWEITQgQAKVcAHkAHIAOnijAwAHM0DJ8IU6i5vBB6Go1N4AEylSReoNwP0B-TQ+UQSQKmZiIpQzqwtW5NIFXiV7wiWjMejwfIxxPNACiwQHuAAQvgAJIiJFgKCEQgqYoAQmrYHeQA

Steps to Reproduce the Bug or Issue

  1. Go to the above playground
  2. Click the button "Push me"
  3. No changes on Result although setState(reconcile(...)) is called.

Expected behavior

Clicking the button modifies the dumped state in Result. For example the "value" property should be 100.

Screenshots or Videos

No response

Platform

  • OS: Windows 11
  • Browser: Edge
  • Version: 101.0.1210.53

Also reproduced on Firefox 100.0.2.

Additional context

I'm trying to use reconcile() for a tree structure like directory hierarchy of a file system. First it is initialized with a dummy empty root (because a store cannot be null) with a dummy id, and then updated with an actual value. So I need id on the top-level of a store.

@izreit izreit changed the title reconcile() does nothing for top-level key change reconcile does nothing for top-level key change Jun 2, 2022
@ryansolid
Copy link
Member

ryansolid commented Jun 2, 2022

Hmm.. that's an interesting one. Thanks for reporting. Technically with a different key it should "replace" but you can't replace top-level as its a proxy. The underlying object can't be replaced. Which means the only option is to merge. We could probably make that behavior even with its side effects because there isn't really anything else it could be doing.

I'd recommend putting the whole structure under a key for now though and not treat the top level as part of your data structure and call reconcile on a path. Ie.. give your key and then call reconcile as your second argument.

@ryansolid ryansolid added the bug Something isn't working label Jun 2, 2022
@izreit
Copy link
Author

izreit commented Jun 2, 2022

Thanks for the very rapid response!

call reconcile on a path. Ie.. give your key and then call reconcile as your second argument.

Uh... you mean a code like the following?

const [state, setState] = createStore({ dummyRoot: { id: "id0", value: 0 } });
setState("dummyRoot", reconcile({ id: "id1", value: 42 }, { key: "id" }));

Unfortunately it also changes nothing as far as I tested on Playground. (i.e. state.dummyRoot.value is still 0)

Instead I will use the following for now (by putting the whole data under a key, as you said):

const [state, setState] = createStore({ dummyRoot: { id: "id0", value: 0 } });
setState(reconcile({ dummyRoot: { id: "id1", value: 42 } }, { key: "id" }));

@ryansolid
Copy link
Member

Right it would be the same problem wouldn't it. It would have the chance to swap in that case mind you by returning the new item. Ok I will add that to the list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants