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.

Sign up for GitHub

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

Jump to bottom

refactor(web): adopt TanStack Query for state management #1439

Merged
merged 39 commits into from
Jul 8, 2024
Merged

refactor(web): adopt TanStack Query for state management #1439

merged 39 commits into from
Jul 8, 2024

Conversation

imobachgs
Copy link
Contributor

@imobachgs imobachgs commented Jul 4, 2024 •
edited
Loading

Trello: https://trello.com/c/8u1WOJz4/3723-8-agama-ui-adopt-a-state-management-solution

Agama's UI state management could be better. For that reason, and after evaluating a few alternatives (see the list of considered alternatives), we decided to adopt Tanstack Query (former React Query).

Why TanStack query

  • It offers a mechanism (based on promises) to fetch/update the state.
  • It is not limited to the client state: it works great with the server state, too.
  • It takes care of caching, re-fetching, error handling, etc.
  • It is a mature, well-documented and widely known project.
  • Plays nicely with React Router, although a TanStack Router project exits.

What is included in this PR

There is quite some work to do so let's consider this pull request as the first step in this path. It introduces the following changes:

  • Add TanStack query as a dependency.
  • Replace L10Provider with a queries-based approach, including data mutation. Check queries/l10n.js if you are curious.
  • Re-enable L10nPage tests (and add other tests).
  • Use React Router loaders to make sure data is loaded before the components are rendered.

Considered Alternatives

We had a look at others but did not consider them at all.

imobachgs and others added 30 commits July 2, 2024 09:09
@imobachgs
feat(web): add TanStack Query
d5cf137
@imobachgs
feat(web): use React Query to fetch locales data
1054332
@imobachgs
refactor(web): move QueryClientProvider to app context
f975f0f
@imobachgs
refactor(web): expose the client's websocket
503e372
@imobachgs
refactor(web): use queries to listen for l10n changes
e263dbe
@imobachgs
refactor(web): adapt L10nPage to use queries
7472ab8
@imobachgs
refactor(web): adapt L10nSection to use queries
5d6232a
@imobachgs
refactor(web): use ~/queries instead of relative paths
2b83836
@imobachgs
refactor(web): use functions to build queries
eeeb61a
@imobachgs
fix(web): minimize l10n data retrieving
82c20df
@dgdavid
refactor(web): move l10n routes to ~/routes
37a7e80
@imobachgs
refactor(web): adapt InstallerOptions to use queries
227c42f
@dgdavid
fix(web): add missing file
179b65e 
Forgotten at 37a7e80
@imobachgs
refactor(web): drop the l10n context
79ab607
@imobachgs
test(web): adapt test-utils to react-query
4c02c9c
@imobachgs
test(web): adapt basic L10nPage test to react-query
059ba54
@dgdavid
refactor(web): improve l10n selectors using RR loaders
8bbac2c 
Part of the code was accidentally sent as part of 179b65e.
@imobachgs
fix(web): listen for L10n changes from App
a8543eb
@imobachgs
fix(web): fix locales update
bc6ff01
@dgdavid
fix(web): revalidate loader on config change
ccaa0f0 
By making use of https://reactrouter.com/en/main/hooks/use-revalidator

See
remix-run/react-router#10381 (reply in thread)
too
@imobachgs
fix(web): add a role to CardField
c0367be 
* The label is not properly set yet.
@imobachgs
test(web): write a new test for L10nPage
122bf20
@imobachgs
test: drop the queryRender helper
0fd96ff
@imobachgs
doc(web): document l10n queries
e018d5f
@imobachgs
fix(web): make ESLint happy
3cdcaf6
@imobachgs
refactor(web): remove unused methods from L10nClient
58a27ff
@imobachgs
test(web): fix App tests
f633c95
@imobachgs
refactor(web): drop unused L10nClient tests
6f16921
@imobachgs
feat(web): wrap plainRender on a QueryClientProvider
4766a2d 
* In case you need something really minimal, use
  the "render" function from React Testing Library.
