Repository: tannerlinsley/react-query
Pull request #1578 (2021-01-05 16:16:26)

Feature/use tracked query
I made a first POC of what we discussed here: #1554

I tried it out in the simple example and it worked fine - just looks too easy to be true. Am I missing something obvious? Apart from:

- [x] tests (no idea how to test the proxy tracking, ideas welcome
- [x] documentation

just wanted to ask upfront if a new hook is wanted / the way to go before I put any more effort into this :)

vercel[bot]: This pull request is being automatically deployed with Vercel ([learn more](https://vercel.link/github-learn-more)).  
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: [https://vercel.com/tannerlinsley/react-query/hlu0p1rzx](https://vercel.com/tannerlinsley/react-query/hlu0p1rzx)  
✅ Preview: *In Progress*


boschni: Nice! Couple of thoughts:
- Think we don't necessarily need a separate hook for this, maybe a query option to disable/enable this behavior like `notifyOnChangeTracked: false`.
- The proxy could be replaced with `Object.defineProperty`? In the descriptor you can define a `get` function which can be used to track if a property is accessed.
TkDodo: > Think we don't necessarily need a separate hook for this, maybe a query option to disable/enable this behavior like notifyOnChangeTracked: false.

yeah, I’ve been thinking that a separate hook might be suboptimal, as you can’t combine it easily with `useInfiniteQuery`. The option might make more sense, and you can always set it globally to enable it everywhere. Let me see how that looks :)

> The proxy could be replaced with Object.defineProperty?

Hmm i honestly don’t know. I thought proxies are the idiomatic way to do “interceptors”. Do you know of any example code with `defineProperty` I could take a look at?
dai-shi: I suppose you can use `defineProperty` to define object getters. It should work because there are only predefined set of properties.

For example:
```js
const target = { isFetching: true, data: {} }
const trackingObject = {
  get isFething() {
    addNotifyOnChangeProps('isFetching')
    return target.isFetching
  },
  get data() {
    addNotifyOnChangeProps('data')
    return target.data
  },
}
```

`defineProperty` is a way do this dynamically.

boschni: Example with `Object.defineProperty`:
```js
const target = { isFetching: true, data: {} }
const result = {}

Object.keys(target).forEach(key => {
  Object.defineProperty(result, key, {
    configurable: false,
    enumerable: true,
    get: () => {
      addNotifyOnChangeProps(key)
      return target[key]
    }
  });
});
```
TkDodo: @boschni I changed the implementation from a new hook to a new option, and it also uses `Object.defineProperty` - works like before! Have a look please: 761a1a76
TkDodo: I did some documentation, is that enough or do we want:
- a separate docs page?
- an example?
tannerlinsley: Where are we sitting with this awesome feature?
TkDodo: > Where are we sitting with this awesome feature?

basically this discussion on api design needs to be decided: https://github.com/tannerlinsley/react-query/pull/1578#discussion_r554466089

- to we want to turn tracking on per default?
- how should it interact with other `notifyOnChange*` props.

what would be your opinion / preference?

once this is decided I can add more tests for that interaction and then we are good to go I believe :)
tannerlinsley: I think it would be awesome to have it enabled by default, as long as it’s backwards compatible for existing tests and use cases.  Curious, does it still work with destructuring?
TkDodo: > as long as it’s backwards compatible 

I have found one edge case where this might lead to stale data: You need to access a field _during render_ to subscribe to it. Accessing it in an effect only is also fine, but if you subscribe to one field, but access a different field during a callback only, you might not see the latest data. Example:

```
  const queryInfo = useQuery("repoData", () =>
    fetch(
      "https://api.github.com/repos/tannerlinsley/react-query"
    ).then((res) => res.json())
  );

return (
    <div>
      {queryInfo.data?.name}
      <button
        onClick={() => {
          alert(queryInfo.dataUpdatedAt);
        }}
      >
        When did I update?
      </button>
    </div>
  );
```

Here, we only subscribe to `data`, so the component will only re-render when `data` changes. If we re-focus the window and get the same data from the request, we won't re-render due to structural sharing. Awesome. But internally, `dataUpdatedAt` was updated. If I now click the button, I will see the old `dataUpdatedAt`, because I didn't access that property during render, so I'm not subscribed to it. Hope that makes sense.

> Curious, does it still work with destructuring?

yes, it does. destructuring does count as property access. And actually, destructuring would solve the above problem, because you would subscribe to both:

```
const { data, dataUpdatedAt } = useQuery(...)
```

---

Regarding API design: I don't think we should do an extra flag, but rather combine it with `notifyOnChangeProps`, see my proposal above.

- `notifyOnChangeProps`: undefined --> re-render always
- `notifyOnChangeProps: ['data', 'error']` --> re-render if data or error changes
- `notifyOnChangeProps: 'tracked'` --> re-render if a tracked property changes 

Because of the edge case (and maybe there I more I haven't considered), I would maybe keep defaulting it to undefined and let people opt-in (easy with a default config), and maybe we can set it to `'tracked'` per default in the next major version. How does that sound?
TkDodo: @boschni could you have another look please, i think i addressed all points 🙂
boschni: > @boschni could you have another look please, i think i addressed all points 🙂

lgtm 👍 
TkDodo: > > @boschni could you have another look please, i think i addressed all points 🙂
> 
> lgtm 👍

cool! @tannerlinsley if you're also happy I think we could merge this. Would you want more docs or examples?

Patch:
diff --git a/docs/src/pages/comparison.md b/docs/src/pages/comparison.md
index 34ca529ca..ce8c08c19 100644
--- a/docs/src/pages/comparison.md
+++ b/docs/src/pages/comparison.md
@@ -63,7 +63,7 @@ Feature/Capability Key:
 
 > **<sup>1</sup> Lagged Query Data** - React Query provides a way to continue to see an existing query's data while the next query loads (similar to the same UX that suspense will soon provide natively). This is extremely important when writing pagination UIs or infinite loading UIs where you do not want to show a hard loading state whenever a new query is requested. Other libraries do not have this capability and render a hard loading state for the new query (unless it has been prefetched), while the new query loads.
 
-> **<sup>2</sup> Render Optimization** - React Query has excellent rendering performance. It will only re-render your components when a query is updated. For example because it has new data, or to indicate it is fetching. React Query also batches updates together to make sure your application only re-renders once when multiple components are using the same query. If you are only interested in the `data` or `error` properties, you can reduce the number of renders even more by setting `notifyOnChangeProps` to `['data', 'error']`.
+> **<sup>2</sup> Render Optimization** - React Query has excellent rendering performance. It will only re-render your components when a query is updated. For example because it has new data, or to indicate it is fetching. React Query also batches updates together to make sure your application only re-renders once when multiple components are using the same query. If you are only interested in the `data` or `error` properties, you can reduce the number of renders even more by setting `notifyOnChangeProps` to `['data', 'error']`. Set `notifyOnChangeProps: 'tracked'` to automatically track which fields are accessed and only re-render if one of them changes.
 
 > **<sup>3</sup> Partial query matching** - Because React Query uses deterministic query key serialization, this allows you to manipulate variable groups of queries without having to know each individual query-key that you want to match, eg. you can refetch every query that starts with `todos` in its key, regardless of variables, or you can target specific queries with (or without) variables or nested properties, and even use a filter function to only match queries that pass your specific conditions.
 
diff --git a/docs/src/pages/reference/useQuery.md b/docs/src/pages/reference/useQuery.md
index d8a1bc474..9cb8be065 100644
--- a/docs/src/pages/reference/useQuery.md
+++ b/docs/src/pages/reference/useQuery.md
@@ -39,7 +39,7 @@ const {
   refetchOnWindowFocus,
   retry,
   retryDelay,
-  select
+  select,
   staleTime,
   structuralSharing,
   suspense,
@@ -109,10 +109,11 @@ const result = useQuery({
   - If set to `true`, the query will refetch on reconnect if the data is stale.
   - If set to `false`, the query will not refetch on reconnect.
   - If set to `"always"`, the query will always refetch on reconnect.
-- `notifyOnChangeProps: string[]`
+- `notifyOnChangeProps: string[] | "tracked"`
   - Optional
   - If set, the component will only re-render if any of the listed properties change.
   - If set to `['data', 'error']` for example, the component will only re-render when the `data` or `error` properties change.
+  - If set to `"tracked"`, access to properties will be tracked, and the component will only re-render when one of the tracked properties change.
 - `notifyOnChangePropsExclusions: string[]`
   - Optional
   - If set, the component will not re-render if any of the listed properties change.
diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts
index e40d5cc76..986326aab 100644
--- a/src/core/queryObserver.ts
+++ b/src/core/queryObserver.ts
@@ -54,6 +54,8 @@ export class QueryObserver<
   private initialErrorUpdateCount: number
   private staleTimeoutId?: number
   private refetchIntervalId?: number
+  private trackedProps!: Array<keyof QueryObserverResult>
+  private trackedCurrentResult!: QueryObserverResult<TData, TError>
 
   constructor(
     client: QueryClient,
@@ -65,6 +67,7 @@ export class QueryObserver<
     this.options = options
     this.initialDataUpdateCount = 0
     this.initialErrorUpdateCount = 0
+    this.trackedProps = []
     this.bindMethods()
     this.setOptions(options)
   }
@@ -208,6 +211,10 @@ export class QueryObserver<
     return this.currentResult
   }
 
+  getTrackedCurrentResult(): QueryObserverResult<TData, TError> {
+    return this.trackedCurrentResult
+  }
+
   getNextResult(
     options?: ResultOptions
   ): Promise<QueryObserverResult<TData, TError>> {
@@ -449,11 +456,15 @@ export class QueryObserver<
     }
 
     const keys = Object.keys(result)
+    const includedProps =
+      notifyOnChangeProps === 'tracked'
+        ? this.trackedProps
+        : notifyOnChangeProps
 
     for (let i = 0; i < keys.length; i++) {
       const key = keys[i] as keyof QueryObserverResult
       const changed = prevResult[key] !== result[key]
-      const isIncluded = notifyOnChangeProps?.some(x => x === key)
+      const isIncluded = includedProps?.some(x => x === key)
       const isExcluded = notifyOnChangePropsExclusions?.some(x => x === key)
 
       if (changed) {
@@ -461,7 +472,11 @@ export class QueryObserver<
           continue
         }
 
-        if (!notifyOnChangeProps || isIncluded) {
+        if (
+          !notifyOnChangeProps ||
+          isIncluded ||
+          (notifyOnChangeProps === 'tracked' && this.trackedProps.length === 0)
+        ) {
           return true
         }
       }
@@ -479,6 +494,26 @@ export class QueryObserver<
     // Only update if something has changed
     if (!shallowEqualObjects(result, this.currentResult)) {
       this.currentResult = result
+
+      if (this.options.notifyOnChangeProps === 'tracked') {
+        const addTrackedProps = (prop: keyof QueryObserverResult) => {
+          if (!this.trackedProps.includes(prop)) {
+            this.trackedProps.push(prop)
+          }
+        }
+        this.trackedCurrentResult = {} as QueryObserverResult<TData, TError>
+
+        Object.keys(result).forEach(key => {
+          Object.defineProperty(this.trackedCurrentResult, key, {
+            configurable: false,
+            enumerable: true,
+            get() {
+              addTrackedProps(key as keyof QueryObserverResult)
+              return result[key as keyof QueryObserverResult]
+            },
+          })
+        })
+      }
     }
   }
 
diff --git a/src/core/types.ts b/src/core/types.ts
index 7ad1fd1e8..9cabdcc2a 100644
--- a/src/core/types.ts
+++ b/src/core/types.ts
@@ -136,8 +136,9 @@ export interface QueryObserverOptions<
   /**
    * If set, the component will only re-render if any of the listed properties change.
    * When set to `['data', 'error']`, the component will only re-render when the `data` or `error` properties change.
+   * When set to `tracked`, access to properties will be tracked, and the component will only re-render when one of the tracked properties change.
    */
-  notifyOnChangeProps?: Array<keyof InfiniteQueryObserverResult>
+  notifyOnChangeProps?: Array<keyof InfiniteQueryObserverResult> | 'tracked'
   /**
    * If set, the component will not re-render if any of the listed properties change.
    */
diff --git a/src/react/useBaseQuery.ts b/src/react/useBaseQuery.ts
index 05c7becdd..c75338b5a 100644
--- a/src/react/useBaseQuery.ts
+++ b/src/react/useBaseQuery.ts
@@ -79,5 +79,7 @@ export function useBaseQuery<TQueryFnData, TError, TData, TQueryData>(
     }
   }
 
-  return currentResult
+  return observer.options.notifyOnChangeProps === 'tracked'
+    ? observer.getTrackedCurrentResult()
+    : currentResult
 }


Test patch:
diff --git a/src/react/tests/useQuery.test.tsx b/src/react/tests/useQuery.test.tsx
index 67d21e6ef..472bd6166 100644
--- a/src/react/tests/useQuery.test.tsx
+++ b/src/react/tests/useQuery.test.tsx
@@ -739,6 +739,146 @@ describe('useQuery', () => {
     expect(states[1].dataUpdatedAt).not.toBe(states[2].dataUpdatedAt)
   })
 
+  it('should track properties and only re-render when a tracked property changes', async () => {
+    const key = queryKey()
+    const states: UseQueryResult<string>[] = []
+
+    function Page() {
+      const state = useQuery(key, () => 'test', {
+        notifyOnChangeProps: 'tracked',
+      })
+
+      states.push(state)
+
+      const { refetch, data } = state
+
+      React.useEffect(() => {
+        if (data) {
+          refetch()
+        }
+      }, [refetch, data])
+
+      return (
+        <div>
+          <h1>{data ?? null}</h1>
+        </div>
+      )
+    }
+
+    const rendered = renderWithClient(queryClient, <Page />)
+
+    await waitFor(() => rendered.getByText('test'))
+
+    expect(states.length).toBe(2)
+    expect(states[0]).toMatchObject({ data: undefined })
+    expect(states[1]).toMatchObject({ data: 'test' })
+  })
+
+  it('should not re-render if a tracked prop changes, but it was excluded', async () => {
+    const key = queryKey()
+    const states: UseQueryResult<string>[] = []
+
+    function Page() {
+      const state = useQuery(key, () => 'test', {
+        notifyOnChangeProps: 'tracked',
+        notifyOnChangePropsExclusions: ['data'],
+      })
+
+      states.push(state)
+
+      return (
+        <div>
+          <h1>{state.data ?? 'null'}</h1>
+        </div>
+      )
+    }
+
+    const rendered = renderWithClient(queryClient, <Page />)
+
+    await waitFor(() => rendered.getByText('null'))
+    expect(states.length).toBe(1)
+    expect(states[0]).toMatchObject({ data: undefined })
+
+    await queryClient.refetchQueries(key)
+    await waitFor(() => rendered.getByText('null'))
+    expect(states.length).toBe(1)
+    expect(states[0]).toMatchObject({ data: undefined })
+  })
+
+  it('should return the referentially same object if nothing changes between fetches', async () => {
+    const key = queryKey()
+    let renderCount = 0
+    const states: UseQueryResult<string>[] = []
+
+    function Page() {
+      const state = useQuery(key, () => 'test', {
+        notifyOnChangeProps: 'tracked',
+      })
+
+      states.push(state)
+
+      const { data } = state
+
+      React.useEffect(() => {
+        renderCount++
+      }, [state])
+
+      return (
+        <div>
+          <h1>{data ?? null}</h1>
+        </div>
+      )
+    }
+
+    const rendered = renderWithClient(queryClient, <Page />)
+
+    await waitFor(() => rendered.getByText('test'))
+    expect(renderCount).toBe(2)
+    expect(states.length).toBe(2)
+    expect(states[0]).toMatchObject({ data: undefined })
+    expect(states[1]).toMatchObject({ data: 'test' })
+
+    act(() => rendered.rerender(<Page />))
+    await waitFor(() => rendered.getByText('test'))
+    expect(renderCount).toBe(2)
+    expect(states.length).toBe(3)
+    expect(states[0]).toMatchObject({ data: undefined })
+    expect(states[1]).toMatchObject({ data: 'test' })
+    expect(states[2]).toMatchObject({ data: 'test' })
+  })
+
+  it('should always re-render if we are tracking props but not using any', async () => {
+    const key = queryKey()
+    let renderCount = 0
+    const states: UseQueryResult<string>[] = []
+
+    function Page() {
+      const state = useQuery(key, () => 'test', {
+        notifyOnChangeProps: 'tracked',
+      })
+
+      states.push(state)
+
+      React.useEffect(() => {
+        renderCount++
+      }, [state])
+
+      return (
+        <div>
+          <h1>hello</h1>
+        </div>
+      )
+    }
+
+    renderWithClient(queryClient, <Page />)
+
+    await waitFor(() => renderCount > 1)
+    expect(renderCount).toBe(2)
+    expect(states.length).toBe(2)
+    expect(states[0]).toMatchObject({ data: undefined })
+    expect(states[1]).toMatchObject({ data: 'test' })
+  })
+
   it('should be able to remove a query', async () => {
     const key = queryKey()
     const states: UseQueryResult<number>[] = []