Skip to content

refactor(web): use query key factories for safer and more consistent queries #2447

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

Closed
dgdavid wants to merge 1 commit into from
Closed

refactor(web): use query key factories for safer and more consistent queries #2447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions web/src/queries/issues.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,14 @@ const scopesFromPath = {
"/org/opensuse/Agama/Users1": "users",
};

const issuesKeys = {
all: () => ["issues"] as const,
byScope: (scope: IssuesScope) => [...issuesKeys.all(), scope] as const,
};

const issuesQuery = (scope: IssuesScope) => {
return {
queryKey: ["issues", scope],
queryKey: issuesKeys.byScope(scope),
queryFn: () => fetchIssues(scope),
};
};
Expand Down Expand Up @@ -77,7 +82,7 @@ const useIssuesChanges = () => {
const scope = scopesFromPath[path];
// TODO: use setQueryData because all the issues are included in the event
if (scope) {
queryClient.invalidateQueries({ queryKey: ["issues", scope] });
queryClient.invalidateQueries({ queryKey: issuesKeys.byScope(scope) });
} else {
console.warn(`Unknown scope ${path}`);
}
Expand Down
18 changes: 13 additions & 5 deletions web/src/queries/l10n.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,20 @@ import { useQueryClient, useMutation, useSuspenseQueries } from "@tanstack/react
import { useInstallerClient } from "~/context/installer";
import { fetchConfig, fetchKeymaps, fetchLocales, fetchTimezones, updateConfig } from "~/api/l10n";

const l10nKeys = {
all: () => ["l10n"] as const,
config: () => [...l10nKeys.all(), "config"] as const,
locales: () => [...l10nKeys.all(), "locales"] as const,
keymaps: () => [...l10nKeys.all(), "keymaps"] as const,
timezones: () => [...l10nKeys.all(), "timezones"] as const,
};

Comment on lines +28 to +35
Copy link
Copy Markdown
Contributor Author

@dgdavid dgdavid Jun 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Of course, would be more readable something like

const queryKeys = {
  all:       () => ["l10n"] as const,
  config:    () => ["l10n", "config"] as const,
  locales:   () => ["l10n", "locales"] as const,
  keymaps:   () => ["l10n", "keymaps"] as const,
  timezones: () => ["l10n", "timezones"] as const
}

or even,

const queryKeys = {
  all:       ["l10n"] as const,
  config:    ["l10n", "config"] as const,
  locales:   ["l10n", "locales"] as const,
  keymaps:   ["l10n", "keymaps"] as const,
  timezones: ["l10n", "timezones"] as const
}

But I'd go with the proposed approach of always using functions, to stay consistent with the rest of the query keys in the codebase. Some keys take parameters, and defining them all as functions keeps the API uniform and easier to work with.

About using composed keys ([...queryKeys.all(), "config"]) instead of hardcoding them (["l10n", "config"]), it also follows TanStack Query’s recommended practices. While it may slightly impact readability, it enables a layered composition strategy that simplifies future changes, especially when modifying top-level segments or adding new levels of nesting.

Copy link
Copy Markdown
Contributor Author

@dgdavid dgdavid Jun 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Above comment becomes more evident when looking at more complex query keys such as those in the software, network, or storage domains. There, the benefits of consistent structure and layered composition are much easier to appreciate.

/**
* Returns a query for retrieving the localization configuration
*/
const configQuery = () => {
return {
queryKey: ["l10n", "config"],
queryKey: l10nKeys.config(),
queryFn: fetchConfig,
};
};
Expand All @@ -39,7 +47,7 @@ const configQuery = () => {
* Returns a query for retrieving the list of known locales
*/
const localesQuery = () => ({
queryKey: ["l10n", "locales"],
queryKey: l10nKeys.locales(),
queryFn: fetchLocales,
staleTime: Infinity,
});
Expand All @@ -48,7 +56,7 @@ const localesQuery = () => ({
* Returns a query for retrieving the list of known timezones
*/
const timezonesQuery = () => ({
queryKey: ["l10n", "timezones"],
queryKey: l10nKeys.timezones(),
queryFn: fetchTimezones,
staleTime: Infinity,
});
Expand All @@ -57,7 +65,7 @@ const timezonesQuery = () => ({
* Returns a query for retrieving the list of known keymaps
*/
const keymapsQuery = () => ({
queryKey: ["l10n", "keymaps"],
queryKey: l10nKeys.keymaps(),
queryFn: fetchKeymaps,
staleTime: Infinity,
});
Expand Down Expand Up @@ -88,7 +96,7 @@ const useL10nConfigChanges = () => {

return client.onEvent((event) => {
if (event.type === "L10nConfigChanged") {
queryClient.invalidateQueries({ queryKey: ["l10n"] });
queryClient.invalidateQueries({ queryKey: l10nKeys.all() });
}
});
}, [client, queryClient]);
Expand Down
43 changes: 26 additions & 17 deletions web/src/queries/network.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,21 @@ import {
updateConnection,
} from "~/api/network";

const networkKeys = {
all: () => ["network"] as const,
state: () => [...networkKeys.all(), "state"] as const,
devices: () => [...networkKeys.all(), "devices"] as const,
connections: () => [...networkKeys.all(), "connections"] as const,
connection: (name: string) => [...networkKeys.connections(), name] as const,
accessPoints: () => [...networkKeys.all(), "accessPoints"] as const,
};

/**
* Returns a query for retrieving the general network configuration
*/
const stateQuery = () => {
return {
queryKey: ["network", "state"],
queryKey: networkKeys.state(),
queryFn: fetchState,
};
};
Expand All @@ -60,7 +69,7 @@ const stateQuery = () => {
* Returns a query for retrieving the list of known devices
*/
const devicesQuery = () => ({
queryKey: ["network", "devices"],
queryKey: networkKeys.devices(),
queryFn: async () => {
const devices = await fetchDevices();
return devices.map(Device.fromApi);
Expand All @@ -72,7 +81,7 @@ const devicesQuery = () => ({
* Returns a query for retrieving data for the given connection name
*/
const connectionQuery = (name: string) => ({
queryKey: ["network", "connections", name],
queryKey: networkKeys.connection(name),
queryFn: async () => {
const connection = await fetchConnection(name);
return Connection.fromApi(connection);
Expand All @@ -84,7 +93,7 @@ const connectionQuery = (name: string) => ({
* Returns a query for retrieving the list of known connections
*/
const connectionsQuery = () => ({
queryKey: ["network", "connections"],
queryKey: networkKeys.connections(),
queryFn: async () => {
const connections = await fetchConnections();
return connections.map(Connection.fromApi);
Expand All @@ -97,7 +106,7 @@ const connectionsQuery = () => ({
* the signal strength.
*/
const accessPointsQuery = () => ({
queryKey: ["network", "accessPoints"],
queryKey: networkKeys.accessPoints(),
queryFn: async (): Promise<AccessPoint[]> => {
const accessPoints = await fetchAccessPoints();
return accessPoints.map(AccessPoint.fromApi).sort((a, b) => b.strength - a.strength);
Expand All @@ -117,9 +126,9 @@ const useAddConnectionMutation = () => {
mutationFn: (newConnection: Connection) =>
addConnection(newConnection.toApi()).then(() => applyChanges()),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ["network", "connections"] });
queryClient.invalidateQueries({ queryKey: ["network", "devices"] });
queryClient.invalidateQueries({ queryKey: ["network", "accessPoints"] });
queryClient.invalidateQueries({ queryKey: networkKeys.connections() });
queryClient.invalidateQueries({ queryKey: networkKeys.devices() });
queryClient.invalidateQueries({ queryKey: networkKeys.accessPoints() });
},
};
return useMutation(query);
Expand All @@ -136,8 +145,8 @@ const useConnectionMutation = () => {
mutationFn: (newConnection: Connection) =>
updateConnection(newConnection.toApi()).then(() => applyChanges()),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ["network", "connections"] });
queryClient.invalidateQueries({ queryKey: ["network", "devices"] });
queryClient.invalidateQueries({ queryKey: networkKeys.connections() });
queryClient.invalidateQueries({ queryKey: networkKeys.devices() });
},
};
return useMutation(query);
Expand Down Expand Up @@ -173,7 +182,7 @@ const useConnectionPersistMutation = () => {
});

// Update the cached data with the optimistically updated connections
queryClient.setQueryData(["network", "connections"], updatedConnections);
queryClient.setQueryData(networkKeys.connections(), updatedConnections);

// Return the previous state for potential rollback
return { previousConnections };
Expand Down Expand Up @@ -203,8 +212,8 @@ const useRemoveConnectionMutation = () => {
.then(() => applyChanges())
.catch((e) => console.log(e)),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ["network", "connections"] });
queryClient.invalidateQueries({ queryKey: ["network", "devices"] });
queryClient.invalidateQueries({ queryKey: networkKeys.connections() });
queryClient.invalidateQueries({ queryKey: networkKeys.devices() });
},
};
return useMutation(query);
Expand All @@ -222,18 +231,18 @@ const useNetworkChanges = () => {

const updateDevices = useCallback(
(func: (devices: Device[]) => Device[]) => {
const devices: Device[] = queryClient.getQueryData(["network", "devices"]);
const devices: Device[] = queryClient.getQueryData(networkKeys.devices());
if (!devices) return;

const updatedDevices = func(devices);
queryClient.setQueryData(["network", "devices"], updatedDevices);
queryClient.setQueryData(networkKeys.devices(), updatedDevices);
},
[queryClient],
);

const updateConnectionState = useCallback(
(id: string, state: string) => {
const connections: Connection[] = queryClient.getQueryData(["network", "connections"]);
const connections: Connection[] = queryClient.getQueryData(networkKeys.connections());
if (!connections) return;

const updatedConnections = connections.map((conn) => {
Expand All @@ -244,7 +253,7 @@ const useNetworkChanges = () => {
}
return conn;
});
queryClient.setQueryData(["network", "connections"], updatedConnections);
queryClient.setQueryData(networkKeys.connections(), updatedConnections);
},
[queryClient],
);
Expand Down
13 changes: 9 additions & 4 deletions web/src/queries/progress.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ const servicesMap = {
"/org/opensuse/Agama/Storage1": "storage",
};

const progressKeys = {
all: () => ["progress"] as const,
byService: (service: string) => [...progressKeys.all(), service] as const,
};

/**
* Returns a query for retrieving the progress information for a given service
*
Expand All @@ -43,7 +48,7 @@ const servicesMap = {
*/
const progressQuery = (service: string) => {
return {
queryKey: ["progress", service],
queryKey: progressKeys.byService(service),
queryFn: () => fetchProgress(service),
};
};
Expand Down Expand Up @@ -83,12 +88,12 @@ const useProgressChanges = () => {
return;
}

const data = queryClient.getQueryData(["progress", service]);
const data = queryClient.getQueryData(progressKeys.byService(service));
if (data) {
// NOTE: steps are not coming in the updates
const steps = (data as Progress).steps;
const fromEvent = Progress.fromApi(event);
queryClient.setQueryData(["progress", service], { ...fromEvent, steps });
queryClient.setQueryData(progressKeys.byService(service), { ...fromEvent, steps });
}
}
});
Expand All @@ -106,7 +111,7 @@ const useResetProgress = () => {

React.useEffect(() => {
return () => {
queryClient.invalidateQueries({ queryKey: ["progress"] });
queryClient.invalidateQueries({ queryKey: progressKeys.all() });
};
}, [queryClient]);
};
Expand Down
8 changes: 6 additions & 2 deletions web/src/queries/questions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ import { useInstallerClient } from "~/context/installer";
import { Question } from "~/types/questions";
import { fetchQuestions, updateAnswer } from "~/api/questions";

const questionsKeys = {
all: () => ["questions"] as const,
};

/**
* Query to retrieve questions
*/
Expand Down Expand Up @@ -60,7 +64,7 @@ const useQuestionsChanges = () => {

return client.onEvent((event) => {
if (event.type === "QuestionsChanged") {
queryClient.invalidateQueries({ queryKey: ["questions"] });
queryClient.invalidateQueries({ queryKey: questionsKeys.all() });
}
});
}, [client, queryClient]);
Expand All @@ -69,7 +73,7 @@ const useQuestionsChanges = () => {
if (!client) return;

return client.onConnect(() => {
queryClient.invalidateQueries({ queryKey: ["questions"] });
queryClient.invalidateQueries({ queryKey: questionsKeys.all() });
});
}, [client, queryClient]);
};
Expand Down
Loading