feat: add support for `React.use()` #7988

KATT · 2024-08-30T10:50:09Z

Progress

add basic functionality for useQuery()
make other tests pass (👋 help wanted!)
add test for useInfiniteQuery()
do more tests with some example
update docs?

nx-cloud · 2024-08-30T10:50:41Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b573cc6. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✄1�7 Successfully ran 2 targets

Sent with 💌 from NxCloud.

pkg-pr-new · 2024-08-30T10:52:36Z

Open in Stackblitz

More templates

@tanstack/angular-query-devtools-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@7988

@tanstack/angular-query-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-experimental@7988

@tanstack/eslint-plugin-query

pnpm add https://pkg.pr.new/@tanstack/eslint-plugin-query@7988

@tanstack/query-broadcast-client-experimental

pnpm add https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@7988

@tanstack/query-async-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-async-storage-persister@7988

@tanstack/query-core

pnpm add https://pkg.pr.new/@tanstack/query-core@7988

@tanstack/query-devtools

pnpm add https://pkg.pr.new/@tanstack/query-devtools@7988

@tanstack/query-persist-client-core

pnpm add https://pkg.pr.new/@tanstack/query-persist-client-core@7988

@tanstack/query-sync-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-sync-storage-persister@7988

@tanstack/react-query-devtools

pnpm add https://pkg.pr.new/@tanstack/react-query-devtools@7988

@tanstack/react-query

pnpm add https://pkg.pr.new/@tanstack/react-query@7988

@tanstack/react-query-next-experimental

pnpm add https://pkg.pr.new/@tanstack/react-query-next-experimental@7988

@tanstack/react-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/react-query-persist-client@7988

@tanstack/solid-query

pnpm add https://pkg.pr.new/@tanstack/solid-query@7988

@tanstack/solid-query-devtools

pnpm add https://pkg.pr.new/@tanstack/solid-query-devtools@7988

@tanstack/solid-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/solid-query-persist-client@7988

@tanstack/svelte-query

pnpm add https://pkg.pr.new/@tanstack/svelte-query@7988

@tanstack/svelte-query-devtools

pnpm add https://pkg.pr.new/@tanstack/svelte-query-devtools@7988

@tanstack/vue-query

pnpm add https://pkg.pr.new/@tanstack/vue-query@7988

@tanstack/svelte-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/svelte-query-persist-client@7988

@tanstack/vue-query-devtools

pnpm add https://pkg.pr.new/@tanstack/vue-query-devtools@7988

commit: b573cc6

packages/query-core/src/queryObserver.ts

packages/react-query/src/useBaseQuery.ts

…ly have no cache entry yet

TkDodo · 2024-09-09T14:09:51Z

I made two changes:

e217605 re-use fetchOptimistic util - it does some more stuff around error boundary reset, and it also has a catch already so we shouldn't need the empty .catch
be6c1f0 check if we really don't have a cache entry yet before we fire the fetchOptimistic. Checking for no subscribers isn't enough - data can get into the cache by other means, even if there are no subscribers. This has to be done before we call getOptimisticResult because that potentially creates a cache entry.

KATT · 2024-09-13T10:06:55Z

cc @tkodo I've updated it so it's behind a feature flag now

TkDodo · 2024-09-13T10:07:39Z

I will talk with @Ephem about this next week

TkDodo · 2024-09-14T13:04:21Z

I added some more tests where I wasn't sure if that will work, but it does 👏 .

I will discuss this PR with @Ephem next week but I think we can ship it then. I might want to rename the feature flag to experimental_autoPrefetching or something like that, because you can theoretically turn it on without ever using the promise, and it will give you auto prefetching during rendering.

TkDodo · 2024-09-14T13:33:20Z

So I made some more tests for normal useQuery with auto-prefetching on and off, on a simple component:

it('test', async () => {
  function Page() {
    console.log('render', Date.now())
    const { data } = useQuery({
      queryKey: ['test'],
      queryFn: () => {
        console.log('queryFn', Date.now())
        return 'test'
      },
    })

    return (
      <div>
        <h1>{data}</h1>
      </div>
    )
  }

  renderWithClient(queryClient, <Page />)
})

with the flag on, the two logged timestamps consistently differ by 2ms, because we start the fetch immediately.

with the flag off, we start the fetch in the uSES subscription / an effect, which can be later, depending on how intensive rendering is. In this little example, it was usually 4-5ms. However, if we add a bit of cpu intensive work to our component, like rendering 1k divs:

