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

fix: improve long running task performance in query core #8107

Merged
merged 12 commits into from
Oct 9, 2024
Merged
Prev Previous commit
Next Next commit
refactor: remove queryObserver improvement from PR
  • Loading branch information
davidaghassi committed Oct 3, 2024
commit 73b683f6c68339a60bf88d798c0565cda24b2efe
95 changes: 18 additions & 77 deletions packages/query-core/src/queryObserver.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
import { focusManager } from './focusManager'
import { notifyManager } from './notifyManager'
import { fetchState } from './query'
import { Subscribable } from './subscribable'
import { pendingThenable } from './thenable'
import {
isServer,
isValidTimeout,
Expand All @@ -13,9 +8,12 @@ import {
shallowEqualObjects,
timeUntilStale,
} from './utils'
import { notifyManager } from './notifyManager'
import { focusManager } from './focusManager'
import { Subscribable } from './subscribable'
import { fetchState } from './query'
import type { FetchOptions, Query, QueryState } from './query'
import type { QueryClient } from './queryClient'
import type { PendingThenable, Thenable } from './thenable'
import type {
DefaultError,
DefaultedQueryObserverOptions,
Expand Down Expand Up @@ -59,7 +57,6 @@ export class QueryObserver<
TQueryData,
TQueryKey
>
#currentThenable: Thenable<TData>
#selectError: TError | null
#selectFn?: (data: TQueryData) => TData
#selectResult?: TData
Expand All @@ -85,13 +82,6 @@ export class QueryObserver<

this.#client = client
this.#selectError = null
this.#currentThenable = pendingThenable()
if (!this.options.experimental_prefetchInRender) {
this.#currentThenable.reject(
new Error('experimental_prefetchInRender feature flag is not enabled'),
)
}

this.bindMethods()
this.setOptions(options)
}
Expand Down Expand Up @@ -273,21 +263,21 @@ export class QueryObserver<
result: QueryObserverResult<TData, TError>,
onPropTracked?: (key: keyof QueryObserverResult) => void,
): QueryObserverResult<TData, TError> {
const handler: ProxyHandler<QueryObserverResult<TData, TError>> = {
get: (target, key: keyof QueryObserverResult<TData, TError>) => {
if (key in target) {
this.trackProp(key)
if (onPropTracked) {
onPropTracked(key)
}
return target[key]
}
// Ensure all paths are covered by explicitly handling "undefined" properties
return undefined
},
}
const trackedResult = {} as QueryObserverResult<TData, TError>

Object.keys(result).forEach((key) => {
Object.defineProperty(trackedResult, key, {
configurable: false,
enumerable: true,
get: () => {
this.trackProp(key as keyof QueryObserverResult)
onPropTracked?.(key as keyof QueryObserverResult)
return result[key as keyof QueryObserverResult]
},
})
})

return new Proxy(result, handler)
return trackedResult
}

trackProp(key: keyof QueryObserverResult) {
Expand Down Expand Up @@ -592,7 +582,6 @@ export class QueryObserver<
isRefetchError: isError && hasData,
isStale: isStale(query, options),
refetch: this.refetch,
promise: this.#currentThenable,
}

return result as QueryObserverResult<TData, TError>
Expand All @@ -604,7 +593,6 @@ export class QueryObserver<
| undefined

const nextResult = this.createResult(this.#currentQuery, this.options)

this.#currentResultState = this.#currentQuery.state
this.#currentResultOptions = this.options

Expand All @@ -617,52 +605,6 @@ export class QueryObserver<
return
}

if (this.options.experimental_prefetchInRender) {
const finalizeThenableIfPossible = (thenable: PendingThenable<TData>) => {
if (nextResult.status === 'error') {
thenable.reject(nextResult.error)
} else if (nextResult.data !== undefined) {
thenable.resolve(nextResult.data)
}
}

/**
* Create a new thenable and result promise when the results have changed
*/
const recreateThenable = () => {
const pending =
(this.#currentThenable =
nextResult.promise =
pendingThenable())

finalizeThenableIfPossible(pending)
}

const prevThenable = this.#currentThenable
switch (prevThenable.status) {
case 'pending':
// Finalize the previous thenable if it was pending
finalizeThenableIfPossible(prevThenable)
break
case 'fulfilled':
if (
nextResult.status === 'error' ||
nextResult.data !== prevThenable.value
) {
recreateThenable()
}
break
case 'rejected':
if (
nextResult.status !== 'error' ||
nextResult.error !== prevThenable.reason
) {
recreateThenable()
}
break
}
}

this.#currentResult = nextResult

// Determine which callbacks to trigger
Expand Down Expand Up @@ -697,7 +639,6 @@ export class QueryObserver<
return Object.keys(this.#currentResult).some((key) => {
const typedKey = key as keyof QueryObserverResult
const changed = this.#currentResult[typedKey] !== prevResult[typedKey]

return changed && includedProps.has(typedKey)
})
}
Expand Down