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

Move network to TanStack query #1489

Merged
merged 2 commits into from
Jul 23, 2024
Merged

Move network to TanStack query #1489

merged 2 commits into from
Jul 23, 2024

Conversation

teclator
Copy link
Contributor

@teclator teclator commented Jul 22, 2024

Adapted the UI using TanStack query instead of route loaders and useEffect hooks.


Related to #1439, #1452, #1470, and #1483

@teclator teclator requested a review from dgdavid July 22, 2024 08:39
@teclator teclator marked this pull request as ready for review July 22, 2024 09:04
@dgdavid
Copy link
Contributor

dgdavid commented Jul 22, 2024

@teclator, thanks a lot for moving this on.

Let's try to fix failing test before merging changes. Ping me if you need help to do so.

@dgdavid
Copy link
Contributor

dgdavid commented Jul 23, 2024

Let's try to fix failing test before merging changes.

Done at c82cc1e

Basically there were two problems, both related to NetworkConnectionForm component

  • a useNetwork left over at NetworkConnectionForm.jsx; since it's build on top of useSuspenseQuery the assertion was performed when nothing rendered yet.
  • a lack of useNetworkConfigChanges mock at NetworkConnectionForm.test.jsx

Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Since this is a kind of intermediate step and everything is green, it looks good to be merged in mater and continue working from there. Thanks!

@teclator teclator merged commit 55afa6e into master Jul 23, 2024
2 checks passed
@teclator teclator deleted the network_query branch July 23, 2024 08:23
@imobachgs imobachgs mentioned this pull request Sep 20, 2024
imobachgs added a commit that referenced this pull request Sep 20, 2024
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.

2 participants