Skip to content

Commit 0f2d4fd

Browse files
ParkreinerParkreiner
authored
fix: prevent metadata queries from short-circuiting (#10312)
* fix: prevent metadata queries from short-circuiting * fix: use correct type definitions
1 parent 8f39ec5 commit 0f2d4fd

File tree

8 files changed

+92
-44
lines changed

8 files changed

+92
-44
lines changed

‎site/src/api/queries/appearance.ts

+17-8
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,30 @@
1-
import { QueryClient } from "react-query";
1+
import { QueryClient, type UseQueryOptions } from "react-query";
22
import * as API from "api/api";
3-
import { AppearanceConfig } from "api/typesGenerated";
3+
import { type AppearanceConfig } from "api/typesGenerated";
44
import { getMetadataAsJSON } from "utils/metadata";
55

6-
export const appearance = () => {
6+
const initialAppearanceData = getMetadataAsJSON<AppearanceConfig>("appearance");
7+
const appearanceConfigKey = ["appearance"] as const;
8+
9+
export const appearance = (queryClient: QueryClient) => {
710
return {
8-
queryKey: ["appearance"],
9-
queryFn: async () =>
10-
getMetadataAsJSON<AppearanceConfig>("appearance") ?? API.getAppearance(),
11-
};
11+
queryKey: appearanceConfigKey,
12+
queryFn: async () => {
13+
const cachedData = queryClient.getQueryData(appearanceConfigKey);
14+
if (cachedData === undefined && initialAppearanceData !== undefined) {
15+
return initialAppearanceData;
16+
}
17+
18+
return API.getAppearance();
19+
},
20+
} satisfies UseQueryOptions<AppearanceConfig>;
1221
};
1322

1423
export const updateAppearance = (queryClient: QueryClient) => {
1524
return {
1625
mutationFn: API.updateAppearance,
1726
onSuccess: (newConfig: AppearanceConfig) => {
18-
queryClient.setQueryData(["appearance"], newConfig);
27+
queryClient.setQueryData(appearanceConfigKey, newConfig);
1928
},
2029
};
2130
};

‎site/src/api/queries/buildInfo.ts

+16-6
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,21 @@
1+
import { QueryClient, type UseQueryOptions } from "react-query";
2+
import { type BuildInfoResponse } from "api/typesGenerated";
13
import * as API from "api/api";
2-
import { BuildInfoResponse } from "api/typesGenerated";
34
import { getMetadataAsJSON } from "utils/metadata";
45

5-
export const buildInfo = () => {
6+
const initialBuildInfoData = getMetadataAsJSON<BuildInfoResponse>("build-info");
7+
const buildInfoKey = ["buildInfo"] as const;
8+
9+
export const buildInfo = (queryClient: QueryClient) => {
610
return {
7-
queryKey: ["buildInfo"],
8-
queryFn: async () =>
9-
getMetadataAsJSON<BuildInfoResponse>("build-info") ?? API.getBuildInfo(),
10-
};
11+
queryKey: buildInfoKey,
12+
queryFn: async () => {
13+
const cachedData = queryClient.getQueryData(buildInfoKey);
14+
if (cachedData === undefined && initialBuildInfoData !== undefined) {
15+
return initialBuildInfoData;
16+
}
17+
18+
return API.getBuildInfo();
19+
},
20+
} satisfies UseQueryOptions<BuildInfoResponse>;
1121
};

‎site/src/api/queries/experiments.ts

+16-6
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,21 @@
11
import * as API from "api/api";
2-
import { Experiments } from "api/typesGenerated";
32
import { getMetadataAsJSON } from "utils/metadata";
3+
import { type Experiments } from "api/typesGenerated";
4+
import { QueryClient, type UseQueryOptions } from "react-query";
45

5-
export const experiments = () => {
6+
const initialExperimentsData = getMetadataAsJSON<Experiments>("experiments");
7+
const experimentsKey = ["experiments"] as const;
8+
9+
export const experiments = (queryClient: QueryClient) => {
610
return {
7-
queryKey: ["experiments"],
8-
queryFn: async () =>
9-
getMetadataAsJSON<Experiments>("experiments") ?? API.getExperiments(),
10-
};
11+
queryKey: experimentsKey,
12+
queryFn: async () => {
13+
const cachedData = queryClient.getQueryData(experimentsKey);
14+
if (cachedData === undefined && initialExperimentsData !== undefined) {
15+
return initialExperimentsData;
16+
}
17+
18+
return API.getExperiments();
19+
},
20+
} satisfies UseQueryOptions<Experiments>;
1121
};

‎site/src/api/queries/users.ts

+16-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { QueryClient, QueryOptions } from "react-query";
1+
import { QueryClient, type UseQueryOptions } from "react-query";
22
import * as API from "api/api";
33
import {
44
AuthorizationRequest,
@@ -11,7 +11,7 @@ import {
1111
import { getMetadataAsJSON } from "utils/metadata";
1212
import { getAuthorizationKey } from "./authCheck";
1313

14-
export const users = (req: UsersRequest): QueryOptions<GetUsersResponse> => {
14+
export const users = (req: UsersRequest): UseQueryOptions<GetUsersResponse> => {
1515
return {
1616
queryKey: ["users", req],
1717
queryFn: ({ signal }) => API.getUsers(req, signal),
@@ -89,12 +89,21 @@ export const authMethods = () => {
8989
};
9090
};
9191

92-
export const me = () => {
92+
const initialMeData = getMetadataAsJSON<User>("user");
93+
const meKey = ["me"] as const;
94+
95+
export const me = (queryClient: QueryClient) => {
9396
return {
94-
queryKey: ["me"],
95-
queryFn: async () =>
96-
getMetadataAsJSON<User>("user") ?? API.getAuthenticatedUser(),
97-
};
97+
queryKey: meKey,
98+
queryFn: async () => {
99+
const cachedData = queryClient.getQueryData(meKey);
100+
if (cachedData === undefined && initialMeData !== undefined) {
101+
return initialMeData;
102+
}
103+
104+
return API.getAuthenticatedUser();
105+
},
106+
} satisfies UseQueryOptions<User>;
98107
};
99108

100109
export const hasFirstUser = () => {

‎site/src/components/AuthProvider/AuthProvider.tsx

+3-2
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ type AuthContextValue = {
4545
const AuthContext = createContext<AuthContextValue | undefined>(undefined);
4646

4747
export const AuthProvider: FC<PropsWithChildren> = ({ children }) => {
48-
const meOptions = me();
48+
const queryClient = useQueryClient();
49+
const meOptions = me(queryClient);
50+
4951
const userQuery = useQuery(meOptions);
5052
const authMethodsQuery = useQuery(authMethods());
5153
const hasFirstUserQuery = useQuery(hasFirstUser());
@@ -54,7 +56,6 @@ export const AuthProvider: FC<PropsWithChildren> = ({ children }) => {
5456
enabled: userQuery.data !== undefined,
5557
});
5658

57-
const queryClient = useQueryClient();
5859
const loginMutation = useMutation(
5960
login({ checks: permissionsToCheck }, queryClient),
6061
);

‎site/src/components/Dashboard/DashboardProvider.tsx

+7-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useQuery } from "react-query";
1+
import { useQuery, useQueryClient } from "react-query";
22
import { buildInfo } from "api/queries/buildInfo";
33
import { experiments } from "api/queries/experiments";
44
import { entitlements } from "api/queries/entitlements";
@@ -30,19 +30,21 @@ interface Appearance {
3030
interface DashboardProviderValue {
3131
buildInfo: BuildInfoResponse;
3232
entitlements: Entitlements;
33-
appearance: Appearance;
3433
experiments: Experiments;
34+
appearance: Appearance;
3535
}
3636

3737
export const DashboardProviderContext = createContext<
3838
DashboardProviderValue | undefined
3939
>(undefined);
4040

4141
export const DashboardProvider: FC<PropsWithChildren> = ({ children }) => {
42-
const buildInfoQuery = useQuery(buildInfo());
42+
const queryClient = useQueryClient();
43+
const buildInfoQuery = useQuery(buildInfo(queryClient));
4344
const entitlementsQuery = useQuery(entitlements());
44-
const experimentsQuery = useQuery(experiments());
45-
const appearanceQuery = useQuery(appearance());
45+
const experimentsQuery = useQuery(experiments(queryClient));
46+
const appearanceQuery = useQuery(appearance(queryClient));
47+
4648
const isLoading =
4749
!buildInfoQuery.data ||
4850
!entitlementsQuery.data ||

‎site/src/theme/colors.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import { getMetadataAsJSON } from "utils/metadata";
66
// so you can just set this to true.
77
export const experimentalTheme =
88
typeof document !== "undefined" &&
9-
getMetadataAsJSON("experiments")?.includes("dashboard_theme");
9+
(getMetadataAsJSON<string[]>("experiments")?.includes("dashboard_theme") ??
10+
false);
1011

1112
export const colors = {
1213
white: "hsl(0, 0%, 100%)",

‎site/src/utils/metadata.ts

+15-9
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,24 @@
1-
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- It can be any
2-
export const getMetadataAsJSON = <T extends Record<string, any>>(
1+
export const getMetadataAsJSON = <T extends NonNullable<unknown>>(
32
property: string,
43
): T | undefined => {
54
const appearance = document.querySelector(`meta[property=${property}]`);
5+
66
if (appearance) {
77
const rawContent = appearance.getAttribute("content");
8-
try {
9-
return JSON.parse(rawContent as string);
10-
} catch (ex) {
11-
// In development the metadata is always going to be empty throwing this
12-
// error
13-
if (process.env.NODE_ENV === "production") {
14-
console.warn(`Failed to parse ${property} metadata`);
8+
9+
if (rawContent) {
10+
try {
11+
return JSON.parse(rawContent);
12+
} catch (err) {
13+
// In development, the metadata is always going to be empty; error is
14+
// only a concern for production
15+
if (process.env.NODE_ENV === "production") {
16+
console.warn(`Failed to parse ${property} metadata. Error message:`);
17+
console.warn(err);
18+
}
1519
}
1620
}
1721
}
22+
23+
return undefined;
1824
};

0 commit comments

Comments
 (0)