Skip to content

Commit

Permalink
refactor(web): adopt TanStack Query for state management (#1439)
Browse files Browse the repository at this point in the history
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)](https://tanstack.com/query/v5).

## 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](https://reactrouter.com/), although a
[TanStack Router](https://tanstack.com/router/latest) 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](https://github.com/openSUSE/agama/blob/react-query/web/src/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

- [Redux](https://react-redux.js.org/)
- [Jotai](https://jotai.org/)
- [Zustand](https://docs.pmnd.rs/zustand/getting-started/introduction)
- [Tanstack Query (former React Query)](https://tanstack.com/query/v5)
- [Recoil](https://recoiljs.org/)

We had a look at others but did not consider them at all.
  • Loading branch information
imobachgs committed Jul 8, 2024
2 parents ad025f4 + 8838564 commit 05e7963
Show file tree
Hide file tree
Showing 31 changed files with 743 additions and 1,106 deletions.
5 changes: 3 additions & 2 deletions service/test/agama/software/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@
expect(products).to all(be_a(Agama::Software::Product))
expect(products).to contain_exactly(
an_object_having_attributes(id: "Tumbleweed"),
an_object_having_attributes(id: "MicroOS")
an_object_having_attributes(id: "MicroOS"),
an_object_having_attributes(id: "Leap_16.0")
)
end
end
Expand Down Expand Up @@ -335,7 +336,7 @@
expect(proposal).to receive(:set_resolvables)
.with("agama", :pattern, [], { optional: true })
expect(proposal).to receive(:set_resolvables)
.with("agama", :package, ["NetworkManager", "openSUSE-repos"])
.with("agama", :package, ["NetworkManager", "openSUSE-repos-Tumbleweed"])
expect(proposal).to receive(:set_resolvables)
.with("agama", :package, [], { optional: true })
subject.propose
Expand Down
25 changes: 25 additions & 0 deletions web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
"@patternfly/patternfly": "^5.1.0",
"@patternfly/react-core": "^5.1.1",
"@patternfly/react-table": "^5.1.1",
"@tanstack/react-query": "^5.49.2",
"fast-sort": "^3.4.0",
"ipaddr.js": "^2.1.0",
"react": "^18.2.0",
Expand Down
8 changes: 8 additions & 0 deletions web/package/agama-web-ui.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
-------------------------------------------------------------------
Mon Jul 8 10:56:14 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

- Introduce TanStack Query for data fetching and state management
(gh#openSUSE/agama#1439).
- Replace the l10n context with a solution based on TanStack
Query.

-------------------------------------------------------------------
Wed Jul 3 07:17:55 UTC 2024 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>

Expand Down
5 changes: 5 additions & 0 deletions web/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ import { useInstallerClientStatus } from "~/context/installer";
import { useProduct } from "./context/product";
import { CONFIG, INSTALL, STARTUP } from "~/client/phase";
import { BUSY } from "~/client/status";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import { useL10nConfigChanges } from "~/queries/l10n";

const queryClient = new QueryClient();

/**
* Main application component.
Expand All @@ -42,6 +46,7 @@ function App() {
const { connected, error, phase, status } = useInstallerClientStatus();
const { selectedProduct, products } = useProduct();
const { language } = useInstallerL10n();
useL10nConfigChanges();

const Content = () => {
if (error) return <ServerError />;
Expand Down
17 changes: 7 additions & 10 deletions web/src/App.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@
*/

import React from "react";
import { act, screen } from "@testing-library/react";
import { screen } from "@testing-library/react";
import { installerRender } from "~/test-utils";

import App from "./App";
import { createClient } from "~/client";
import { STARTUP, CONFIG, INSTALL } from "~/client/phase";
import { IDLE, BUSY } from "~/client/status";
import { useL10nConfigChanges } from "./queries/l10n";

jest.mock("~/client");

Expand All @@ -44,6 +45,11 @@ jest.mock("~/context/product", () => ({
}
}));

jest.mock("~/queries/l10n", () => ({
...jest.requireActual("~/queries/l10n"),
useL10nConfigChanges: () => jest.fn()
}));

const mockClientStatus = {
connected: true,
error: false,
Expand All @@ -70,18 +76,9 @@ describe("App", () => {
createClient.mockImplementation(() => {
return {
l10n: {
locales: jest.fn().mockResolvedValue([["en_us", "English", "United States"]]),
getLocales: jest.fn().mockResolvedValue(["en_us"]),
timezones: jest.fn().mockResolvedValue([]),
getTimezone: jest.fn().mockResolvedValue("Europe/Berlin"),
keymaps: jest.fn().mockResolvedValue([]),
getKeymap: jest.fn().mockResolvedValue(undefined),
getUIKeymap: jest.fn().mockResolvedValue("en"),
getUILocale: jest.fn().mockResolvedValue("en_us"),
setUILocale: jest.fn().mockResolvedValue("en_us"),
onTimezoneChange: jest.fn(),
onLocalesChange: jest.fn(),
onKeymapChange: jest.fn()
}
};
});
Expand Down
4 changes: 3 additions & 1 deletion web/src/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ const createClient = url => {

const isConnected = () => client.ws?.isConnected() || false;
const isRecoverable = () => !!client.ws?.isRecoverable();
const ws = () => client.ws;

return {
l10n,
Expand All @@ -145,7 +146,8 @@ const createClient = url => {
isConnected,
isRecoverable,
onConnect: handler => client.ws.onOpen(handler),
onDisconnect: handler => client.ws.onClose(handler)
onDisconnect: handler => client.ws.onClose(handler),
ws: () => client.ws
};
};

Expand Down
Loading

0 comments on commit 05e7963

Please sign in to comment.