From a122e7cca52eed17d223e491a3c470ce72904d9b Mon Sep 17 00:00:00 2001 From: Aryan Deora Date: Sat, 13 Apr 2024 02:13:07 -0400 Subject: [PATCH] feat(solid-query): Rework internals of createBaseQuery (#7272) * 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 --- .../PersistQueryClientProvider.test.tsx | 10 +- packages/solid-query/package.json | 3 - packages/solid-query/src/createBaseQuery.ts | 95 +++++++++---------- packages/solid-query/src/index.ts | 3 - packages/solid-query/src/setBatchUpdatesFn.ts | 4 - 5 files changed, 54 insertions(+), 61 deletions(-) delete mode 100644 packages/solid-query/src/setBatchUpdatesFn.ts diff --git a/packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx b/packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx index 8438a425cd..94fc511ae4 100644 --- a/packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx +++ b/packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx @@ -557,7 +557,7 @@ describe('PersistQueryClientProvider', () => { expect(queryFn2).toHaveBeenCalledTimes(1) expect(onSuccess).toHaveBeenCalledTimes(1) - expect(states).toHaveLength(3) + expect(states).toHaveLength(4) expect(states[0]).toMatchObject({ status: 'pending', @@ -566,12 +566,18 @@ describe('PersistQueryClientProvider', () => { }) expect(states[1]).toMatchObject({ + status: 'pending', + fetchStatus: 'idle', + data: undefined, + }) + + expect(states[2]).toMatchObject({ status: 'success', fetchStatus: 'fetching', data: 'hydrated', }) - expect(states[2]).toMatchObject({ + expect(states[3]).toMatchObject({ status: 'success', fetchStatus: 'idle', data: 'queryFn2', diff --git a/packages/solid-query/package.json b/packages/solid-query/package.json index d4ef8a6729..f07c7131b1 100644 --- a/packages/solid-query/package.json +++ b/packages/solid-query/package.json @@ -39,9 +39,6 @@ "default": "./build/index.cjs" } }, - "sideEffects": [ - "./src/setBatchUpdatesFn.ts" - ], "scripts": { "clean": "rimraf ./build && rimraf ./coverage", "test:eslint": "eslint --ext .ts,.tsx ./src", diff --git a/packages/solid-query/src/createBaseQuery.ts b/packages/solid-query/src/createBaseQuery.ts index 9621271a92..4f80daae03 100644 --- a/packages/solid-query/src/createBaseQuery.ts +++ b/packages/solid-query/src/createBaseQuery.ts @@ -16,7 +16,7 @@ import { useQueryClient } from './QueryClientProvider' import { shouldThrowError } from './utils' import { useIsRestoring } from './isRestoring' import type { CreateBaseQueryOptions } from './types' -import type { Accessor } from 'solid-js' +import type { Accessor, Signal } from 'solid-js' import type { QueryClient } from './QueryClient' import type { InfiniteQueryObserverResult, @@ -144,9 +144,9 @@ export function createBaseQuery< new Observer(client(), defaultedOptions()), ) - const [state, setState] = createStore>( - observer().getOptimisticResult(defaultedOptions()), - ) + let observerResult = observer().getOptimisticResult(defaultedOptions()) + const [state, setState] = + createStore>(observerResult) const createServerSubscriber = ( resolve: ( @@ -180,49 +180,43 @@ export function createBaseQuery< const createClientSubscriber = () => { const obs = observer() return obs.subscribe((result) => { - notifyManager.batchCalls(() => { - // @ts-expect-error - This will error because the reconcile option does not - // exist on the query-core QueryObserverResult type - const reconcileOptions = obs.options.reconcile + observerResult = result + queueMicrotask(() => refetch()) + }) + } - // If the query has data we don't suspend but instead mutate the resource - // This could happen when placeholderData/initialData is defined - if ( - !queryResource.error && - queryResource()?.data && - result.data && - !queryResource.loading - ) { - setState((store) => { - return reconcileFn( - store, - result, - reconcileOptions === undefined ? false : reconcileOptions, - ) - }) - mutate(state) - } else { - setState((store) => { - return reconcileFn( - store, - result, - reconcileOptions === undefined ? false : reconcileOptions, - ) - }) - refetch() - } - })() + function setStateWithReconciliation(res: typeof observerResult) { + // @ts-expect-error - Reconcile option is not correctly typed internally + const reconcileOptions = observer().options.reconcile + + setState((store) => { + return reconcileFn( + store, + res, + reconcileOptions === undefined ? false : reconcileOptions, + ) }) } + function createDeepSignal(): Signal { + return [ + () => state, + (v: T) => { + const unwrapped = unwrap(state) + if (typeof v === 'function') { + v = v(unwrapped) + } + setStateWithReconciliation(v as any) + }, + ] as Signal + } + /** * Unsubscribe is set lazily, so that we can subscribe after hydration when needed. */ let unsubscribe: (() => void) | null = null - const [queryResource, { refetch, mutate }] = createResource< - ResourceData | undefined - >( + const [queryResource, { refetch }] = createResource( () => { const obs = observer() return new Promise((resolve, reject) => { @@ -233,19 +227,16 @@ export function createBaseQuery< } obs.updateResult() - if (!state.isLoading) { + if (!observerResult.isLoading) { const query = obs.getCurrentQuery() - resolve(hydratableObserverResult(query, state)) + resolve(hydratableObserverResult(query, observerResult)) + } else { + setStateWithReconciliation(observerResult) } }) }, { - initialValue: state, - - // If initialData is provided, we resolve the resource immediately - get ssrLoadFrom() { - return options().initialData ? 'initial' : 'server' - }, + storage: createDeepSignal, get deferStream() { return options().deferStream @@ -338,7 +329,7 @@ export function createBaseQuery< [observer, defaultedOptions], ([obs, opts]) => { obs.setOptions(opts) - setState(obs.getOptimisticResult(opts)) + setStateWithReconciliation(obs.getOptimisticResult(opts)) }, { defer: true }, ), @@ -369,8 +360,14 @@ export function createBaseQuery< target: QueryObserverResult, prop: keyof QueryObserverResult, ): any { - const val = queryResource()?.[prop] - return val !== undefined ? val : Reflect.get(target, prop) + if (prop === 'data') { + const opts = observer().options + if (opts.placeholderData) { + return queryResource.latest?.data + } + return queryResource()?.data + } + return Reflect.get(target, prop) }, } diff --git a/packages/solid-query/src/index.ts b/packages/solid-query/src/index.ts index b6045df6db..9d894999bf 100644 --- a/packages/solid-query/src/index.ts +++ b/packages/solid-query/src/index.ts @@ -1,8 +1,5 @@ /* istanbul ignore file */ -// Side Effects -import './setBatchUpdatesFn' - // Re-export core export * from '@tanstack/query-core' diff --git a/packages/solid-query/src/setBatchUpdatesFn.ts b/packages/solid-query/src/setBatchUpdatesFn.ts deleted file mode 100644 index f53c357200..0000000000 --- a/packages/solid-query/src/setBatchUpdatesFn.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { notifyManager } from '@tanstack/query-core' -import { batch } from 'solid-js' - -notifyManager.setBatchNotifyFunction(batch)