@imobachgs
test(web): fix L10nSectino tests
b893146
@imobachgs imobachgs merged commit 05e7963 into master Jul 8, 2024
7 checks passed
@imobachgs imobachgs deleted the react-query branch July 8, 2024 12:33
@dgdavid dgdavid mentioned this pull request Jul 10, 2024
Merged
@imobachgs imobachgs mentioned this pull request Jul 11, 2024
Merged
jreidinger added a commit that referenced this pull request Jul 11, 2024
@jreidinger
refactor(web): drop products context (#1454)
fd69dfc 
Use TanStack to replace `ProductProvider`. It is related to #1439 and
#1452.
@imobachgs imobachgs mentioned this pull request Jul 16, 2024
Merged
imobachgs added a commit that referenced this pull request Jul 17, 2024
@imobachgs
refactor(web): drop issues context (#1470)
53eb89e 
Use TanStack to replace IssuesProvider. It is related to #1439 and
#1452.
This was referenced Jul 18, 2024
Merged
Merged
teclator added a commit that referenced this pull request Jul 23, 2024
@teclator
Move network to TanStack query (#1489)
55afa6e 
Adapted the UI using TanStack query instead of route loaders and
useEffect hooks.


---

Related to #1439,
#1452,
#1470, and
#1483
@imobachgs imobachgs mentioned this pull request Jul 23, 2024
Merged
imobachgs added a commit that referenced this pull request Jul 23, 2024
@imobachgs
refactor(web): replace the UsersClient with queries (#1492)
fc5a871 
Adapt the users management code to use queries instead of `UsersClient`.
Related to #1439.
dgdavid added a commit that referenced this pull request Jul 23, 2024
@dgdavid
refactor(web): use queries for dealing with software (#1483)
22d0125 
Similar to #1439, #1452, and #1470, this set of changes aims to replace
the client/software with Tanstack queries.

Note that was not possible to fully drop the software client. It has to
wait until migration of
[`WithStatus`](https://github.com/openSUSE/agama/blob/bd2f35d0ead6d74931189f5619579f6c3ffa2770/web/src/client/mixins.js#L83)
and
[`WithProgress`](https://github.com/openSUSE/agama/blob/bd2f35d0ead6d74931189f5619579f6c3ffa2770/web/src/client/mixins.js#L147)
mixins too.
@dgdavid dgdavid mentioned this pull request Jul 24, 2024
Merged
imobachgs added a commit that referenced this pull request Jul 25, 2024
@imobachgs
refactor(web): drop product registration code (#1499)
25c6814 
At this moment, the code for working with product registration is not
used. Moreover, it has to be migrated to a TanStack Query approach
because reasons stated at #1439


Thus, we've agreed to drop it meanwhile in order to have less
dead/unused/pending to adapt code.

Please, note that changes were done on top of
#1498, reason why would be nice to
merge this after it. Additionally, it includes a removal of software
typedef comments that were forgotten at
#1483 and
#1496.
@imobachgs imobachgs mentioned this pull request Jul 26, 2024
Merged
imobachgs added a commit that referenced this pull request Jul 29, 2024
@imobachgs
refactor(web): use queries to track progress (#1504)
305f35f 
Follow-up of #1439 the goal of this PR is to change the progress track
to use Tanstack Query.
@imobachgs imobachgs mentioned this pull request Jul 30, 2024
Merged
@dgdavid dgdavid mentioned this pull request Jul 31, 2024
Merged
imobachgs added a commit that referenced this pull request Aug 1, 2024
@imobachgs
refactor(web): use queries to track the installer status (#1514)
6965ccf 
Follow-up of #1439. The goal of this PR is to change installer status
tracking to Tanstack Query. Additionally, it adjust the HTTP API a bit.

## Notes

The `WithStatus` mixin should not be relevant anymore. However, the
storage UI still uses it to detect when the proposal changes. We should
use a dedicated event instead.
dgdavid added a commit that referenced this pull request Aug 2, 2024
@dgdavid
refactor(web): use queries for dealing with questions (#1516)
b6de3f1 
In the context of #1439, replace Questions client by its equivalent
using TanStack Query.

Additionally, it includes two commits with changes out of the scope of
the PBI we're working on. Namely:

* Migrates the _components/questions_ files to TypeScript,
560f953
* Stop re-exporting everything in _components/questions/index.ts_,
7c7b049

---

Related to https://trello.com/c/8u1WOJz4 (internal link)
@dgdavid dgdavid mentioned this pull request Aug 12, 2024
Merged
teclator added a commit that referenced this pull request Aug 16, 2024
@teclator
refactor(web): replace NetworkClient with queries (#1519)
d7eec2a 
Adapt the network management code to use queries instead of
NetworkCient. Related to #1439.

Additionally, starts migrating the code to TypeScript and fixes bugs
found by the type system.

---

Reviewers, please note that this ended up being a rather long PR, reason
why is better to not only review the code but also to do manual testing
if possible. Take your time, but any issue out of the scope of state
management / queries migration should be reported as a new issue to be
addressed in a new PBI.

---

Notes for creating follow-up work, 

* Fix WifiConnectionForm to avoid `"error": "Network system error:
Network state error: Connection '<ssid>' already exists"` when
connecting to hidden network that previously failed. In short, we should
either, to drop the configuration created for a failed { hidden: true }
connection or to implement a mechanism to know that such a configuration
already exists and performs an _update_ instead of _add_.
* Review when it's possible to define a gateway and fix the UI
information accordingly
* Check if we can use the type Address from ipaddr package instead of
our own IPAddress type (most probably not)
* Add support for IPv6 since as @jreidinger
[said](#1519 (comment))
its usage is becoming more common. Look for `FIXME: IPv6 is not
supported yet.` comments
@imobachgs imobachgs mentioned this pull request Sep 20, 2024
Merged
imobachgs added a commit that referenced this pull request Sep 20, 2024
@imobachgs
Update to release 10 (#1622)
574a1a5 
Prepare for releasing Agama 10·

* #1263
* #1330
* #1407
* #1408
* #1410
* #1411
* #1412
* #1416
* #1417
* #1419
* #1420
* #1421
* #1422
* #1423
* #1424
* #1425
* #1428
* #1429
* #1430
* #1431
* #1432
* #1433
* #1436
* #1437
* #1438
* #1439
* #1440
* #1441
* #1443
* #1444
* #1445
* #1449
* #1450
* #1451
* #1452
* #1453
* #1454
* #1455
* #1456
* #1457
* #1459
* #1460
* #1462
* #1464
* #1465
* #1466
* #1467
* #1468
* #1469
* #1470
* #1471
* #1472
* #1473
* #1475
* #1476
* #1477
* #1478
* #1479
* #1480
* #1481
* #1482
* #1483
* #1484
* #1485
* #1486
* #1487
* #1488
* #1489
* #1491
* #1492
* #1493
* #1494
* #1496
* #1497
* #1498
* #1499
* #1500
* #1501
* #1502
* #1503
* #1504
* #1505
* #1506
* #1507
* #1508
* #1510
* #1511
* #1512
* #1513
* #1514
* #1515
* #1516
* #1517
* #1518
* #1519
* #1520
* #1522
* #1523
* #1524
* #1525
* #1526
* #1527
* #1528
* #1529
* #1530
* #1531
* #1532
* #1533
* #1534
* #1535
* #1536
* #1537
* #1540
* #1541
* #1543
* #1544
* #1545
* #1546
* #1547
* #1548
* #1549
* #1550
* #1552
* #1553
* #1554
* #1555
* #1556
* #1557
* #1558
* #1559
* #1560
* #1562
* #1563
* #1565
* #1566
* #1567
* #1568
* #1569
* #1570
* #1571
* #1572
* #1573
* #1574
* #1575
* #1576
* #1577
* #1578
* #1579
* #1580
* #1581
* #1583
* #1584
* #1585
* #1586
* #1587
* #1588
* #1589
* #1590
* #1591
* #1592
* #1593
* #1596
* #1597
* #1598
* #1600
* #1602
* #1605
* #1606
* #1607
* #1608
* #1610
* #1611
* #1612
* #1613
* #1614
* #1619
* #1620
* #1621
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joseivanlopez joseivanlopez joseivanlopez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@imobachgs @joseivanlopez @dgdavid