Skip to content

Commit 1e1993b

Browse files
prc5prc5
authored
Fix/issue 94 (#96)
* fix: 🐛 State processing race condition ✅ Closes: #94 * refactor: 💡 bring back old example code * fix: 🐛 deps * refactor: 💡 adjusted keys comparison
1 parent 9fe13e5 commit 1e1993b

File tree

13 files changed

+306
-94
lines changed

13 files changed

+306
-94
lines changed

‎.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ coverage
5151

5252
# Next.js
5353
.next
54+
.nx
5455

5556
# Firebase
5657
database-debug.log

‎examples/reactjs/src/main.tsx

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import React from "react";
12
import { Stack } from "@mui/material";
23
import * as ReactDOM from "react-dom/client";
34
import { SnackbarProvider } from "notistack";
@@ -11,23 +12,25 @@ import DashboardPage from "pages";
1112

1213
const root = ReactDOM.createRoot(document.getElementById("root") as HTMLElement);
1314
root.render(
14-
<SnackbarProvider
15-
maxSnack={6}
16-
autoHideDuration={1000}
17-
anchorOrigin={{
18-
vertical: "top",
19-
horizontal: "right",
20-
}}
21-
>
22-
<BrowserRouter>
23-
<Stack direction="row">
24-
<Routes>
25-
<Route path={DASHBOARD_PAGE.path} element={<DashboardPage />} />
26-
<Route path={DETAILS_PAGE.path} element={<DetailsPage />} />
27-
<Route path={LIST_PAGE.path} element={<ListPage />} />
28-
<Route path={FORM_PAGE.path} element={<FormPage />} />
29-
</Routes>
30-
</Stack>
31-
</BrowserRouter>
32-
</SnackbarProvider>,
15+
<React.StrictMode>
16+
<SnackbarProvider
17+
maxSnack={6}
18+
autoHideDuration={1000}
19+
anchorOrigin={{
20+
vertical: "top",
21+
horizontal: "right",
22+
}}
23+
>
24+
<BrowserRouter>
25+
<Stack direction="row">
26+
<Routes>
27+
<Route path={DASHBOARD_PAGE.path} element={<DashboardPage />} />
28+
<Route path={DETAILS_PAGE.path} element={<DetailsPage />} />
29+
<Route path={LIST_PAGE.path} element={<ListPage />} />
30+
<Route path={FORM_PAGE.path} element={<FormPage />} />
31+
</Routes>
32+
</Stack>
33+
</BrowserRouter>
34+
</SnackbarProvider>
35+
</React.StrictMode>,
3336
);

‎package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@
9797
"tsd": "^0.28.1",
9898
"tslib": "^2.3.0",
9999
"typescript": "~4.9.5",
100+
"vite": "^4.0.1",
100101
"vite-plugin-eslint": "^1.8.1",
101102
"vite-tsconfig-paths": "^4.0.2",
102103
"vitest": "^0.25.8"

‎packages/core/src/dispatcher/dispatcher.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,9 @@ export class Dispatcher {
529529
timestamp: +new Date(),
530530
};
531531

532+
// Global response emitter to handle request execution
533+
requestManager.events.emitResponse(cacheKey, requestId, response, requestDetails);
534+
532535
// Turn off loading
533536
requestManager.events.emitLoading(queueKey, requestId, {
534537
queueKey,
@@ -537,8 +540,7 @@ export class Dispatcher {
537540
isRetry: !!storageElement.retries,
538541
isOffline,
539542
});
540-
// Global response emitter to handle request execution
541-
requestManager.events.emitResponse(cacheKey, requestId, response, requestDetails);
543+
542544
// Cache event to emit the data inside and store it
543545
cache.set(request, { ...response, ...requestDetails });
544546
this.logger.info("Request finished", { requestId, request, response, requestDetails });

‎packages/react/__tests__/features/use-fetch/use-fetch.revalidate.spec.ts

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { xhrExtra } from "@hyper-fetch/core";
2-
import { act, waitFor } from "@testing-library/react";
2+
import { act } from "@testing-library/react";
33

44
import { startServer, resetInterceptors, stopServer, createRequestInterceptor } from "../../server";
55
import { testSuccessState } from "../../shared";
@@ -83,102 +83,82 @@ describe("useFetch [ refetch ]", () => {
8383

8484
const response = renderUseFetch(request, { revalidate: true });
8585

86-
await waitFor(async () => {
87-
await testSuccessState(mock, response);
88-
});
86+
await testSuccessState(mock, response);
8987
});
9088
it("should allow to refetch current hook", async () => {
9189
const response = renderUseFetch(request);
9290

93-
await waitFor(async () => {
94-
await testSuccessState(mock, response);
95-
});
91+
await testSuccessState(mock, response);
9692
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });
9793

9894
act(() => {
9995
response.result.current.refetch();
10096
});
10197

102-
await waitFor(async () => {
103-
await testSuccessState(customMock, response);
104-
});
98+
await testSuccessState(customMock, response);
10599
});
106100
it("should allow to refetch hook by RegExp", async () => {
107101
const regexp = /(Maciej|Kacper)/;
102+
108103
const responseOne = renderUseFetch(request.setCacheKey("Maciej"));
109104
const responseTwo = renderUseFetch(request.setCacheKey("Kacper"));
110105

111-
await waitFor(async () => {
112-
await testSuccessState(mock, responseOne);
113-
await testSuccessState(mock, responseTwo);
114-
});
106+
await testSuccessState(mock, responseOne);
107+
await testSuccessState(mock, responseTwo);
108+
115109
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });
116110

