feat: (breaking) api overhaul #45

kalijonn · 2023-09-21T11:39:20Z

I wanted to get your feedback on some of the changes. some tests are still breaking here.

My idea was to use as much from tanstack query as possible with very little implementation in jotai. With this, I've setup conditions which make suspense work only if suspense is true. tanstack query wants users to be explicit about suspense. Even in v5, they're creating new hooks (useSuspenseQuery) rather than making suspense default. This goes against jotai's default implementation? useAtom throws promise be default if it receives a promise.

Another one is refetch. some of the tests indicate that refetch triggers suspense but that's different from tanstack query. tanstack query does not trigger suspense when a query is refetching.

…tch from tanstack

dai-shi · 2023-09-27T01:24:38Z

src/atomWithQuery.ts

+  })
+
+  return resultAtom
+}


Is the implementation mostly similar to atomsWithQuery? Any fundamental difference?

Implementation would be very close to atomsWithQuery with some differences in how we handle features like suspense, keepPreviousData (I've been working on this locally not in the PR yet). I'll update this PR soon.

I didn't have an opinion on how to handle those but having tested things out quite a bit more in the past week, I think we should keep things consistent with what users expect from tanstack. Refetch shouldn't trigger suspense. Suspense should be opt-in, probably with atomWithSuspenseQuery (for v5) like you mentioned. Let me know what you think.

I think we should keep things consistent with what users expect from tanstack.

I agree with it. That's the major goal of the overhaul #42.

dai-shi · 2023-09-27T01:25:48Z

Even in v5, they're creating new hooks (useSuspenseQuery) rather than making suspense default.

In that case, the better abstraction might be separating atomWithQuery and atomWithSuspenseQuery.

dai-shi · 2023-09-27T01:26:35Z

And, it feels like the new jotai-tanstanck-query version should use v5, no?

kalijonn · 2023-09-27T06:56:32Z

Even in v5, they're creating new hooks (useSuspenseQuery) rather than making suspense default.

In that case, the better abstraction might be separating atomWithQuery and atomWithSuspenseQuery.

I agree.

And, it feels like the new jotai-tanstanck-query version should use v5, no?

they released a release candidate 2 days ago. I think the work I'm doing can work for both v4 and v5 with very little changes.

dai-shi · 2023-09-27T11:13:28Z

In that case, the better abstraction might be separating atomWithQuery and atomWithSuspenseQuery.

I agree.

Let's try that then.

they released a release candidate 2 days ago. I think the work I'm doing can work for both v4 and v5 with very little changes.

Do you want to continue supporting v4 for your new api? I think it can only be for v5.

codesandbox-ci · 2023-10-01T13:01:08Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2da7b56:

Sandbox	Source
React	Configuration
React TypeScript	Configuration

kalijonn · 2023-10-01T13:03:47Z

src/QueryAtomErrorResetBoundary.tsx

+  // feels hacky.
+  // Right after the reset is set, we need to set it back to clear so that  when we refetch, it doesn't think it's in a reset state
+  store.sub(isResetAtom, () => {
+    Promise.resolve().then(() => store.set(valueAtom, 'clear'))
+  })
+


I'm trying to replicate QueryErrorResetBoundary which is implemented in @tanstack/react-query and not core.

This bit feels hacky. I'm not sure if there's a cleaner way to do it in jotai

kalijonn · 2023-10-01T13:11:39Z

I'm going the route of separating creating atomWithQuery and atomWithSuspenseQuery. will do the same for other *WithQueries as well.

atomWithQuery passes all tests. I've adjusted the tests to keep them inline with tanstack/query functionality.

created a new component QueryAtomErrorResetBoundary, to use in the place of QueryErrorResetBoundary from tanstack. tanstack's boundary uses context. I don't think we can use that directly within jotai atoms. tanstack-reference

dai-shi · 2023-10-01T13:19:19Z

src/QueryAtomErrorResetBoundary.tsx

@@ -0,0 +1,48 @@
+import React from 'react'
+import { Provider, atom, createStore } from 'jotai'


This feels very strange.
Basically, jotai-tanstack-query is a package for Jotai, but it should be independent from React. Jotai atoms is framework-agnostic, like tanstack-query/core.
Please consider making QueryErrorResetBoundary a recipe in docs, instead of providing it as a library function.

I thought we could do something specific for react (jotai-tanstack-query/react), but I agree with you. It's probably better off as a recipe.

Also, I think I can come up with some framework agnostic solution to reset error boundaries.

Will take a look.

kalijonn · 2023-12-04T21:34:08Z

@dai-shi everything except resetting error boundary support is done.

main changes:

moved to v5
separated suspense vs non-suspense

let me know what you think

dai-shi · 2023-12-06T01:43:55Z

Hi, thanks for working on this.
I can't review everything, but looking at atomWithQuery and its related code, it generally looks good. Great work!

I would like to discuss one fundamental idea. Sorry, if I raise it too late.
I don't remember which issues/discussions/chats we discussed,
but someone suggested to use query cache as the source of truth.
So, instead of creating query observer in each atom, we directly subscribe to query cache.
How does that sound? We would like to consider if it's a viable solution.

kalijonn · 2023-12-07T07:27:12Z

I've looked into QueryCache and realized there's a lot of functionality that QueryObserver implements in terms of returing promises, conditionally fetching/pausing which we'll have to implement. We'd have to build our own QueryObserver.

While refactoring, I have tried to replicate the logic that tanstack hooks perform, in atoms. And in this process, I'm questioning that statement that 'query observer might not be a reliable source of truth'. I've been trying to add tests as issues come up. (most recent "on reset, atom wasn't suspending" - raised in discord)

For now, I haven't seen anything that points towards queryobserver not being reliable.

dai-shi · 2023-12-07T09:05:45Z

@kalijonn Can you join my Discord server and DM me? https://discord.gg/MrQdmzd

dai-shi · 2023-12-07T14:01:05Z

For now, I haven't seen anything that points towards queryobserver not being reliable.

Sounds good. If you considered it and the current conclusion is so, it's perfect.

dai-shi

Great contribution!

I don't think I can understand it fully, but it looks good from the jotai usage perspective. We should release it and collect feedbacks including bug reports.

We also need to update docs and add a migration guide.

dai-shi · 2023-12-07T14:14:53Z

package.json

  },
  "peerDependencies": {
    "@tanstack/query-core": "*",
-    "jotai": ">=1.11.0"
+    "jotai": ">=1.11.0",
+    "wonka": "^6.3.4"


we have a new depenency.

cyrilchapon · 2023-12-07T17:08:59Z

@dai-shi I might install this on my side project; give it a shot and feedback.
Any preferred channel ?

dai-shi · 2023-12-07T22:13:41Z

@dai-shi I might install this on my side project; give it a shot and feedback. Any preferred channel ?

That would be great! Please open a new issue in this repo for the feedback.
You can install from the csb build. See "Local Install Instructions" in: https://ci.codesandbox.io/status/jotaijs/jotai-tanstack-query/pr/45

BTW, from now on, @kalijonn will be a new maintainer of this package.

feat: atomWithQuery. make suspense consistent with tanstack. use refe…

8df0e80

…tch from tanstack

dai-shi reviewed Sep 27, 2023

View reviewed changes

dai-shi mentioned this pull request Sep 28, 2023

[atomsWithMutation] Feature Consideration: automatic dependency mutation #47

Closed

kalijonn added 5 commits September 30, 2023 16:02

arbitrary change

1f80f9f

final atomWithQuery

7c751a5

address pnpm issues

5dc8a0b

actually address pnpm issues

a2168fc

address microbundle errors

5a245d7

kalijonn commented Oct 1, 2023

View reviewed changes

dai-shi reviewed Oct 1, 2023

View reviewed changes

arbitrary change

8d3fbde

dai-shi mentioned this pull request Oct 19, 2023

React-query v5 OnSuccess, OnError ... #51

Closed

kalijonn added 6 commits November 30, 2023 12:24

use v5, add atomWithMutation, atomWithSuspense with existing tests

c1a064b

add atomWithInfiniteQuery with existing tests adjusted to v5 api

2207e4c

add atomWithSuspenseInfiniteQuery

c412300

refactor to baseAtom pattern

76104f1

add examples

e26964e

add atomWithMutationState

076ccb3

kalijonn changed the title ~~feat: atomWithQuery~~ feat: (breaking) api overhaul Dec 4, 2023

kalijonn added 2 commits December 5, 2023 12:01

use node v20 for codesandbox ci

48139f3

use node v18 because v20 isn't supported yet by codesandbox

9632766

add suspense test, on reset throws suspense

0b22ad0

can recover from error for non-suspense queries

92aa08d

kalijonn added 3 commits December 7, 2023 13:07

mark debugPrivate true

a817ccc

remove atomWithObservable, stale observer ref

3d4fb87

update examples

7e7261e

remove QueryAtomErrorResetBoundary.tsx

758d1c1

dai-shi linked an issue Dec 7, 2023 that may be closed by this pull request

Idea for overhaul #42

Closed

cleanup, update package.json

2da7b56

dai-shi approved these changes Dec 7, 2023

View reviewed changes

dai-shi merged commit e6f6cb7 into jotaijs:main Dec 7, 2023
2 checks passed

kalijonn deleted the kj/atomWithQuery branch December 7, 2023 14:18

kalijonn mentioned this pull request Dec 9, 2023

docs: update tanstack query integration documentation to the new api pmndrs/jotai#2294

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: (breaking) api overhaul #45

feat: (breaking) api overhaul #45

kalijonn commented Sep 21, 2023

dai-shi Sep 27, 2023

kalijonn Sep 27, 2023

dai-shi Sep 27, 2023

dai-shi commented Sep 27, 2023

dai-shi commented Sep 27, 2023

kalijonn commented Sep 27, 2023

dai-shi commented Sep 27, 2023

codesandbox-ci bot commented Oct 1, 2023 •

edited

Loading

kalijonn Oct 1, 2023

kalijonn commented Oct 1, 2023

dai-shi Oct 1, 2023

kalijonn Oct 1, 2023

kalijonn commented Dec 4, 2023

dai-shi commented Dec 6, 2023

kalijonn commented Dec 7, 2023

dai-shi commented Dec 7, 2023

dai-shi commented Dec 7, 2023

dai-shi left a comment

dai-shi Dec 7, 2023

cyrilchapon commented Dec 7, 2023 •

edited

Loading

dai-shi commented Dec 7, 2023

		@@ -0,0 +1,48 @@
		import React from 'react'
		import { Provider, atom, createStore } from 'jotai'

feat: (breaking) api overhaul #45

feat: (breaking) api overhaul #45

Conversation

kalijonn commented Sep 21, 2023

dai-shi Sep 27, 2023

Choose a reason for hiding this comment

kalijonn Sep 27, 2023

Choose a reason for hiding this comment

dai-shi Sep 27, 2023

Choose a reason for hiding this comment

dai-shi commented Sep 27, 2023

dai-shi commented Sep 27, 2023

kalijonn commented Sep 27, 2023

dai-shi commented Sep 27, 2023

codesandbox-ci bot commented Oct 1, 2023 • edited Loading

kalijonn Oct 1, 2023

Choose a reason for hiding this comment

kalijonn commented Oct 1, 2023

dai-shi Oct 1, 2023

Choose a reason for hiding this comment

kalijonn Oct 1, 2023

Choose a reason for hiding this comment

kalijonn commented Dec 4, 2023

dai-shi commented Dec 6, 2023

kalijonn commented Dec 7, 2023

dai-shi commented Dec 7, 2023

dai-shi commented Dec 7, 2023

dai-shi left a comment

Choose a reason for hiding this comment

dai-shi Dec 7, 2023

Choose a reason for hiding this comment

cyrilchapon commented Dec 7, 2023 • edited Loading

dai-shi commented Dec 7, 2023

codesandbox-ci bot commented Oct 1, 2023 •

edited

Loading

cyrilchapon commented Dec 7, 2023 •

edited

Loading