-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
fix(solid-query): hydrate preloaded data correctly #5775
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
Changes from all commits
4a1b93d
86a60b0
b41b0d8
347f6f2
5aae9ad
d79aef9
b57acbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { useQueryClient } from '@tanstack/solid-query' | ||
| import { isServer } from 'solid-js/web' | ||
| import { Title } from 'solid-start' | ||
| import { UserInfo, userInfoQueryOpts } from '~/components/user-info' | ||
|
|
||
| export default function Prefetch() { | ||
| const queryClient = useQueryClient() | ||
|
|
||
| if (isServer) { | ||
| void queryClient.prefetchQuery(userInfoQueryOpts({ sleep: 500 })) | ||
| } | ||
|
|
||
| return ( | ||
| <main> | ||
| <Title>Solid Query - Prefetch</Title> | ||
|
|
||
| <h1>Solid Query - Prefetch Example</h1> | ||
|
|
||
| <div class="description"> | ||
| <p> | ||
| In some cases you may want to prefetch a query on the server before | ||
| the component with the relevant `createQuery` call is mounted. A major | ||
| use case for this is in router data loaders, in order to avoid request | ||
| waterfalls. | ||
| </p> | ||
| <p> | ||
| In this example we prefetch the user query (on the server only). There | ||
| should be no extra `fetchUser.start` and `fetchUser.done` logs in the | ||
| console on the client when refreshing the page. | ||
| </p> | ||
| </div> | ||
|
|
||
| <UserInfo sleep={500} deferStream /> | ||
| </main> | ||
| ) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,9 +18,11 @@ import type { CreateBaseQueryOptions } from './types' | |
| import type { Accessor } from 'solid-js' | ||
| import type { QueryClient } from './QueryClient' | ||
| import type { | ||
| Query, | ||
| QueryKey, | ||
| QueryObserver, | ||
| QueryObserverResult, | ||
| QueryState, | ||
| } from '@tanstack/query-core' | ||
|
|
||
| function reconcileFn<TData, TError>( | ||
|
|
@@ -40,6 +42,49 @@ function reconcileFn<TData, TError>( | |
| return { ...result, data: newData } as typeof result | ||
| } | ||
|
|
||
| type HydrateableQueryState<TData, TError> = QueryObserverResult<TData, TError> & | ||
| QueryState<TData, TError> | ||
|
|
||
| /** | ||
| * Solid's `onHydrated` functionality will silently "fail" (hydrate with an empty object) | ||
| * if the resource data is not serializable. | ||
| */ | ||
| const hydrateableObserverResult = < | ||
ardeora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| TQueryFnData, | ||
| TError, | ||
| TData, | ||
| TQueryKey extends QueryKey, | ||
| T2, | ||
| >( | ||
| query: Query<TQueryFnData, TError, TData, TQueryKey>, | ||
| result: QueryObserverResult<T2, TError>, | ||
| ): HydrateableQueryState<T2, TError> => { | ||
| // Including the extra properties is only relevant on the server | ||
| if (!isServer) return result as HydrateableQueryState<T2, TError> | ||
|
|
||
| return { | ||
| ...unwrap(result), | ||
|
|
||
| // cast to refetch function should be safe, since we only remove it on the server, | ||
| // and refetch is not relevant on the server | ||
| refetch: undefined as unknown as HydrateableQueryState< | ||
| T2, | ||
| TError | ||
| >['refetch'], | ||
|
|
||
| // hydrate() expects a QueryState object, which is similar but not | ||
| // quite the same as a QueryObserverResult object. Thus, for now, we're | ||
| // copying over the missing properties from state in order to support hydration | ||
| dataUpdateCount: query.state.dataUpdateCount, | ||
| fetchFailureCount: query.state.fetchFailureCount, | ||
| isInvalidated: query.state.isInvalidated, | ||
|
|
||
| // Unsetting these properties on the server since they might not be serializable | ||
| fetchFailureReason: null, | ||
| fetchMeta: null, | ||
| } | ||
| } | ||
|
|
||
| // Base Query Function that is used to create the query. | ||
| export function createBaseQuery< | ||
| TQueryFnData, | ||
|
|
@@ -54,6 +99,10 @@ export function createBaseQuery< | |
| Observer: typeof QueryObserver, | ||
| queryClient?: Accessor<QueryClient>, | ||
| ) { | ||
| type ResourceData = | ||
| | HydrateableQueryState<TData, TError> | ||
| | QueryObserverResult<TData, TError> | ||
|
|
||
| const client = createMemo(() => useQueryClient(queryClient?.())) | ||
|
|
||
| const defaultedOptions = client().defaultQueryOptions(options()) | ||
|
|
@@ -71,41 +120,19 @@ export function createBaseQuery< | |
|
|
||
| const createServerSubscriber = ( | ||
| resolve: ( | ||
| data: | ||
| | QueryObserverResult<TData, TError> | ||
| | PromiseLike<QueryObserverResult<TData, TError> | undefined> | ||
| | undefined, | ||
| data: ResourceData | PromiseLike<ResourceData | undefined> | undefined, | ||
| ) => void, | ||
| reject: (reason?: any) => void, | ||
| ) => { | ||
| return observer.subscribe((result) => { | ||
| notifyManager.batchCalls(() => { | ||
| const query = observer.getCurrentQuery() | ||
| const { refetch, ...rest } = unwrap(result) | ||
| const unwrappedResult = { | ||
| ...rest, | ||
|
|
||
| // hydrate() expects a QueryState object, which is similar but not | ||
| // quite the same as a QueryObserverResult object. Thus, for now, we're | ||
| // copying over the missing properties from state in order to support hydration | ||
| dataUpdateCount: query.state.dataUpdateCount, | ||
| fetchFailureCount: query.state.fetchFailureCount, | ||
| // Removing these properties since they might not be serializable | ||
| // fetchFailureReason: query.state.fetchFailureReason, | ||
| // fetchMeta: query.state.fetchMeta, | ||
| isInvalidated: query.state.isInvalidated, | ||
| } | ||
| const unwrappedResult = hydrateableObserverResult(query, result) | ||
|
|
||
| if (unwrappedResult.isError) { | ||
| if (process.env['NODE_ENV'] === 'development') { | ||
| console.error(unwrappedResult.error) | ||
| } | ||
|
Comment on lines
-100
to
-102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wasn't sure if this log is supposed to be here or an accidental leftover - lmk if it should stay and I can add it back. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We added a log in there because of this discussion https://discord.com/channels/719702312431386674/1072140965910949929 Please let me know if you have a better way to resolve/fix this 😅 |
||
| reject(unwrappedResult.error) | ||
| } | ||
| if (unwrappedResult.isSuccess) { | ||
| // Use of any here is fine | ||
| // We cannot include refetch since it is not serializable | ||
| resolve(unwrappedResult as any) | ||
| } else { | ||
| resolve(unwrappedResult) | ||
|
Comment on lines
-105
to
+135
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO we should always fall back to calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we dont call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well it's still calling |
||
| } | ||
| })() | ||
| }) | ||
|
|
@@ -148,7 +175,7 @@ export function createBaseQuery< | |
| let unsubscribe: (() => void) | null = null | ||
|
|
||
| const [queryResource, { refetch, mutate }] = createResource< | ||
| QueryObserverResult<TData, TError> | undefined | ||
| ResourceData | undefined | ||
| >( | ||
| () => { | ||
| return new Promise((resolve, reject) => { | ||
|
|
@@ -159,8 +186,10 @@ export function createBaseQuery< | |
| unsubscribe = createClientSubscriber() | ||
| } | ||
| } | ||
|
|
||
| if (!state.isLoading) { | ||
| resolve(state) | ||
| const query = observer.getCurrentQuery() | ||
| resolve(hydrateableObserverResult(query, state)) | ||
|
Comment on lines
-163
to
+192
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is the code path that wasn't covered - this is triggered if the data is already pre-loaded prior to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Awesome! This is looking great! Thanks for working on this :D |
||
| } | ||
| }) | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vitest --watchis a thing,vitest run --watchisn't - they have a dedicatedwatchcommand instead.