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 fails to replace nested array with object #2017

Closed
Bubz43 opened this issue Jan 6, 2024 · 2 comments · Fixed by #2020
Closed

reconcile fails to replace nested array with object #2017

Bubz43 opened this issue Jan 6, 2024 · 2 comments · Fixed by #2020

Comments

@Bubz43
Copy link

Bubz43 commented Jan 6, 2024

Describe the bug

Basically the reverse of this issue. Using reconcile to replace a nested value that is an array with an object still leaves it as an array.

Your Example Website or App

https://playground.solidjs.com/anonymous/d43decad-3911-4beb-99e0-6a166b6655c6

Steps to Reproduce the Bug or Issue

  const [store, setStore] = createStore<{ value: [] | {} }>({ value: [] });
  setStore(reconcile({ value: {} }));
  expect(Array.isArray(store.value)).toBe(false);

Expected behavior

Expect the array to be replaced with an object, works without reconcile.

Screenshots or Videos

No response

Platform

  • OS: Linux
  • Browser: Vivaldi
  • Version: 6.5

Additional context

I don't think the playground version of solid has been updated to use the latest version so obj -> array bug exists there, but this bug exists both on older and latest (1.8.8) version.

@takeramagan
Copy link
Contributor

takeramagan commented Jan 7, 2024

I have an example here:

const state = {
  value: []
}
Object.assign(state.value, { a: 'b'})
console.log(Array.isArray(state.value)); // true

const state2 = {
  value: {}
}

Object.assign(state.value, [2,3])
console.log(Array.isArray(state2.value)); // false

We are doing something similar in reconcile, so this bug happens

@ryansolid
Copy link
Member

Hmm.. ok. I need to think about this one a moment because this one is innately trickier. I think you are right that reconcile should update. What comes to mind is that like:

setStore("value", {0: something });

Shouldn't turn it into an object if value was an array. But this will:

setStore({value: {0: something }});

But yeah reconcile doesn't care about any of that. It should just swap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants