Skip to content

Commit

Permalink
Fix/issue 94 (#96)
Browse files Browse the repository at this point in the history
* fix: πŸ› State processing race condition

βœ… Closes: #94

* refactor: πŸ’‘ bring back old example code

* fix: πŸ› deps

* refactor: πŸ’‘ adjusted keys comparison
  • Loading branch information
prc5 authored Dec 4, 2024
1 parent 9fe13e5 commit 1e1993b
Show file tree
Hide file tree
Showing 13 changed files with 306 additions and 94 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ coverage

# Next.js
.next
.nx

# Firebase
database-debug.log
Expand Down
41 changes: 22 additions & 19 deletions examples/reactjs/src/main.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import React from "react";
import { Stack } from "@mui/material";
import * as ReactDOM from "react-dom/client";
import { SnackbarProvider } from "notistack";
Expand All @@ -11,23 +12,25 @@ import DashboardPage from "pages";

const root = ReactDOM.createRoot(document.getElementById("root") as HTMLElement);
root.render(
<SnackbarProvider
maxSnack={6}
autoHideDuration={1000}
anchorOrigin={{
vertical: "top",
horizontal: "right",
}}
>
<BrowserRouter>
<Stack direction="row">
<Routes>
<Route path={DASHBOARD_PAGE.path} element={<DashboardPage />} />
<Route path={DETAILS_PAGE.path} element={<DetailsPage />} />
<Route path={LIST_PAGE.path} element={<ListPage />} />
<Route path={FORM_PAGE.path} element={<FormPage />} />
</Routes>
</Stack>
</BrowserRouter>
</SnackbarProvider>,
<React.StrictMode>
<SnackbarProvider
maxSnack={6}
autoHideDuration={1000}
anchorOrigin={{
vertical: "top",
horizontal: "right",
}}
>
<BrowserRouter>
<Stack direction="row">
<Routes>
<Route path={DASHBOARD_PAGE.path} element={<DashboardPage />} />
<Route path={DETAILS_PAGE.path} element={<DetailsPage />} />
<Route path={LIST_PAGE.path} element={<ListPage />} />
<Route path={FORM_PAGE.path} element={<FormPage />} />
</Routes>
</Stack>
</BrowserRouter>
</SnackbarProvider>
</React.StrictMode>,
);
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
"tsd": "^0.28.1",
"tslib": "^2.3.0",
"typescript": "~4.9.5",
"vite": "^4.0.1",
"vite-plugin-eslint": "^1.8.1",
"vite-tsconfig-paths": "^4.0.2",
"vitest": "^0.25.8"
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/dispatcher/dispatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,9 @@ export class Dispatcher {
timestamp: +new Date(),
};

// Global response emitter to handle request execution
requestManager.events.emitResponse(cacheKey, requestId, response, requestDetails);

// Turn off loading
requestManager.events.emitLoading(queueKey, requestId, {
queueKey,
Expand All @@ -537,8 +540,7 @@ export class Dispatcher {
isRetry: !!storageElement.retries,
isOffline,
});
// Global response emitter to handle request execution
requestManager.events.emitResponse(cacheKey, requestId, response, requestDetails);

// Cache event to emit the data inside and store it
cache.set(request, { ...response, ...requestDetails });
this.logger.info("Request finished", { requestId, request, response, requestDetails });
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { xhrExtra } from "@hyper-fetch/core";
import { act, waitFor } from "@testing-library/react";
import { act } from "@testing-library/react";

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

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

await waitFor(async () => {
await testSuccessState(mock, response);
});
await testSuccessState(mock, response);
});
it("should allow to refetch current hook", async () => {
const response = renderUseFetch(request);

await waitFor(async () => {
await testSuccessState(mock, response);
});
await testSuccessState(mock, response);
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });

act(() => {
response.result.current.refetch();
});

await waitFor(async () => {
await testSuccessState(customMock, response);
});
await testSuccessState(customMock, response);
});
it("should allow to refetch hook by RegExp", async () => {
const regexp = /(Maciej|Kacper)/;

const responseOne = renderUseFetch(request.setCacheKey("Maciej"));
const responseTwo = renderUseFetch(request.setCacheKey("Kacper"));

await waitFor(async () => {
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);
});
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);

const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });

act(() => {
responseOne.result.current.refetch(regexp);
});

await waitFor(async () => {
await testSuccessState(customMock, responseOne);
await testSuccessState(customMock, responseTwo);
});
await testSuccessState(customMock, responseOne);
await testSuccessState(customMock, responseTwo);
});
it("should allow to refetch hook by keys array", async () => {
const responseOne = renderUseFetch(request.setCacheKey("Maciej"));
const responseTwo = renderUseFetch(request.setCacheKey("Kacper"));

await waitFor(async () => {
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);
});
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });

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

await waitFor(async () => {
await testSuccessState(customMock, responseOne);
await testSuccessState(mock, responseTwo);
});
await testSuccessState(customMock, responseOne);
await testSuccessState(mock, responseTwo);
});
it("should allow to refetch hook by key", async () => {
const responseOne = renderUseFetch(request.setCacheKey("Maciej"));
const responseTwo = renderUseFetch(request.setCacheKey("Kacper"));

await waitFor(async () => {
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);
});
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });

act(() => {
responseOne.result.current.refetch("Maciej");
});

await waitFor(async () => {
await testSuccessState(customMock, responseOne);
await testSuccessState(mock, responseTwo);
});
await testSuccessState(customMock, responseOne);
await testSuccessState(mock, responseTwo);
});
it("should allow to refetch hook by request", async () => {
const responseOne = renderUseFetch(request.setQueryParams("?something=123"));
const responseTwo = renderUseFetch(request.setQueryParams("?other=999"));

await waitFor(async () => {
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);
});
await testSuccessState(mock, responseOne);
await testSuccessState(mock, responseTwo);
const customMock = createRequestInterceptor(request, { fixture: { something: 123 } });

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

await waitFor(async () => {
await testSuccessState(customMock, responseOne);
await testSuccessState(mock, responseTwo);
});
await testSuccessState(customMock, responseOne);
await testSuccessState(mock, responseTwo);
});
it("should not refetch while toggling query", async () => {
const spy = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const useRequestEvents = <T extends RequestInstance>({
logger,
actions,
setCacheData,
getIsDataProcessing,
}: UseRequestEventsPropsType<T>): UseRequestEventsReturnType<T> => {
const { responseMapper } = request;
const { cache, requestManager } = request.client;
Expand Down Expand Up @@ -107,9 +108,15 @@ export const useRequestEvents = <T extends RequestInstance>({
// Lifecycle
// ******************

const handleGetLoadingEvent = (queueKey: string) => {
const handleGetLoadingEvent = (req: T) => {
return ({ loading }: RequestLoadingEventType) => {
const canDisableLoading = !loading && !dispatcher.hasRunningRequests(queueKey);
const isProcessing = getIsDataProcessing(req.cacheKey);

// When we process the cache data, we don't want to change the loading state during it
// This prevents the UI from flickering with { data: null, loading: false }
if (isProcessing) return;

const canDisableLoading = !loading && !dispatcher.hasRunningRequests(req.queueKey);
if (loading || canDisableLoading) {
actions.setLoading(loading, false);
}
Expand Down Expand Up @@ -160,14 +167,14 @@ export const useRequestEvents = <T extends RequestInstance>({
// Data Listeners
// ******************

const clearDataListener = () => {
const clearCacheDataListener = () => {
dataEvents.current?.unmount();
dataEvents.current = null;
};

const addDataListener = (req: T) => {
const addCacheDataListener = (req: T) => {
// Data handlers
const loadingUnmount = requestManager.events.onLoading(req.queueKey, handleGetLoadingEvent(req.queueKey));
const loadingUnmount = requestManager.events.onLoading(req.queueKey, handleGetLoadingEvent(req));
const getResponseUnmount = cache.events.onData<ExtractResponseType<T>, ExtractErrorType<T>, ExtractAdapterType<T>>(
req.cacheKey,
setCacheData,
Expand All @@ -178,7 +185,7 @@ export const useRequestEvents = <T extends RequestInstance>({
getResponseUnmount();
};

clearDataListener();
clearCacheDataListener();
dataEvents.current = { unmount };

return unmount;
Expand Down Expand Up @@ -264,7 +271,7 @@ export const useRequestEvents = <T extends RequestInstance>({
useWillUnmount(() => {
// Unmount listeners
clearLifecycleListeners();
clearDataListener();
clearCacheDataListener();
});

return [
Expand Down Expand Up @@ -299,8 +306,8 @@ export const useRequestEvents = <T extends RequestInstance>({
},
},
{
addDataListener,
clearDataListener,
addCacheDataListener,
clearCacheDataListener,
addLifecycleListeners,
removeLifecycleListener,
clearLifecycleListeners,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export type UseRequestEventsPropsType<T extends RequestInstance> = {
logger: LoggerType;
actions: UseTrackedStateActions<T>;
setCacheData: (cacheData: CacheValueType<ExtractResponseType<T>, ExtractErrorType<T>>) => void;
getIsDataProcessing: (cacheKey: string) => boolean;
};

export type UseRequestEventsActionsType<T extends RequestInstance> = {
Expand Down Expand Up @@ -73,8 +74,8 @@ export type UseRequestEventsActionsType<T extends RequestInstance> = {
export type UseRequestEventsReturnType<T extends RequestInstance> = [
UseRequestEventsActionsType<T>,
{
addDataListener: (request: RequestInstance) => VoidFunction;
clearDataListener: VoidFunction;
addCacheDataListener: (request: RequestInstance) => VoidFunction;
clearCacheDataListener: VoidFunction;
addLifecycleListeners: (request: RequestInstance, requestId?: string) => VoidFunction;
removeLifecycleListener: (requestId: string) => void;
clearLifecycleListeners: () => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export const useTrackedState = <T extends RequestInstance>({

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

// ******************
// Utils
Expand Down Expand Up @@ -143,7 +144,7 @@ export const useTrackedState = <T extends RequestInstance>({
extra: cacheData.extra,
retries: cacheData.retries,
timestamp: new Date(cacheData.timestamp),
loading: state.current.loading,
loading: dispatcher.hasRunningRequests(queueKey),
};

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

const setIsDataProcessing = ({
processingCacheKey,
isProcessing,
}: {
processingCacheKey: string;
isProcessing: boolean;
}) => {
if (isProcessing) {
isProcessingData.current = processingCacheKey;
}
// Do not turn off other keys processing
else if (isProcessingData.current === cacheKey) {
isProcessingData.current = "";
}
};

const getIsDataProcessing = (processingCacheKey: string) => {
return isProcessingData.current === processingCacheKey;
};

const setCacheData = (
cacheData: CacheValueType<ExtractResponseType<T>, ExtractErrorType<T>, ExtractAdapterType<T>>,
): Promise<void> | void => {
setIsDataProcessing({ processingCacheKey: cacheKey, isProcessing: true });
const data = responseMapper ? responseMapper(cacheData) : cacheData;

if (data instanceof Promise) {
return (async () => {
const promiseData = await data;
handleCacheData({ ...cacheData, ...promiseData });
setIsDataProcessing({ processingCacheKey: cacheKey, isProcessing: false });
})();
}
setIsDataProcessing({ processingCacheKey: cacheKey, isProcessing: false });
return handleCacheData({ ...cacheData, ...data });
};

Expand Down Expand Up @@ -260,5 +284,5 @@ export const useTrackedState = <T extends RequestInstance>({
},
};

return [state.current, actions, { setRenderKey, setCacheData, getStaleStatus }];
return [state.current, actions, { setRenderKey, setCacheData, getStaleStatus, getIsDataProcessing }];
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export type UseTrackedStateReturn<T extends RequestInstance> = [
setRenderKey: (renderKey: keyof UseTrackedStateType<T>) => void;
setCacheData: (cacheData: CacheValueType<ExtractResponseType<T>, ExtractErrorType<T>>) => void;
getStaleStatus: () => boolean;
getIsDataProcessing: (cacheKey: string) => boolean;
},
];

Expand Down
Loading

0 comments on commit 1e1993b

Please sign in to comment.