117111
act(() => {
118112
responseOne.result.current.refetch(regexp);
119113
});
120114

121-
await waitFor(async () => {
122-
await testSuccessState(customMock, responseOne);
123-
await testSuccessState(customMock, responseTwo);
124-
});
115+
await testSuccessState(customMock, responseOne);
116+
await testSuccessState(customMock, responseTwo);
125117
});
126118
it("should allow to refetch hook by keys array", async () => {
127119
const responseOne = renderUseFetch(request.setCacheKey("Maciej"));
128120
const responseTwo = renderUseFetch(request.setCacheKey("Kacper"));
129121

130-
await waitFor(async () => {
131-
await testSuccessState(mock, responseOne);
132-
await testSuccessState(mock, responseTwo);
133-
});
122+
await testSuccessState(mock, responseOne);
123+
await testSuccessState(mock, responseTwo);
134124
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });
135125

136126
act(() => {
137127
responseOne.result.current.refetch(["Maciej"]);
138128
});
139129

140-
await waitFor(async () => {
141-
await testSuccessState(customMock, responseOne);
142-
await testSuccessState(mock, responseTwo);
143-
});
130+
await testSuccessState(customMock, responseOne);
131+
await testSuccessState(mock, responseTwo);
144132
});
145133
it("should allow to refetch hook by key", async () => {
146134
const responseOne = renderUseFetch(request.setCacheKey("Maciej"));
147135
const responseTwo = renderUseFetch(request.setCacheKey("Kacper"));
148136

149-
await waitFor(async () => {
150-
await testSuccessState(mock, responseOne);
151-
await testSuccessState(mock, responseTwo);
152-
});
137+
await testSuccessState(mock, responseOne);
138+
await testSuccessState(mock, responseTwo);
153139
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });
154140

155141
act(() => {
156142
responseOne.result.current.refetch("Maciej");
157143
});
158144

159-
await waitFor(async () => {
160-
await testSuccessState(customMock, responseOne);
161-
await testSuccessState(mock, responseTwo);
162-
});
145+
await testSuccessState(customMock, responseOne);
146+
await testSuccessState(mock, responseTwo);
163147
});
164148
it("should allow to refetch hook by request", async () => {
165149
const responseOne = renderUseFetch(request.setQueryParams("?something=123"));
166150
const responseTwo = renderUseFetch(request.setQueryParams("?other=999"));
167151

168-
await waitFor(async () => {
169-
await testSuccessState(mock, responseOne);
170-
await testSuccessState(mock, responseTwo);
171-
});
152+
await testSuccessState(mock, responseOne);
153+
await testSuccessState(mock, responseTwo);
172154
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });
173155

