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

Solid Query: reconcile and placeholderData don't work together #7173

Closed
PeterDraex opened this issue Mar 25, 2024 · 7 comments · Fixed by #7277
Closed

Solid Query: reconcile and placeholderData don't work together #7173

PeterDraex opened this issue Mar 25, 2024 · 7 comments · Fixed by #7277

Comments

@PeterDraex
Copy link

PeterDraex commented Mar 25, 2024

Describe the bug

When using placeholderData: keepPreviousData and reconcile together, the previous data is not kept. Instead, a result of another query is used as placeholder data.

Your minimal, reproducible example

https://stackblitz.com/edit/tanstack-query-bvyrzi?file=src%2FCounter.tsx

Steps to reproduce

  1. Reload the page, wait until loading finishes
  2. Click "Load 2 items", wait until data has 2 items
  3. Click "Load 0 items", wait until data has 0 items
  4. Click "Load 3 items".
  5. See that "data has 2 items" is displayed as placeholder data while the new data is loading.

Expected behavior

The data has 3 items is displayed immediately, as this query is cached.
If new data should be loaded, the previous result of data has 0 items should be visible while loading.

Tanstack Query adapter

solid-query 5.28.6

@PeterDraex
Copy link
Author

@TkDodo @ardeora Could you have a look at this issue pls? It's causing bugs in multiple places in my app

@ardeora
Copy link
Contributor

ardeora commented Apr 6, 2024

Apologies for the delay here. I'll be taking a look at this again this weekend. The reason we made reconcile off by default is because of this exact reason. The immutable nature of the server resources make it very tricky to implement correct reconciliation. The Keyed solid primitive (docs here) is a great alternative to solve this by taking the reconciliation outside of solid-query.

I have been thinking about it this week, and will possibly have an internal solution to this soon which would not need the Key workaround, but I need to test it out to see if this is feasible.

ardeora added a commit that referenced this issue Apr 13, 2024
* feat(solid-query): Rework internals of createBaseQuery

This change aims to simplify and enhance the internals of `createBaseQuery`.
This change is a precursor to fix a couple of pressing issues in solid-query mentioned in
#7079 
#7083 
#7173 
#7036
#6620 
#6373
PRs for which are coming soon.

A key few highlights coming with this change:
1. Removal of `mutate` and `refetch` branches in `createClientSubscriber`: We had two different ways of resolving a query resource. The `mutate` option that optimistically updated a resource proved to be a problem with error queries. A query that has `placeholder` data and fails for any reason would see a flash of blank content before yielding to the ErrorBoundary. Using `refetch` for all query resolutions gives us a nice and simple way to transition to error boundaries
2. Removal of batched calls with `notifyManager`: With the new approach we dont need to batch anything. Everything is taken care of automatically. This removes all sideEffects from solid-query as a nice plus
3. Central place to update state: The new `setStateWithReconciliation` function provides a nice and easy way to make the `reconcile` option way more powerful
4. Only accessing the `data` property now would trigger Suspense boundary. Previously any property accessed on a query would trigger the suspense boundary. This was not intentional and has been fixed now
5. Proxied `data` doesn't jump between different values in most cases
@PeterDraex
Copy link
Author

PeterDraex commented Apr 18, 2024

@ardeora I'm still having trouble with using reconcile and placeholderData together. Am I doing something wrong here?
https://stackblitz.com/edit/tanstack-query-enmaew?file=src%2FSearch.tsx

If you type something in the input, I think it should appear as current query result, but it doesn't.

@ardeora
Copy link
Contributor

ardeora commented Apr 18, 2024

@PeterDraex
You dont need reconcile here https://stackblitz.com/edit/tanstack-query-q5juyf?file=src%2FSearch.tsx

In React, when rendering a list of JSX elements, each element must have a unique key prop. This key helps React identify which items have changed, are added, or are removed, thus optimizing the rendering process.

In contrast, SolidJS handles this differently. Instead of reconciling JSX elements directly via a key, SolidJS uses the For component, which requires a reconcile function to manage the list based on a specific key property, typically id. This approach ensures that the items in the list maintain the same reference, allowing SolidJS to efficiently update the DOM when the data changes.

In the example above there is no key to reconcile here

@PeterDraex
Copy link
Author

PeterDraex commented Apr 18, 2024

@ardeora I thought that if I call reconcile with no key, it'll still prevent unnecessary updates to DOM. If I load 100 objects without ids and one gets changed, without reconcile all 100 are rerendered, but with reconcile, same value writes are blocked, and only the changed properties of the changed object are rerendered. Is that not true?

Also, I had configured reconcile: 'ID' as the default value. Does that mean that I have to set reconcile: false every time the returned object has no ID?

@ardeora
Copy link
Contributor

ardeora commented Apr 18, 2024

@PeterDraex
It would be very difficult for reconcile to figure which value in the array or object changed if there was no data key

Say if there is an array like so

array = [
  {name: 'John', status: 'inactive'},
  {name: 'David', status: 'deactivated'},
  {name: 'Dominik', status: 'active'},
  {name: 'Peter', status: 'active'},
] 

Now you refetch the data and the data looks like so

array = [
  {name: 'John', status: 'inactive'},
  {name: 'David', status: 'deactivated'},
  {name: 'Dominik', status: 'active'},
  {name: 'Aryan', status: 'active'},
  {name: 'Peter', status: 'active'},
] 

What changed here?
Was Peter's name changed to Aryan?
Was Dominik's name changed to Aryan and there was a new object with name 'Dominik' inserted above Aryan?

If we use the name property as the reconcile key with reconcile: 'name' we now know exactly where the change occurred and can make efficient DOM updates.

I would suggest not setting a global reconcile option for your app.

If you see this example in Astro https://github.com/TanStack/query/blob/main/examples/solid/astro/src/components/SolidApp.tsx#L75-L112

You can see that I set two different reconcile keys for the same query! And this makes me keep the same DOM nodes in place while switching between pokemons

Instead you should set a reconcile id on a per query basis when required.

@PeterDraex
Copy link
Author

@ardeora Ok, thanks!

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.

2 participants