Skip to content

Commit

Permalink
feat(solid-query): Rework internals of createBaseQuery (#7272)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ardeora committed Apr 13, 2024
1 parent a4ad0e1 commit a122e7c
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 61 deletions.
Expand Up @@ -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',
Expand All @@ -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',
Expand Down
3 changes: 0 additions & 3 deletions packages/solid-query/package.json
Expand Up @@ -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",
Expand Down
95 changes: 46 additions & 49 deletions packages/solid-query/src/createBaseQuery.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -144,9 +144,9 @@ export function createBaseQuery<
new Observer(client(), defaultedOptions()),
)

const [state, setState] = createStore<QueryObserverResult<TData, TError>>(
observer().getOptimisticResult(defaultedOptions()),
)
let observerResult = observer().getOptimisticResult(defaultedOptions())
const [state, setState] =
createStore<QueryObserverResult<TData, TError>>(observerResult)

const createServerSubscriber = (
resolve: (
Expand Down Expand Up @@ -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<T>(): Signal<T> {
return [
() => state,
(v: T) => {
const unwrapped = unwrap(state)
if (typeof v === 'function') {
v = v(unwrapped)
}
setStateWithReconciliation(v as any)
},
] as Signal<T>
}

/**
* 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<ResourceData | undefined>(
() => {
const obs = observer()
return new Promise((resolve, reject) => {
Expand All @@ -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
Expand Down Expand Up @@ -338,7 +329,7 @@ export function createBaseQuery<
[observer, defaultedOptions],
([obs, opts]) => {
obs.setOptions(opts)
setState(obs.getOptimisticResult(opts))
setStateWithReconciliation(obs.getOptimisticResult(opts))
},
{ defer: true },
),
Expand Down Expand Up @@ -369,8 +360,14 @@ export function createBaseQuery<
target: QueryObserverResult<TData, TError>,
prop: keyof QueryObserverResult<TData, TError>,
): 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)
},
}

Expand Down
3 changes: 0 additions & 3 deletions 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'

Expand Down
4 changes: 0 additions & 4 deletions packages/solid-query/src/setBatchUpdatesFn.ts

This file was deleted.

0 comments on commit a122e7c

Please sign in to comment.