174156
act(() => {
175157
responseOne.result.current.refetch(request.setQueryParams("?something=123"));
176158
});
177159

178-
await waitFor(async () => {
179-
await testSuccessState(customMock, responseOne);
180-
await testSuccessState(mock, responseTwo);
181-
});
160+
await testSuccessState(customMock, responseOne);
161+
await testSuccessState(mock, responseTwo);
182162
});
183163
it("should not refetch while toggling query", async () => {
184164
const spy = jest.fn();

‎packages/react/src/helpers/use-request-events/use-request-events.hooks.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export const useRequestEvents = <T extends RequestInstance>({
3636
logger,
3737
actions,
3838
setCacheData,
39+
getIsDataProcessing,
3940
}: UseRequestEventsPropsType<T>): UseRequestEventsReturnType<T> => {
4041
const { responseMapper } = request;
4142
const { cache, requestManager } = request.client;
@@ -107,9 +108,15 @@ export const useRequestEvents = <T extends RequestInstance>({
107108
// Lifecycle
108109
// ******************
109110

110-
const handleGetLoadingEvent = (queueKey: string) => {
111+
const handleGetLoadingEvent = (req: T) => {
111112
return ({ loading }: RequestLoadingEventType) => {
112-
const canDisableLoading = !loading && !dispatcher.hasRunningRequests(queueKey);
113+
const isProcessing = getIsDataProcessing(req.cacheKey);
114+
115+
// When we process the cache data, we don't want to change the loading state during it
116+
// This prevents the UI from flickering with { data: null, loading: false }
117+
if (isProcessing) return;
118+
119+
const canDisableLoading = !loading && !dispatcher.hasRunningRequests(req.queueKey);
113120
if (loading || canDisableLoading) {
114121
actions.setLoading(loading, false);
115122
}
@@ -160,14 +167,14 @@ export const useRequestEvents = <T extends RequestInstance>({
160167
// Data Listeners
161168
// ******************
162169

163-
const clearDataListener = () => {
170+
const clearCacheDataListener = () => {
164171
dataEvents.current?.unmount();
165172
dataEvents.current = null;
166173
};
167174

168-
const addDataListener = (req: T) => {
175+
const addCacheDataListener = (req: T) => {
169176
// Data handlers
170-
const loadingUnmount = requestManager.events.onLoading(req.queueKey, handleGetLoadingEvent(req.queueKey));
177+
const loadingUnmount = requestManager.events.onLoading(req.queueKey, handleGetLoadingEvent(req));
171178
const getResponseUnmount = cache.events.onData<ExtractResponseType<T>, ExtractErrorType<T>, ExtractAdapterType<T>>(
172179
req.cacheKey,
173180
setCacheData,
@@ -178,7 +185,7 @@ export const useRequestEvents = <T extends RequestInstance>({
178185
getResponseUnmount();
179186
};
180187

181-
clearDataListener();
188+
clearCacheDataListener();
182189
dataEvents.current = { unmount };
183190

184191
return unmount;
@@ -264,7 +271,7 @@ export const useRequestEvents = <T extends RequestInstance>({
264271
useWillUnmount(() => {
265272
// Unmount listeners
266273
clearLifecycleListeners();
267-
clearDataListener();
274+
clearCacheDataListener();
268275
});
269276

270277
return [
@@ -299,8 +306,8 @@ export const useRequestEvents = <T extends RequestInstance>({
299306
},
300307
},
301308
{
302-
addDataListener,
303-
clearDataListener,
309+
addCacheDataListener,
310+
clearCacheDataListener,
304311
addLifecycleListeners,
305312
removeLifecycleListener,
306313
clearLifecycleListeners,

‎packages/react/src/helpers/use-request-events/use-request-events.types.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export type UseRequestEventsPropsType<T extends RequestInstance> = {
2424
logger: LoggerType;
2525
actions: UseTrackedStateActions<T>;
2626
setCacheData: (cacheData: CacheValueType<ExtractResponseType<T>, ExtractErrorType<T>>) => void;
27+
getIsDataProcessing: (cacheKey: string) => boolean;
2728
};
2829

2930
export type UseRequestEventsActionsType<T extends RequestInstance> = {
@@ -73,8 +74,8 @@ export type UseRequestEventsActionsType<T extends RequestInstance> = {
7374
export type UseRequestEventsReturnType<T extends RequestInstance> = [
7475
UseRequestEventsActionsType<T>,
7576
{
76-
addDataListener: (request: RequestInstance) => VoidFunction;
77-
clearDataListener: VoidFunction;
77+
addCacheDataListener: (request: RequestInstance) => VoidFunction;
78+
clearCacheDataListener: VoidFunction;
7879
addLifecycleListeners: (request: RequestInstance, requestId?: string) => VoidFunction;
7980
removeLifecycleListener: (requestId: string) => void;
8081
clearLifecycleListeners: () => void;

‎packages/react/src/helpers/use-tracked-state/use-tracked-state.hooks.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export const useTrackedState = <T extends RequestInstance>({
4141

4242
const state = useRef<UseTrackedStateType<T>>(getInitialState(initialData, dispatcher, request));
4343
const renderKeys = useRef<Array<keyof UseTrackedStateType<T>>>([]);
44+
const isProcessingData = useRef("");
4445

4546
// ******************
4647
// Utils
@@ -143,7 +144,7 @@ export const useTrackedState = <T extends RequestInstance>({
143144
extra: cacheData.extra,
144145
retries: cacheData.retries,
145146
timestamp: new Date(cacheData.timestamp),
146-
loading: state.current.loading,
147+
loading: dispatcher.hasRunningRequests(queueKey),
147148
};
148149

149150
const changedKeys = Object.keys(newStateValues).filter((key) => {
@@ -162,17 +163,40 @@ export const useTrackedState = <T extends RequestInstance>({
162163
renderKeyTrigger(changedKeys);
163164
};
164165

166+
const setIsDataProcessing = ({
167+
processingCacheKey,
168+
isProcessing,
169+
}: {
170+
processingCacheKey: string;
171+
isProcessing: boolean;
172+
}) => {
173+
if (isProcessing) {
174+
isProcessingData.current = processingCacheKey;
175+
}
176+
// Do not turn off other keys processing
177+
else if (isProcessingData.current === cacheKey) {
178+
isProcessingData.current = "";
179+
}
180+
};
181+
182+
const getIsDataProcessing = (processingCacheKey: string) => {
183+
return isProcessingData.current === processingCacheKey;
184+
};
185+
165186
const setCacheData = (
166187
cacheData: CacheValueType<ExtractResponseType<T>, ExtractErrorType<T>, ExtractAdapterType<T>>,
167188
): Promise<void> | void => {
189+
setIsDataProcessing({ processingCacheKey: cacheKey, isProcessing: true });
168190
const data = responseMapper ? responseMapper(cacheData) : cacheData;
169191

170192
if (data instanceof Promise) {
171193
return (async () => {
172194
const promiseData = await data;
173195
handleCacheData({ ...cacheData, ...promiseData });
196+
setIsDataProcessing({ processingCacheKey: cacheKey, isProcessing: false });
174197
})();
175198
}
199+
setIsDataProcessing({ processingCacheKey: cacheKey, isProcessing: false });
176200
return handleCacheData({ ...cacheData, ...data });
177201
};
178202

@@ -260,5 +284,5 @@ export const useTrackedState = <T extends RequestInstance>({
260284
},
261285
};
262286

263-
return [state.current, actions, { setRenderKey, setCacheData, getStaleStatus }];
287+
return [state.current, actions, { setRenderKey, setCacheData, getStaleStatus, getIsDataProcessing }];
264288
};

‎packages/react/src/helpers/use-tracked-state/use-tracked-state.types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export type UseTrackedStateReturn<T extends RequestInstance> = [
3131
setRenderKey: (renderKey: keyof UseTrackedStateType<T>) => void;
3232
setCacheData: (cacheData: CacheValueType<ExtractResponseType<T>, ExtractErrorType<T>>) => void;
3333
getStaleStatus: () => boolean;
34+
getIsDataProcessing: (cacheKey: string) => boolean;
3435
},
3536
];
3637

0 commit comments

Comments
 (0)