{Array.from({ length: 1000 }, (_, i) => String(i)).map((v) => (
  <div key={v}>{v}</div>
))}

the delay was around 30ms, but obviously still a constant 2ms with the flag on.

This probably doesn't matter if your fetch takes 500ms, but I've just had a discussion with someone who said their fetches were delayed by 1-3 seconds, and it turns out it was something in their component rendering.

So the question is: is this actually a good optimization for normal useQuery users too? Even if they don't suspend, it should be safe to trigger the fetch during render because there are no active observers (as we trigger the fetch conditionally). It's similar to how our hydration writes to the cache during render for seeding it, or how usePrefetchQuery can fetch during render. Thoughts @Ephem ?

Ephem

Fantastic! 👏👏 This is pretty complex, so I'll need to some more time to really wrap my head around things.

So many great tests here! Do you think it would make sense to also add some tests with <HydrationBoundary>, both when we de/rehydrate already awaited data, and when we de/rehydrate a promise?

Ephem · 2024-09-16T18:15:22Z

packages/react-query/src/useBaseQuery.ts

+    !observer.hasListeners()
+  ) {
+    const promise = isNewCacheEntry
+      ? // Fetch immediately on mount in order to ensure `.promise` is resolved even if the component is unmounted


NIT: This comment has me a bit confused, is it "in render" instead of "on mount"?

Ephem · 2024-09-16T18:31:38Z

packages/react-query/src/useBaseQuery.ts

+        client.getQueryCache().get(defaultedOptions.queryHash)?.promise
+
+    promise?.catch(noop).finally(() => {
+      if (!observer.hasListeners()) {


I'm trying to wrap my head around this all. Why are we checking for !observer.hasListeners() here?

packages/query-core/src/retryer.ts

Ephem · 2024-09-16T18:51:01Z

packages/query-core/src/queryObserver.ts

@@ -82,6 +85,13 @@ export class QueryObserver<

    this.#client = client
    this.#selectError = null
+    this.#currentThenable = pendingThenable()


I just want to double check that I'm thinking about this correctly. This is a "proxy"-thenable that is kept in sync with the promise that lives in the query cache, and the mechanism for that "syncing" is the updateResult-function?

So any means of resolving, rejecting or replacing the original promise needs to call updateResult on all observers to "sync" to the thenables?

Just thinking out loud, this makes me wonder what happens if you:

useQuery -> use(promise)

fetchQuery / refetch before above query resolves, and either:

The first query resolves first

The fetchQuery / refetch resolves first

Does this have a defined behaviour? I'm not even sure what should happen. 😅

we can add tests for this, but the way I see it, fetchQuery would just pick up the promise that's already running and not start a new fetch. Of course, if you do queryClient.refetchQueries and target this key, it would cancel the ongoing fetch per default. Not sure what would happen then, cancel should silently rollback to the previous (pending) state, so maybe it would hang forever.

Definitely interesting case that's worth a couple of tests 👍

Ephem · 2024-09-16T19:11:19Z

Thoughts @Ephem ?

I think I need to marinate this a bit. I mean, on one hand it should be "safe" to do. On the other, is it always desirable? I'm thinking about (future) cases like pre-rendering, the <Offscreen> API etc. Probably? But maybe not if a router prerenders all possible routes based on Links on a page? Will devs need some more control (prefetch, but not if this is a prerender).

If there are any issues there, I fully expect usePrefetchQuery to have the same ones, but at least that's more explicit. I think that's mostly an argument for not aiming to make this the "default" behaviour though and a big part of me wants to say this is a nice little optimisation and we can tackle any challenges when they come up. 😄

TkDodo · 2024-09-16T19:44:31Z

If there are any issues there, I fully expect usePrefetchQuery to have the same ones

yes, this auto_prefetching was pretty much taken from the usePrefetchQuery implementation. I think we are aligned that we could keep it as an experimental flag though, and that you need to turn it on to be able to use(promise), which makes the whole promise feature experimental, too :)

TkDodo · 2024-09-17T08:04:03Z

packages/query-core/src/queryObserver.ts

@@ -82,6 +85,13 @@ export class QueryObserver<

    this.#client = client
    this.#selectError = null
+    this.#currentThenable = pendingThenable()
+    if (!this.options.experimental_promise) {


let's rename to: experimental_prefetchInRender

github-actions bot added package: react-query package: query-core labels Aug 30, 2024

TkDodo reviewed Aug 30, 2024

View reviewed changes

packages/query-core/src/queryObserver.ts Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

KATT closed this Aug 31, 2024

KATT reopened this Sep 1, 2024

let’s do it again

8cbd6a0

KATT force-pushed the discussions-7980-promises branch from ec583d5 to 8cbd6a0 Compare September 1, 2024 19:32

KATT added 4 commits September 1, 2024 21:34

fix test group

739d023

maybe

7fe1d33

mkay

757aeab

cool

7c88325

This comment was marked as outdated.

Sign in to view

KATT added 3 commits September 1, 2024 23:27

rm console.logs

c2a2cb5

mkay

d290fca

mkay

da88d68

KATT and others added 4 commits September 6, 2024 14:04

tweak more

8a64488

just naming and comments

27996a4

tweak

011e373

fix: use fetchOptimistic util instead of observer.fetchOptimistic

e217605

TkDodo reviewed Sep 9, 2024

View reviewed changes

packages/react-query/src/useBaseQuery.ts Outdated Show resolved Hide resolved

TkDodo added 2 commits September 9, 2024 16:07

refactor: make sure to only trigger fetching during render if we real 1�7

be6c1f0

…ly have no cache entry yet

Merge branch 'main' into discussions-7980-promises

6f8b777

TkDodo and others added 8 commits September 9, 2024 16:28

fix: move the isNewCacheEntry check before observer creation

9b4e38b

chore: avoid rect key warnings

b8cb318

fix: add an updateResult for all observers to finalize currentThenable

3b33b03

chore: logs during suspense errors

86b8b03

fix: empty catch

9df70ac

feature flag

ac8754a

Merge branch 'main' into discussions-7980-promises

1b7f5d4

add comment

1e4f719

simplify

4ee2728

KATT requested a review from TkDodo September 13, 2024 10:09

KATT and others added 4 commits September 13, 2024 12:11

omit from suspense

2fa83df

feat flag

9f281f4

more tests

540ddd5

test: scope experimental_promise to useQuery().promise tests

b573cc6

Ephem reviewed Sep 16, 2024

View reviewed changes

TkDodo reviewed Sep 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for `React.use()` #7988

feat: add support for `React.use()` #7988

KATT commented Aug 30, 2024 •

edited

Loading

nx-cloud bot commented Aug 30, 2024 •

edited

Loading

pkg-pr-new bot commented Aug 30, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

TkDodo commented Sep 9, 2024

KATT commented Sep 13, 2024

TkDodo commented Sep 13, 2024

TkDodo commented Sep 14, 2024 •

edited

Loading

TkDodo commented Sep 14, 2024

Ephem left a comment

Ephem Sep 16, 2024

Ephem Sep 16, 2024

Ephem Sep 16, 2024

TkDodo Sep 16, 2024

Ephem commented Sep 16, 2024

TkDodo commented Sep 16, 2024

TkDodo Sep 17, 2024

feat: add support for React.use() #7988

Are you sure you want to change the base?

feat: add support for React.use() #7988

Conversation

KATT commented Aug 30, 2024 • edited Loading

Progress

nx-cloud bot commented Aug 30, 2024 • edited Loading

☁️ Nx Cloud Report

pkg-pr-new bot commented Aug 30, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

TkDodo commented Sep 9, 2024

KATT commented Sep 13, 2024

TkDodo commented Sep 13, 2024

TkDodo commented Sep 14, 2024 • edited Loading

TkDodo commented Sep 14, 2024

Ephem left a comment

Choose a reason for hiding this comment

Ephem Sep 16, 2024

Choose a reason for hiding this comment

Ephem Sep 16, 2024

Choose a reason for hiding this comment

Ephem Sep 16, 2024

Choose a reason for hiding this comment

TkDodo Sep 16, 2024

Choose a reason for hiding this comment

Ephem commented Sep 16, 2024

TkDodo commented Sep 16, 2024

TkDodo Sep 17, 2024

Choose a reason for hiding this comment

feat: add support for `React.use()` #7988

feat: add support for `React.use()` #7988

KATT commented Aug 30, 2024 •

edited

Loading

nx-cloud bot commented Aug 30, 2024 •

edited

Loading

pkg-pr-new bot commented Aug 30, 2024 •

edited

Loading

TkDodo commented Sep 14, 2024 •

edited

Loading