Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7873c96

Browse files
authored
fix: ensure signing out cannot cause any runtime render errors (#13137)
* fix: remove some of the jank around our core App component* refactor: scope navigation logic more aggressively* refactor: add explicit return type to useAuthenticated* refactor: clean up ProxyContext code* wip: add code for consolidating the HTML metadata* refactor: clean up hook logic* refactor: rename useHtmlMetadata to useEmbeddedMetadata* fix: correct names that weren't updated* fix: update type-safety of useEmbeddedMetadata further* wip: switch codebase to use metadata hook* refactor: simplify design of metadata hook* fix: update stray type mismatches* fix: more type fixing* fix: resolve illegal invocation error* fix: get metadata issue resolved* fix: update comments* chore: add unit tests for MetadataManager* fix: beef up tests* fix: update typo in tests
1 parented0ca76 commit7873c96

File tree

20 files changed

+665
-142
lines changed

20 files changed

+665
-142
lines changed

‎site/src/App.tsx

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,23 @@ export const AppProviders: FC<AppProvidersProps> = ({
3838
})=>{
3939
// https://tanstack.com/query/v4/docs/react/devtools
4040
const[showDevtools,setShowDevtools]=useState(false);
41+
4142
useEffect(()=>{
42-
window.toggleDevtools=()=>setShowDevtools((old)=>!old);
43-
// eslint-disable-next-line react-hooks/exhaustive-deps -- no dependencies needed here
43+
// Storing key in variable to avoid accidental typos; we're working with the
44+
// window object, so there's basically zero type-checking available
45+
consttoggleKey="toggleDevtools";
46+
47+
// Don't want to throw away the previous devtools value if some other
48+
// extension added something already
49+
constdevtoolsBeforeSync=window[toggleKey];
50+
window[toggleKey]=()=>{
51+
devtoolsBeforeSync?.();
52+
setShowDevtools((current)=>!current);
53+
};
54+
55+
return()=>{
56+
window[toggleKey]=devtoolsBeforeSync;
57+
};
4458
},[]);
4559

4660
return(
@@ -60,10 +74,10 @@ export const AppProviders: FC<AppProvidersProps> = ({
6074

6175
exportconstApp:FC=()=>{
6276
return(
63-
<AppProviders>
64-
<ErrorBoundary>
77+
<ErrorBoundary>
78+
<AppProviders>
6579
<RouterProviderrouter={router}/>
66-
</ErrorBoundary>
67-
</AppProviders>
80+
</AppProviders>
81+
</ErrorBoundary>
6882
);
6983
};

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
1-
importtype{QueryClient,UseQueryOptions}from"react-query";
1+
importtype{QueryClient}from"react-query";
22
import*asAPIfrom"api/api";
33
importtype{AppearanceConfig}from"api/typesGenerated";
4-
import{getMetadataAsJSON}from"utils/metadata";
4+
importtype{MetadataState}from"hooks/useEmbeddedMetadata";
55
import{cachedQuery}from"./util";
66

7-
constinitialAppearanceData=getMetadataAsJSON<AppearanceConfig>("appearance");
87
constappearanceConfigKey=["appearance"]asconst;
98

10-
exportconstappearance=():UseQueryOptions<AppearanceConfig>=>{
11-
// We either have our initial data or should immediately fetch and never again!
9+
exportconstappearance=(metadata:MetadataState<AppearanceConfig>)=>{
1210
returncachedQuery({
13-
initialData:initialAppearanceData,
11+
metadata,
1412
queryKey:["appearance"],
1513
queryFn:()=>API.getAppearance(),
1614
});

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
1-
importtype{UseQueryOptions}from"react-query";
21
import*asAPIfrom"api/api";
32
importtype{BuildInfoResponse}from"api/typesGenerated";
4-
import{getMetadataAsJSON}from"utils/metadata";
3+
importtype{MetadataState}from"hooks/useEmbeddedMetadata";
54
import{cachedQuery}from"./util";
65

7-
constinitialBuildInfoData=getMetadataAsJSON<BuildInfoResponse>("build-info");
86
constbuildInfoKey=["buildInfo"]asconst;
97

10-
exportconstbuildInfo=():UseQueryOptions<BuildInfoResponse>=>{
8+
exportconstbuildInfo=(metadata:MetadataState<BuildInfoResponse>)=>{
119
// The version of the app can't change without reloading the page.
1210
returncachedQuery({
13-
initialData:initialBuildInfoData,
11+
metadata,
1412
queryKey:buildInfoKey,
1513
queryFn:()=>API.getBuildInfo(),
1614
});

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
1-
importtype{QueryClient,UseQueryOptions}from"react-query";
1+
importtype{QueryClient}from"react-query";
22
import*asAPIfrom"api/api";
33
importtype{Entitlements}from"api/typesGenerated";
4-
import{getMetadataAsJSON}from"utils/metadata";
4+
importtype{MetadataState}from"hooks/useEmbeddedMetadata";
55
import{cachedQuery}from"./util";
66

7-
constinitialEntitlementsData=getMetadataAsJSON<Entitlements>("entitlements");
87
constentitlementsQueryKey=["entitlements"]asconst;
98

10-
exportconstentitlements=():UseQueryOptions<Entitlements>=>{
9+
exportconstentitlements=(metadata:MetadataState<Entitlements>)=>{
1110
returncachedQuery({
12-
initialData:initialEntitlementsData,
11+
metadata,
1312
queryKey:entitlementsQueryKey,
1413
queryFn:()=>API.getEntitlements(),
1514
});

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
1-
importtype{UseQueryOptions}from"react-query";
21
import*asAPIfrom"api/api";
32
importtype{Experiments}from"api/typesGenerated";
4-
import{getMetadataAsJSON}from"utils/metadata";
3+
importtype{MetadataState}from"hooks/useEmbeddedMetadata";
54
import{cachedQuery}from"./util";
65

7-
constinitialExperimentsData=getMetadataAsJSON<Experiments>("experiments");
86
constexperimentsKey=["experiments"]asconst;
97

10-
exportconstexperiments=():UseQueryOptions<Experiments>=>{
8+
exportconstexperiments=(metadata:MetadataState<Experiments>)=>{
119
returncachedQuery({
12-
initialData:initialExperimentsData,
10+
metadata,
1311
queryKey:experimentsKey,
1412
queryFn:()=>API.getExperiments(),
1513
});

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

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
importtype{
22
QueryClient,
3-
QueryKey,
43
UseMutationOptions,
54
UseQueryOptions,
65
}from"react-query";
@@ -15,9 +14,12 @@ import type {
1514
User,
1615
GenerateAPIKeyResponse,
1716
}from"api/typesGenerated";
17+
import{
18+
defaultMetadataManager,
19+
typeMetadataState,
20+
}from"hooks/useEmbeddedMetadata";
1821
importtype{UsePaginatedQueryOptions}from"hooks/usePaginatedQuery";
1922
import{prepareQuery}from"utils/filters";
20-
import{getMetadataAsJSON}from"utils/metadata";
2123
import{getAuthorizationKey}from"./authCheck";
2224
import{cachedQuery}from"./util";
2325

@@ -113,8 +115,6 @@ export const updateRoles = (queryClient: QueryClient) => {
113115
};
114116
};
115117

116-
constinitialUserData=getMetadataAsJSON<User>("user");
117-
118118
exportconstauthMethods=()=>{
119119
return{
120120
// Even the endpoint being /users/authmethods we don't want to revalidate it
@@ -126,11 +126,9 @@ export const authMethods = () => {
126126

127127
constmeKey=["me"];
128128

129-
exportconstme=():UseQueryOptions<User>&{
130-
queryKey:QueryKey;
131-
}=>{
129+
exportconstme=(metadata:MetadataState<User>)=>{
132130
returncachedQuery({
133-
initialData:initialUserData,
131+
metadata,
134132
queryKey:meKey,
135133
queryFn:API.getAuthenticatedUser,
136134
});
@@ -143,10 +141,9 @@ export function apiKey(): UseQueryOptions<GenerateAPIKeyResponse> {
143141
};
144142
}
145143

146-
exportconsthasFirstUser=():UseQueryOptions<boolean>=>{
144+
exportconsthasFirstUser=(userMetadata:MetadataState<User>)=>{
147145
returncachedQuery({
148-
// This cannot be false otherwise it will not fetch!
149-
initialData:Boolean(initialUserData)||undefined,
146+
metadata:userMetadata,
150147
queryKey:["hasFirstUser"],
151148
queryFn:API.hasFirstUser,
152149
});
@@ -193,6 +190,22 @@ export const logout = (queryClient: QueryClient) => {
193190
return{
194191
mutationFn:API.logout,
195192
onSuccess:()=>{
193+
/**
194+
* 2024-05-02 - If we persist any form of user data after the user logs
195+
* out, that will continue to seed the React Query cache, creating
196+
* "impossible" states where we'll have data we're not supposed to have.
197+
*
198+
* This has caused issues where logging out will instantly throw a
199+
* completely uncaught runtime rendering error. Worse yet, the error only
200+
* exists when serving the site from the Go backend (the JS environment
201+
* has zero issues because it doesn't have access to the metadata). These
202+
* errors can only be caught with E2E tests.
203+
*
204+
* Deleting the user data will mean that all future requests have to take
205+
* a full roundtrip, but this still felt like the best way to ensure that
206+
* manually logging out doesn't blow the entire app up.
207+
*/
208+
defaultMetadataManager.clearMetadataByKey("user");
196209
queryClient.removeQueries();
197210
},
198211
};

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

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,74 @@
1-
importtype{UseQueryOptions}from"react-query";
1+
importtype{UseQueryOptions,QueryKey}from"react-query";
2+
importtype{MetadataState,MetadataValue}from"hooks/useEmbeddedMetadata";
3+
4+
constdisabledFetchOptions={
5+
cacheTime:Infinity,
6+
staleTime:Infinity,
7+
refetchOnMount:false,
8+
refetchOnReconnect:false,
9+
refetchOnWindowFocus:false,
10+
}asconstsatisfiesUseQueryOptions;
11+
12+
typeUseQueryOptionsWithMetadata<
13+
TMetadataextendsMetadataValue=MetadataValue,
14+
TQueryFnData=unknown,
15+
TError=unknown,
16+
TData=TQueryFnData,
17+
TQueryKeyextendsQueryKey=QueryKey,
18+
>=Omit<
19+
UseQueryOptions<TQueryFnData,TError,TData,TQueryKey>,
20+
"initialData"
21+
>&{
22+
metadata:MetadataState<TMetadata>;
23+
};
24+
25+
typeFormattedQueryOptionsResult<
26+
TQueryFnData=unknown,
27+
TError=unknown,
28+
TData=TQueryFnData,
29+
TQueryKeyextendsQueryKey=QueryKey,
30+
>=Omit<
31+
UseQueryOptions<TQueryFnData,TError,TData,TQueryKey>,
32+
"initialData"
33+
>&{
34+
queryKey:NonNullable<TQueryKey>;
35+
};
236

337
/**
438
* cachedQuery allows the caller to only make a request a single time, and use
539
* `initialData` if it is provided. This is particularly helpful for passing
640
* values injected via metadata. We do this for the initial user fetch,
741
* buildinfo, and a few others to reduce page load time.
842
*/
9-
exportconstcachedQuery=<
10-
TQueryOptionsextendsUseQueryOptions<TData>,
11-
TData,
43+
exportfunctioncachedQuery<
44+
TMetadataextendsMetadataValue=MetadataValue,
45+
TQueryFnData=unknown,
46+
TError=unknown,
47+
TData=TQueryFnData,
48+
TQueryKeyextendsQueryKey=QueryKey,
1249
>(
13-
options:TQueryOptions,
14-
):TQueryOptions=>
15-
// Only do this if there is initial data, otherwise it can conflict with tests.
16-
({
17-
...(options.initialData
18-
?{
19-
cacheTime:Infinity,
20-
staleTime:Infinity,
21-
refetchOnMount:false,
22-
refetchOnReconnect:false,
23-
refetchOnWindowFocus:false,
24-
}
25-
:{}),
26-
...options,
27-
});
50+
options:UseQueryOptionsWithMetadata<
51+
TMetadata,
52+
TQueryFnData,
53+
TError,
54+
TData,
55+
TQueryKey
56+
>,
57+
):FormattedQueryOptionsResult<TQueryFnData,TError,TData,TQueryKey>{
58+
const{ metadata, ...delegatedOptions}=options;
59+
constnewOptions={
60+
...delegatedOptions,
61+
initialData:metadata.available ?metadata.value :undefined,
62+
63+
// Make sure the disabled options are always serialized last, so that no
64+
// one using this function can accidentally override the values
65+
...(metadata.available ?disabledFetchOptions :{}),
66+
};
67+
68+
returnnewOptionsasFormattedQueryOptionsResult<
69+
TQueryFnData,
70+
TError,
71+
TData,
72+
TQueryKey
73+
>;
74+
}

‎site/src/contexts/ProxyContext.tsx

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { getWorkspaceProxies, getWorkspaceProxyRegions } from "api/api";
1212
import{cachedQuery}from"api/queries/util";
1313
importtype{Region,WorkspaceProxy}from"api/typesGenerated";
1414
import{useAuthenticated}from"contexts/auth/RequireAuth";
15+
import{useEmbeddedMetadata}from"hooks/useEmbeddedMetadata";
1516
import{typeProxyLatencyReport,useProxyLatency}from"./useProxyLatency";
1617

1718
exportinterfaceProxyContextValue{
@@ -94,37 +95,8 @@ export const ProxyProvider: FC<PropsWithChildren> = ({ children }) => {
9495
computeUsableURLS(userSavedProxy),
9596
);
9697

97-
constqueryKey=["get-proxies"];
98-
// This doesn't seem like an idiomatic way to get react-query to use the
99-
// initial data without performing an API request on mount, but it works.
100-
//
101-
// If anyone would like to clean this up in the future, it should seed data
102-
// from the `meta` tag if it exists, and not fetch the regions route.
103-
const[initialData]=useState(()=>{
104-
// Build info is injected by the Coder server into the HTML document.
105-
constregions=document.querySelector("meta[property=regions]");
106-
if(regions){
107-
constrawContent=regions.getAttribute("content");
108-
try{
109-
constobj=JSON.parse(rawContentasstring);
110-
if("regions"inobj){
111-
returnobj.regionsasRegion[];
112-
}
113-
returnobjasRegion[];
114-
}catch(ex){
115-
// Ignore this and fetch as normal!
116-
}
117-
}
118-
});
119-
12098
const{ permissions}=useAuthenticated();
121-
constquery=async():Promise<readonlyRegion[]>=>{
122-
constendpoint=permissions.editWorkspaceProxies
123-
?getWorkspaceProxies
124-
:getWorkspaceProxyRegions;
125-
constresp=awaitendpoint();
126-
returnresp.regions;
127-
};
99+
const{ metadata}=useEmbeddedMetadata();
128100

129101
const{
130102
data:proxiesResp,
@@ -133,9 +105,15 @@ export const ProxyProvider: FC<PropsWithChildren> = ({ children }) => {
133105
isFetched:proxiesFetched,
134106
}=useQuery(
135107
cachedQuery({
136-
initialData,
137-
queryKey,
138-
queryFn:query,
108+
metadata:metadata.regions,
109+
queryKey:["get-proxies"],
110+
queryFn:async():Promise<readonlyRegion[]>=>{
111+
constendpoint=permissions.editWorkspaceProxies
112+
?getWorkspaceProxies
113+
:getWorkspaceProxyRegions;
114+
constresp=awaitendpoint();
115+
returnresp.regions;
116+
},
139117
}),
140118
);
141119

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp