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 954d85b. 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

✅ 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/eslint-plugin-query

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

@tanstack/angular-query-experimental

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

@tanstack/query-async-storage-persister

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

@tanstack/query-broadcast-client-experimental

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

@tanstack/query-core

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

@tanstack/query-persist-client-core

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

@tanstack/query-devtools

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

@tanstack/query-sync-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-sync-storage-persister@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-devtools

pnpm add https://pkg.pr.new/@tanstack/react-query-devtools@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/svelte-query-persist-client

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

@tanstack/vue-query

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

@tanstack/vue-query-devtools

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

commit: 954d85b

packages/query-core/src/queryObserver.ts

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?

packages/react-query/src/useBaseQuery.ts

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?

Just to prevent updateResult() to be called twice - if it has listeners, updateResult will be called

(It will work without this check too 🤷‍♂️)

yeah I just checked, if we remove both checks for !observer.hasListeners(), all the tests still pass. if we want to keep these checks, we should add tests where this fails @KATT. I know that calling updateResult multiple times might e.g. re-run inline select functions, but I'm not sure if that's worth optimizing for ...

I tried adding a test for it by

vi.spyOn(QueryObserver.prototype, 'updateResult')

but it's called a bunch of times within a render so adding a test [that won't break randomly when changing other things] the way I thought it will be quite hard

feel free to delete the optimization

I've deleted the first check, because it's technically impossible that willFetch is true and we have an observer. I've kept the second check because maybe an observer shows up in between: a9148ba

packages/query-core/src/retryer.ts

packages/query-core/src/queryObserver.ts

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 :)

packages/query-core/src/queryObserver.ts

KATT · 2024-09-17T20:09:27Z

Ping me know if you need anything on my end here or have questions that I can help answer, I don't see much tangible feedback as of yet ☺

TkDodo · 2024-09-18T05:29:02Z

This user-land implementation got me thinking: why don't we also do the promise handling in the react layer rather than the observer 🤔. It looks way simplified. What am I missing 😂?

https://github.com/nucleartux/react-query-read

TkDodo · 2024-09-18T08:51:40Z

@KATT I added more tests, and one scenario is failing (this one): if we cancel the query while we are suspending, we stay in a forever pending state.

I think the problem is that we just revert to the previous state if we get a query cancelation, and state happens to be status: 'pending' because we didn't have any previous data:

query/packages/query-core/src/query.ts

Lines 587 to 589 in 55b6c19

    
           if (isCancelledError(error) && error.revert && this.#revertState) { 
        
             return { ...this.#revertState, fetchStatus: 'idle' } 
        
           }

I'm not even sure rejecting the promise the right expectation here, as the query isn't in error state. Honestly not sure what should happen here - maybe staying in pending is the right call, idk ...

KATT · 2024-09-18T16:18:12Z

@KATT I added more tests, and one scenario is failing (this one): if we cancel the query while we are suspending, we stay in a forever pending state.

I think the problem is that we just revert to the previous state if we get a query cancelation, and state happens to be status: 'pending' because we didn't have any previous data:

query/packages/query-core/src/query.ts

Lines 587 to 589 in 55b6c19

if (isCancelledError(error) && error.revert && this.#revertState) {

return { ...this.#revertState, fetchStatus: 'idle' }

}

I'm not even sure rejecting the promise the right expectation here, as the query isn't in error state. Honestly not sure what should happen here - maybe staying in pending is the right call, idk ...

Good catch. Not sure what the right approach here is. I've never cancelled a query and don't see why I would ever do that 😅, but I understand people will use it and break it.

Hm.

I thiiiiiink rejecting it with an AbortError would make sense to me?

packages/query-core/src/queryObserver.ts

…rly bail-out

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

Ephem reviewed Sep 16, 2024

View reviewed changes

TkDodo reviewed Sep 17, 2024

View reviewed changes

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

TkDodo added 2 commits September 18, 2024 10:35

refactor: rename to experimental_prefetchInRender

c617a08

test: more tests

ae93100

test: more cancelation

138b126

fix cancellation

d3bd66c

This comment was marked as outdated.

Sign in to view

KATT added 4 commits September 19, 2024 11:05

make it work

7941921

tweak comment

a47709a

Update packages/react-query/src/useBaseQuery.ts

0df1dc3

simplify code a bit

2cbe35a

KATT commented Sep 19, 2024

View reviewed changes

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

Update packages/query-core/src/queryObserver.ts

60f830b

KATT commented Sep 19, 2024

View reviewed changes

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

KATT requested a review from TkDodo September 19, 2024 09:30

TkDodo reviewed Sep 19, 2024

View reviewed changes

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

TkDodo and others added 7 commits September 20, 2024 09:01

refactor: move experimental_prefetchInRender check until after the ea…

b70d48c

…rly bail-out

fix: when cancelled, the promise should stay pending

849b1b6

test: disabled case

c75afaa

chore: no idea what's going on

dee4546

refactor: delete unnecessary check

a9148ba

revert refactor i did for cancellation when we wanted it to throw

f01a06b

Merge branch 'main' into discussions-7980-promises

954d85b

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.

Ephem left a comment

Ephem Sep 16, 2024

KATT Sep 17, 2024 •

edited

Loading

TkDodo Sep 19, 2024

KATT Sep 20, 2024 •

edited

Loading

TkDodo Sep 20, 2024

Ephem commented Sep 16, 2024

TkDodo commented Sep 16, 2024

KATT commented Sep 17, 2024

TkDodo commented Sep 18, 2024

TkDodo commented Sep 18, 2024

KATT commented Sep 18, 2024

This comment was marked as outdated.

This comment was marked as outdated.

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.

Ephem left a comment

Choose a reason for hiding this comment

Ephem Sep 16, 2024

Choose a reason for hiding this comment

KATT Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

TkDodo Sep 19, 2024

Choose a reason for hiding this comment

KATT Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

TkDodo Sep 20, 2024

Choose a reason for hiding this comment

Ephem commented Sep 16, 2024

TkDodo commented Sep 16, 2024

KATT commented Sep 17, 2024

TkDodo commented Sep 18, 2024

TkDodo commented Sep 18, 2024

KATT commented Sep 18, 2024

This comment was marked as outdated.

This comment was marked as outdated.

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

KATT Sep 17, 2024 •

edited

Loading

KATT Sep 20, 2024 •

edited

Loading