Skip to content

Commit 7d1261a

Browse files
committed
Revert "Refactor useUrlValueProvider (#6269)"
This reverts commit 944569d.
1 parent 42d137d commit 7d1261a

1 file changed

Lines changed: 117 additions & 123 deletions

File tree

Lines changed: 117 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,43 @@
1+
/* eslint-disable react-hooks/exhaustive-deps */
12
import { stringify } from "qs";
2-
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
3+
import { useEffect, useMemo, useState } from "react";
34
import useRouter from "use-react-router";
45

6+
import { InitialAttributesAPIState } from "../API/initialState/attributes/useInitialAttributesState";
7+
import { InitialCollectionAPIState } from "../API/initialState/collections/useInitialCollectionsState";
8+
import { InitialGiftCardsAPIState } from "../API/initialState/giftCards/useInitialGiftCardsState";
9+
import { InitialOrderAPIState } from "../API/initialState/orders/useInitialOrderState";
10+
import { InitialPageAPIState } from "../API/initialState/page/useInitialPageState";
11+
import { InitialProductAPIState } from "../API/initialState/product/useProductInitialAPIState";
12+
import { InitialProductTypesAPIState } from "../API/initialState/productTypes/useInitialProdutTypesState";
13+
import { InitialStaffMembersAPIState } from "../API/initialState/staffMembers/useInitialStaffMemebersState";
14+
import { InitialVoucherAPIState } from "../API/initialState/vouchers/useInitialVouchersState";
515
import { FilterContainer, FilterElement } from "../FilterElement";
616
import { FilterValueProvider } from "../FilterValueProvider";
717
import { FilterProviderType, InitialAPIState } from "../types";
818
import { TokenArray } from "./TokenArray";
9-
import { getEmptyFetchingPrams } from "./TokenArray/fetchingParams";
19+
import {
20+
AttributesFetchingParams,
21+
CollectionFetchingParams,
22+
FetchingParams,
23+
getEmptyFetchingPrams,
24+
GiftCardsFetchingParams,
25+
OrderFetchingParams,
26+
PageFetchingParams,
27+
ProductTypesFetchingParams,
28+
StaffMembersFetchingParams,
29+
VoucherFetchingParams,
30+
} from "./TokenArray/fetchingParams";
1031
import { prepareStructure } from "./utils";
1132

12-
/*
13-
* Race condition fix for URL ↔ React state synchronization.
14-
*
15-
* Problem: Filter value has two sources:
16-
* 1. Rehydration: computed from URL params + fetched GraphQL data
17-
* 2. User action: when user selects a filter, we update URL and set value directly
18-
*
19-
* When user selects a filter:
20-
* - persist() calls router.history.replace() which changes the URL
21-
* - URL change causes tokenizedUrl/rehydratedValue to recompute
22-
* - But GraphQL data is stale (from before the selection)
23-
* - So rehydratedValue overwrites user's selection with old data
24-
*
25-
* Solution: UserOverride stores user's selection + URL snapshot.
26-
* When computing final value, we check if URL still matches the snapshot.
27-
* If yes → user just set this, use their value.
28-
* If no → URL changed externally, use rehydrated value.
29-
*/
30-
interface UserOverride {
31-
urlSnapshot: string;
32-
value: FilterContainer;
33-
}
34-
3533
export const useUrlValueProvider = (
3634
locationSearch: string,
3735
type: FilterProviderType,
3836
initialState?: InitialAPIState,
3937
): FilterValueProvider => {
4038
const router = useRouter();
4139
const params = new URLSearchParams(locationSearch);
42-
const [userOverride, setUserOverride] = useState<UserOverride | null>(null);
40+
const [value, setValue] = useState<FilterContainer>([]);
4341
const activeTab = params.get("activeTab");
4442
const query = params.get("query");
4543
const before = params.get("before");
@@ -52,119 +50,115 @@ export const useUrlValueProvider = (
5250
params.delete("before");
5351
params.delete("after");
5452

55-
const filterQueryString = params.toString();
56-
const tokenizedUrl = useMemo(() => new TokenArray(filterQueryString), [filterQueryString]);
53+
const tokenizedUrl = useMemo(() => new TokenArray(params.toString()), [params.toString()]);
5754
const paramsFromType = getEmptyFetchingPrams(type);
5855
const fetchingParams = paramsFromType
5956
? tokenizedUrl.getFetchingParams(paramsFromType, type)
6057
: null;
6158

62-
const data = initialState?.data;
63-
const loading = initialState?.loading ?? false;
64-
65-
// Store fetchQueries in a ref to avoid triggering useEffect when initialState changes
66-
// but the function reference is the same (common with hooks returning stable callbacks).
67-
// Type assertion needed because InitialAPIState is a union type where each variant
68-
// has fetchQueries accepting different params. The correlation between `type` and
69-
// `initialState` ensures the correct params are passed, but TS can't narrow the union.
70-
const fetchQueriesRef = useRef<((params: typeof fetchingParams) => Promise<void>) | null>(null);
71-
7259
useEffect(() => {
73-
fetchQueriesRef.current = initialState?.fetchQueries as
74-
| ((params: typeof fetchingParams) => Promise<void>)
75-
| null;
76-
}, [initialState?.fetchQueries]);
60+
if (initialState) {
61+
switch (type) {
62+
case "product":
63+
(initialState as InitialProductAPIState).fetchQueries(fetchingParams as FetchingParams);
64+
break;
65+
case "order":
66+
(initialState as InitialOrderAPIState).fetchQueries(
67+
fetchingParams as OrderFetchingParams,
68+
);
69+
break;
70+
case "voucher":
71+
(initialState as InitialVoucherAPIState).fetchQueries(
72+
fetchingParams as VoucherFetchingParams,
73+
);
74+
break;
75+
case "page":
76+
(initialState as InitialPageAPIState).fetchQueries(fetchingParams as PageFetchingParams);
77+
break;
78+
case "gift-cards":
79+
(initialState as InitialGiftCardsAPIState).fetchQueries(
80+
fetchingParams as GiftCardsFetchingParams,
81+
);
82+
break;
83+
case "collection":
84+
(initialState as InitialCollectionAPIState).fetchQueries(
85+
fetchingParams as CollectionFetchingParams,
86+
);
87+
break;
88+
case "product-types":
89+
(initialState as InitialProductTypesAPIState).fetchQueries(
90+
fetchingParams as ProductTypesFetchingParams,
91+
);
92+
break;
93+
case "staff-members":
94+
(initialState as InitialStaffMembersAPIState).fetchQueries(
95+
fetchingParams as StaffMembersFetchingParams,
96+
);
97+
break;
98+
case "attributes":
99+
(initialState as InitialAttributesAPIState).fetchQueries(
100+
fetchingParams as AttributesFetchingParams,
101+
);
102+
break;
103+
}
104+
}
105+
}, [locationSearch]);
77106

