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

feat: atomsWithQueryAsync, plus example #30

Merged
merged 9 commits into from
Apr 3, 2023

Conversation

leweyse
Copy link
Contributor

@leweyse leweyse commented Mar 22, 2023

Fix to #21

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 22, 2023

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 9bb7892:

Sandbox Source
React Configuration
React Typescript Configuration

@dai-shi
Copy link
Member

dai-shi commented Mar 22, 2023

Thanks for working on it. Can you please try adding some tests? You don't need to follow the patterns of existing tests.

@dai-shi dai-shi linked an issue Mar 22, 2023 that may be closed by this pull request
@@ -0,0 +1,56 @@
import type { QueryKey, QueryObserverOptions } from '@tanstack/query-core'
import type { ExtractAtomValue, Getter, WritableAtom } from 'jotai'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With pmndrs/jotai#1741, we should be able to use it.

Suggested change
import type { ExtractAtomValue, Getter, WritableAtom } from 'jotai'
import type { ExtractAtomArgs, ExtractAtomValue, Getter, WritableAtom } from 'jotai'

@leweyse
Copy link
Contributor Author

leweyse commented Mar 25, 2023

Hi @dai-shi , I have a couple of doubts.

First, I tried to add a test for the refetch functionality for the new async atom (async query refetch test), but I have some problems trying to set this up. Do you have any comments on that test? Any ideas on how can we approach it?

Second, when we fire the dispatch setter from the atomsWithQueryAsync, and we pass the force argument as true, the component is suspended forever. You can find the code here: https://codesandbox.io/s/hopeful-silence-f2zs2k?file=/src/App.tsx

@dai-shi
Copy link
Member

dai-shi commented Mar 26, 2023

First, I tried to add a test for the refetch functionality for the new async atom (async query refetch test), but I have some problems trying to set this up. Do you have any comments on that test? Any ideas on how can we approach it?

Not sure. Is it only about tests? It works in codesandbox, right?

Second, when we fire the dispatch setter from the atomsWithQueryAsync, and we pass the force argument as true, the component is suspended forever. You can find the code here: https://codesandbox.io/s/hopeful-silence-f2zs2k?file=/src/App.tsx

Oh, I didn't realized that. Does it work with atomsWithQuery? Not sure if we can fix it. Maybe for now we reject using force in atomsWithQueryAsync?

@leweyse
Copy link
Contributor Author

leweyse commented Mar 28, 2023

I was able to fix the tests, and force has no effect on the query at this moment.

First, I tried to add a test for the refetch functionality for the new async atom (async query refetch test), but I have some problems trying to set this up. Do you have any comments on that test? Any ideas on how can we approach it?

Not sure. Is it only about tests? It works in codesandbox, right?

Second, when we fire the dispatch setter from the atomsWithQueryAsync, and we pass the force argument as true, the component is suspended forever. You can find the code here: https://codesandbox.io/s/hopeful-silence-f2zs2k?file=/src/App.tsx

Oh, I didn't realized that. Does it work with atomsWithQuery? Not sure if we can fix it. Maybe for now we reject using force in atomsWithQueryAsync?

@leweyse leweyse closed this Mar 28, 2023
@leweyse leweyse reopened this Mar 28, 2023
@dai-shi
Copy link
Member

dai-shi commented Apr 1, 2023

I was able to fix the tests

Nice.

and force has no effect on the query at this moment.

Fine. Do you want to disable the force option in atomsWithQueryAsync?

Please check CI results. Seems like trivial linting issue.

@leweyse
Copy link
Contributor Author

leweyse commented Apr 2, 2023

I'd rather not to disable it, so I worked on a fix. I updated the sandbox: https://codesandbox.io/s/hopeful-silence-f2zs2k?file=/src/App.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to use async atom in atomsWithQuery jotai v2?
2 participants