Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { act, renderHook } from '@testing-library/react';
import { describe, expect, test, vi } from 'vitest';
import useRequest from '../index';

describe('useCachePlugin shared promise loading', () => {
vi.useFakeTimers();

const request = () =>
new Promise<string>((resolve) =>
setTimeout(() => {
resolve('success');
}, 1000),
);

const setup = (
service: Parameters<typeof useRequest>[0],
options: Parameters<typeof useRequest>[1],
) => renderHook(() => useRequest(service, options));

// When hook1 refreshes (starts a new shared request), hook2 should also show loading: true
// This is the bug: hook2 is not notified that a request has started for the shared cacheKey
test('when one hook triggers request with shared cacheKey, other hooks should also show loading', async () => {
const hook1 = setup(request, { cacheKey: 'shared-loading-bug-test' });
const hook2 = setup(request, { cacheKey: 'shared-loading-bug-test' });

// Initial: both loading
expect(hook1.result.current.loading).toBe(true);
expect(hook2.result.current.loading).toBe(true);

await act(async () => {
vi.advanceTimersByTime(1000);
});

// After initial request: both have data, not loading
expect(hook1.result.current.loading).toBe(false);
expect(hook2.result.current.loading).toBe(false);

// Advance time past staleTime (default 0) to make data stale
vi.advanceTimersByTime(1);

// Now hook1 manually triggers a new request
act(() => {
hook1.result.current.run();
});

// Both hooks should show loading: true since a shared request is in flight
expect(hook1.result.current.loading).toBe(true);
expect(hook2.result.current.loading).toBe(true); // BUG: this fails without the fix

await act(async () => {
vi.advanceTimersByTime(1000);
});

// After request completes: both have data, not loading
expect(hook1.result.current.loading).toBe(false);
expect(hook2.result.current.loading).toBe(false);

hook1.unmount();
hook2.unmount();
});
});
17 changes: 13 additions & 4 deletions packages/hooks/src/useRequest/src/plugins/useCachePlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Plugin } from '../types';
import { setCache, getCache } from '../utils/cache';
import type { CachedData } from '../utils/cache';
import { setCachePromise, getCachePromise } from '../utils/cachePromise';
import { trigger, subscribe } from '../utils/cacheSubscribe';
import { trigger, subscribe, triggerLoading, subscribeLoading } from '../utils/cacheSubscribe';

const useCachePlugin: Plugin<any, any[]> = (
fetchInstance,
Expand All @@ -18,6 +18,7 @@ const useCachePlugin: Plugin<any, any[]> = (
},
) => {
const unSubscribeRef = useRef<() => void>(undefined);
const unSubscribeLoadingRef = useRef<() => void>(undefined);

const currentPromiseRef = useRef<Promise<any>>(undefined);

Expand Down Expand Up @@ -54,12 +55,18 @@ const useCachePlugin: Plugin<any, any[]> = (

// subscribe same cachekey update, trigger update
unSubscribeRef.current = subscribe(cacheKey, (data) => {
fetchInstance.setState({ data });
fetchInstance.setState({ data, loading: false });
});

// subscribe same cachekey loading update, trigger update
unSubscribeLoadingRef.current = subscribeLoading(cacheKey, () => {
fetchInstance.setState({ loading: true });
});
}, []);

useUnmount(() => {
unSubscribeRef.current?.();
unSubscribeLoadingRef.current?.();
});

if (!cacheKey) {
Expand Down Expand Up @@ -101,6 +108,8 @@ const useCachePlugin: Plugin<any, any[]> = (
servicePromise = service(...args);
currentPromiseRef.current = servicePromise;
setCachePromise(cacheKey, servicePromise);
// notify other same cacheKey components that loading has started
triggerLoading(cacheKey);
return { servicePromise };
},
onSuccess: (data, params) => {
Expand All @@ -114,7 +123,7 @@ const useCachePlugin: Plugin<any, any[]> = (
});
// resubscribe
unSubscribeRef.current = subscribe(cacheKey, (d) => {
fetchInstance.setState({ data: d });
fetchInstance.setState({ data: d, loading: false });
});
}
},
Expand All @@ -129,7 +138,7 @@ const useCachePlugin: Plugin<any, any[]> = (
});
// resubscribe
unSubscribeRef.current = subscribe(cacheKey, (d) => {
fetchInstance.setState({ data: d });
fetchInstance.setState({ data: d, loading: false });
});
}
},
Expand Down
25 changes: 24 additions & 1 deletion packages/hooks/src/useRequest/src/utils/cacheSubscribe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,27 @@ const subscribe = (key: string, listener: Listener) => {
};
};

export { trigger, subscribe };
type LoadingListener = () => void;
const loadingListeners: Record<string, LoadingListener[]> = {};

const triggerLoading = (key: string) => {
if (loadingListeners[key]) {
loadingListeners[key].forEach((item) => item());
}
};

const subscribeLoading = (key: string, listener: LoadingListener) => {
if (!loadingListeners[key]) {
loadingListeners[key] = [];
}
loadingListeners[key].push(listener);

return function unsubscribeLoading() {
const index = loadingListeners[key].indexOf(listener);
if (index > -1) {
loadingListeners[key].splice(index, 1);
}
};
};

export { trigger, subscribe, triggerLoading, subscribeLoading };
Loading
Loading