78107
useEffect(() => {
79-
if (!fetchQueriesRef.current || !fetchingParams) return;
108+
if (!initialState) return;
80109

81-
fetchQueriesRef.current(fetchingParams);
82-
}, [fetchingParams]);
110+
const { data, loading } = initialState;
83111

84-
const rehydratedValue = useMemo((): FilterContainer => {
85-
if (initialState) {
86-
if (loading || !data) {
87-
return [];
88-
}
112+
if (loading) return;
89113

90-
return tokenizedUrl.asFilterValuesFromResponse(data);
91-
}
114+
setValue(tokenizedUrl.asFilterValuesFromResponse(data));
115+
// Only run after fetching the initial data; otherwise, a race condition may occur.
116+
}, [initialState?.data, initialState?.loading]);
92117

93-
return tokenizedUrl.asFilterValueFromEmpty();
94-
}, [initialState, loading, data, tokenizedUrl]);
95-
96-
const value = useMemo((): FilterContainer => {
97-
if (userOverride && userOverride.urlSnapshot === filterQueryString) {
98-
return userOverride.value;
99-
}
118+
useEffect(() => {
119+
if (initialState) return;
100120

101-
return rehydratedValue;
102-
}, [userOverride, filterQueryString, rehydratedValue]);
121+
setValue(tokenizedUrl.asFilterValueFromEmpty());
122+
}, [locationSearch, tokenizedUrl, initialState]);
103123

104-
const persist = useCallback(
105-
(filterValue: FilterContainer) => {
106-
const newParams = {
107-
...prepareStructure(filterValue),
108-
...(activeTab ? { activeTab } : {}),
109-
...(query ? { query } : {}),
110-
...(before ? { before } : {}),
111-
...(after ? { after } : {}),
112-
};
113-
114-
router.history.replace({
115-
pathname: router.location.pathname,
116-
search: stringify(newParams),
117-
});
118-
119-
const filterStructureParams = prepareStructure(filterValue);
120-
const newUrlSnapshot = Object.entries(filterStructureParams)
121-
.map(([k, v]) => `${k}=${encodeURIComponent(String(v))}`)
122-
.join("&");
123-
124-
setUserOverride({
125-
urlSnapshot: newUrlSnapshot,
126-
value: filterValue,
127-
});
128-
},
129-
[router, activeTab, query, before, after],
130-
);
131-
132-
const clear = useCallback(() => {
124+
const persist = (filterValue: FilterContainer) => {
133125
router.history.replace({
134126
pathname: router.location.pathname,
127+
search: stringify({
128+
...prepareStructure(filterValue),
129+
...{ activeTab: activeTab || undefined },
130+
...{ query: query || undefined },
131+
...{ before: before || undefined },
132+
...{ after: after || undefined },
133+
}),
135134
});
136-
setUserOverride({
137-
urlSnapshot: "",
138-
value: [],
135+
setValue(filterValue);
136+
};
137+
138+
const clear = () => {
139+
router.history.replace({
140+
pathname: router.location.pathname,
139141
});
140-
}, [router]);
141-
142-
const isPersisted = useCallback(
143-
(element: FilterElement) => {
144-
return value.some(p => FilterElement.isFilterElement(p) && p.equals(element));
145-
},
146-
[value],
147-
);
148-
149-
const getTokenByName = useCallback(
150-
(name: string) => {
151-
return tokenizedUrl.asFlatArray().find(token => token.name === name);
152-
},
153-
[tokenizedUrl],
154-
);
155-
156-
const count = useMemo(() => value.filter(v => typeof v !== "string").length, [value]);
157-
158-
return useMemo(
159-
() => ({
160-
value,
161-
loading: initialState?.loading || false,
162-
persist,
163-
clear,
164-
isPersisted,
165-
getTokenByName,
166-
count,
167-
}),
168-
[value, initialState?.loading, persist, clear, isPersisted, getTokenByName, count],
169-
);
142+
setValue([]);
143+
};
144+
145+
const isPersisted = (element: FilterElement) => {
146+
return value.some(p => FilterElement.isFilterElement(p) && p.equals(element));
147+
};
148+
149+
const getTokenByName = (name: string) => {
150+
return tokenizedUrl.asFlatArray().find(token => token.name === name);
151+
};
152+
153+
const count = value.filter(v => typeof v !== "string").length;
154+
155+
return {
156+
value,
157+
loading: initialState?.loading || false,
158+
persist,
159+
clear,
160+
isPersisted,
161+
getTokenByName,
162+
count,
163+
};
170164
};

0 commit comments

Comments
 (0)