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 modifies query data on refetch before suspense triggers #6620

Closed
NathanHuisman opened this issue Dec 30, 2023 · 1 comment
Closed
Assignees

Comments

@NathanHuisman
Copy link
Contributor

NathanHuisman commented Dec 30, 2023

Describe the bug

After #6600 I did some more investigating on the solid-query layout shift problem and found it's a synchronization issue with suspense and when the data is actually fetched

The problem is that the is updated first (data becomes undefined), and the suspense boundary a setTImeout later (when using the default batch scheduling behavior). So there is a brief moment (<= one frame/js task) where there is no data but the suspense hasn't triggered yet. This can lead to a layout shift or other unexpected behavior.

Note that this doesn't happen with queueMicrotask/requestAnimationFrame. I presume this is because the event that causes suspense to trigger is batched. This means that in this case, suspense will update before the next frame.

Your minimal, reproducible example

https://codesandbox.io/p/devbox/tanstack-query-example-solid-basic-typescript-forked-vm69np

Steps to reproduce

  1. Open the codesandbox
  2. Press the button a couple times (make sure scheduler is setTimeout)
  3. Observe layout glitch (it doesn't happen 100% of the time)

Expected behavior

The data should be updated after/with suspense so that suspense fallback works as expected

How often does this bug happen?

Often

Screenshots or Videos

No response

Platform

  • Firefox
  • Chrome

Tanstack Query adapter

solid-query

TanStack Query version

5.15.5

TypeScript version

No response

Additional context

No response

@ardeora ardeora self-assigned this Mar 4, 2024
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
@ardeora
Copy link
Contributor

ardeora commented Apr 13, 2024

Hello! Thanks for the very descriptive reproduction! This issue has been fixed in 5.30.0

https://codesandbox.io/p/devbox/tanstack-query-example-solid-basic-typescript-forked-f4dycd?workspaceId=0d8b5620-4f50-49f2-a5b1-e4173816b150

You wont see any layout shifts with setTimeout too!

@ardeora ardeora closed this as completed Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants