Skip to content

Commit 18944d7

Browse files
panteliselefpanteliselef
committed
usePlans should not be triggering requests based on userId changes and orgId changes as it returns unauthenticated data.
1 parent ca591d6 commit 18944d7

File tree

6 files changed

+286
-92
lines changed

6 files changed

+286
-92
lines changed

‎packages/shared/src/react/hooks/__tests__/createBillingPaginatedHook.spec.tsx‎

Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ describe('createBillingPaginatedHook', () => {
9191
expect(useFetcherMock).toHaveBeenCalledWith('user');
9292

9393
expect(fetcherMock).not.toHaveBeenCalled();
94+
// Ensures that SWR does not update the loading state even if the fetcher is not called.
9495
expect(result.current.isLoading).toBe(false);
96+
expect(result.current.isFetching).toBe(false);
9597
});
9698

9799
it('authenticated hook: does not fetch when user is null', () => {
@@ -181,4 +183,220 @@ describe('createBillingPaginatedHook', () => {
181183
expect(fetcherMock).not.toHaveBeenCalled();
182184
expect(result.current.isLoading).toBe(false);
183185
});
186+
187+
describe('authenticated hook - after sign-out previously loaded data are cleared', () => {
188+
it('pagination mode: data is cleared when user signs out', async () => {
189+
fetcherMock.mockImplementation((params: any) =>
190+
Promise.resolve({
191+
data: Array.from({ length: params.pageSize }, (_, i) => ({ id: `p${params.initialPage}-${i}` })),
192+
total_count: 5,
193+
}),
194+
);
195+
196+
const { result, rerender } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 2 }), {
197+
wrapper,
198+
});
199+
200+
await waitFor(() => expect(result.current.isFetching).toBe(true));
201+
await waitFor(() => expect(result.current.isFetching).toBe(false));
202+
expect(result.current.data.length).toBe(2);
203+
expect(result.current.page).toBe(1);
204+
expect(result.current.pageCount).toBe(3); // ceil(5/2)
205+
206+
// Simulate sign-out
207+
mockUser = null;
208+
rerender();
209+
210+
// Data should become empty
211+
await waitFor(() => expect(result.current.data).toEqual([]));
212+
expect(result.current.count).toBe(0);
213+
expect(result.current.page).toBe(1);
214+
expect(result.current.pageCount).toBe(0);
215+
});
216+
217+
it('pagination mode: with keepPreviousData=true data is cleared after sign-out', async () => {
218+
fetcherMock.mockImplementation((params: any) =>
219+
Promise.resolve({
220+
data: Array.from({ length: params.pageSize }, (_, i) => ({ id: `item-${params.initialPage}-${i}` })),
221+
total_count: 20,
222+
}),
223+
);
224+
225+
const { result, rerender } = renderHook(
226+
() => useDummyAuth({ initialPage: 1, pageSize: 5, keepPreviousData: true }),
227+
{
228+
wrapper,
229+
},
230+
);
231+
232+
// Wait for initial data load
233+
await waitFor(() => expect(result.current.isLoading).toBe(true));
234+
await waitFor(() => expect(result.current.isLoading).toBe(false));
235+
236+
expect(result.current.data.length).toBe(5);
237+
expect(result.current.data).toEqual([
238+
{ id: 'item-1-0' },
239+
{ id: 'item-1-1' },
240+
{ id: 'item-1-2' },
241+
{ id: 'item-1-3' },
242+
{ id: 'item-1-4' },
243+
]);
244+
expect(result.current.count).toBe(20);
245+
246+
// Simulate sign-out by setting mockUser to null
247+
mockUser = null;
248+
rerender();
249+
250+
// Attention: We are forcing fetcher to be executed instead of setting the key to null
251+
// because SWR will continue to display the cached data when the key is null and `keepPreviousData` is true.
252+
// This means that SWR will update the loading state to true even if the fetcher is not called,
253+
// because the key changes from `{..., userId: 'user_1'}` to `{..., userId: undefined}`.
254+
await waitFor(() => expect(result.current.isLoading).toBe(true));
255+
await waitFor(() => expect(result.current.isLoading).toBe(false));
256+
257+
// Data should be cleared even with keepPreviousData: true
258+
// The key difference here vs usePagesOrInfinite test: userId in cache key changes
259+
// from 'user_1' to undefined, which changes the cache key (not just makes it null)
260+
await waitFor(() => expect(result.current.data).toEqual([]));
261+
expect(result.current.count).toBe(0);
262+
expect(result.current.page).toBe(1);
263+
expect(result.current.pageCount).toBe(0);
264+
});
265+
266+
it('infinite mode: data is cleared when user signs out', async () => {
267+
fetcherMock.mockImplementation((params: any) =>
268+
Promise.resolve({
269+
data: Array.from({ length: params.pageSize }, (_, i) => ({ id: `p${params.initialPage}-${i}` })),
270+
total_count: 10,
271+
}),
272+
);
273+
274+
const { result, rerender } = renderHook(() => useDummyAuth({ initialPage: 1, pageSize: 2, infinite: true }), {
275+
wrapper,
276+
});
277+
278+
await waitFor(() => expect(result.current.isFetching).toBe(true));
279+
await waitFor(() => expect(result.current.isFetching).toBe(false));
280+
expect(result.current.data.length).toBe(2);
281+
expect(result.current.page).toBe(1);
282+
expect(result.current.pageCount).toBe(5); // ceil(10/2)
283+
284+
// Simulate sign-out
285+
mockUser = null;
286+
rerender();
287+
288+
await waitFor(() => expect(result.current.data).toEqual([]));
289+
expect(result.current.count).toBe(0);
290+
expect(result.current.page).toBe(1);
291+
expect(result.current.pageCount).toBe(0);
292+
});
293+
});
294+
295+
describe('unauthenticated hook - data persists after sign-out', () => {
296+
it('pagination mode: data persists when user signs out', async () => {
297+
fetcherMock.mockImplementation((params: any) =>
298+
Promise.resolve({
299+
data: Array.from({ length: params.pageSize }, (_, i) => ({ id: `p${params.initialPage}-${i}` })),
300+
total_count: 5,
301+
}),
302+
);
303+
304+
const { result, rerender } = renderHook(() => useDummyUnauth({ initialPage: 1, pageSize: 2 }), {
305+
wrapper,
306+
});
307+
308+
await waitFor(() => expect(result.current.isFetching).toBe(true));
309+
await waitFor(() => expect(result.current.isFetching).toBe(false));
310+
expect(result.current.data.length).toBe(2);
311+
expect(result.current.page).toBe(1);
312+
expect(result.current.pageCount).toBe(3); // ceil(5/2)
313+
314+
const originalData = [...result.current.data];
315+
const originalCount = result.current.count;
316+
317+
// Simulate sign-out
318+
mockUser = null;
319+
rerender();
320+
321+
// Data should persist for unauthenticated hooks
322+
expect(result.current.data).toEqual(originalData);
323+
expect(result.current.count).toBe(originalCount);
324+
expect(result.current.page).toBe(1);
325+
expect(result.current.pageCount).toBe(3);
326+
});
327+
328+
it('pagination mode: with keepPreviousData=true data persists after sign-out', async () => {
329+
fetcherMock.mockImplementation((params: any) =>
330+
Promise.resolve({
331+
data: Array.from({ length: params.pageSize }, (_, i) => ({ id: `item-${params.initialPage}-${i}` })),
332+
total_count: 20,
333+
}),
334+
);
335+
336+
const { result, rerender } = renderHook(
337+
() => useDummyUnauth({ initialPage: 1, pageSize: 5, keepPreviousData: true }),
338+
{
339+
wrapper,
340+
},
341+
);
342+
343+
// Wait for initial data load
344+
await waitFor(() => expect(result.current.isLoading).toBe(true));
345+
await waitFor(() => expect(result.current.isLoading).toBe(false));
346+
347+
expect(result.current.data.length).toBe(5);
348+
expect(result.current.data).toEqual([
349+
{ id: 'item-1-0' },
350+
{ id: 'item-1-1' },
351+
{ id: 'item-1-2' },
352+
{ id: 'item-1-3' },
353+
{ id: 'item-1-4' },
354+
]);
355+
expect(result.current.count).toBe(20);
356+
357+
const originalData = [...result.current.data];
358+
359+
// Simulate sign-out by setting mockUser to null
360+
mockUser = null;
361+
rerender();
362+
363+
// Data should persist for unauthenticated hooks even with keepPreviousData: true
364+
expect(result.current.data).toEqual(originalData);
365+
expect(result.current.count).toBe(20);
366+
expect(result.current.page).toBe(1);
367+
expect(result.current.pageCount).toBe(4); // ceil(20/5)
368+
});
369+
370+
it('infinite mode: data persists when user signs out', async () => {
371+
fetcherMock.mockImplementation((params: any) =>
372+
Promise.resolve({
373+
data: Array.from({ length: params.pageSize }, (_, i) => ({ id: `p${params.initialPage}-${i}` })),
374+
total_count: 10,
375+
}),
376+
);
377+
378+
const { result, rerender } = renderHook(() => useDummyUnauth({ initialPage: 1, pageSize: 2, infinite: true }), {
379+
wrapper,
380+
});
381+
382+
await waitFor(() => expect(result.current.isFetching).toBe(true));
383+
await waitFor(() => expect(result.current.isFetching).toBe(false));
384+
expect(result.current.data.length).toBe(2);
385+
expect(result.current.page).toBe(1);
386+
expect(result.current.pageCount).toBe(5); // ceil(10/2)
387+
388+
const originalData = [...result.current.data];
389+
const originalCount = result.current.count;
390+
391+
// Simulate sign-out
392+
mockUser = null;
393+
rerender();
394+
395+
// Data should persist for unauthenticated hooks
396+
expect(result.current.data).toEqual(originalData);
397+
expect(result.current.count).toBe(originalCount);
398+
expect(result.current.page).toBe(1);
399+
expect(result.current.pageCount).toBe(5);
400+
});
401+
});
184402
});

‎packages/shared/src/react/hooks/__tests__/usePagesOrInfinite.spec.ts‎

Lines changed: 0 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -295,83 +295,6 @@ describe('usePagesOrInfinite - keepPreviousData behavior', () => {
295295
});
296296
});
297297

298-
describe('usePagesOrInfinite - sign-out hides previously loaded data', () => {
299-
it('pagination mode: data is cleared when isSignedIn switches to false', async () => {
300-
const fetcher = vi.fn((p: any) =>
301-
Promise.resolve({
302-
data: Array.from({ length: p.pageSize }, (_, i) => ({ id: `p${p.initialPage}-${i}` })),
303-
total_count: 5,
304-
}),
305-
);
306-
307-
const params = { initialPage: 1, pageSize: 2 } as const;
308-
const baseConfig = { infinite: false, enabled: true } as const;
309-
const cacheKeys = { type: 't-signedout-paginated' } as const;
310-
311-
const { result, rerender } = renderHook(
312-
({ signedIn }) =>
313-
usePagesOrInfinite(
314-
params as any,
315-
fetcher as any,
316-
{ ...baseConfig, isSignedIn: signedIn } as any,
317-
cacheKeys as any,
318-
),
319-
{ initialProps: { signedIn: true }, wrapper },
320-
);
321-
322-
await waitFor(() => expect(result.current.isFetching).toBe(true));
323-
await waitFor(() => expect(result.current.isFetching).toBe(false));
324-
expect(result.current.data.length).toBe(2);
325-
expect(result.current.page).toBe(1);
326-
expect(result.current.pageCount).toBe(3); // ceil(5/2)
327-
328-
// simulate sign-out
329-
rerender({ signedIn: false });
330-
// data should become empty
331-
await waitFor(() => expect(result.current.data).toEqual([]));
332-
expect(result.current.count).toBe(0);
333-
expect(result.current.page).toBe(1);
334-
expect(result.current.pageCount).toBe(0);
335-
});
336-
337-
it('infinite mode: data is cleared when isSignedIn switches to false', async () => {
338-
const fetcher = vi.fn((p: any) =>
339-
Promise.resolve({
340-
data: Array.from({ length: p.pageSize }, (_, i) => ({ id: `p${p.initialPage}-${i}` })),
341-
total_count: 10,
342-
}),
343-
);
344-
345-
const params = { initialPage: 1, pageSize: 2 } as const;
346-
const baseConfig = { infinite: true, enabled: true } as const;
347-
const cacheKeys = { type: 't-signedout-infinite' } as const;
348-
349-
const { result, rerender } = renderHook(
350-
({ signedIn }) =>
351-
usePagesOrInfinite(
352-
params as any,
353-
fetcher as any,
354-
{ ...baseConfig, isSignedIn: signedIn } as any,
355-
cacheKeys as any,
356-
),
357-
{ initialProps: { signedIn: true }, wrapper },
358-
);
359-
360-
await waitFor(() => expect(result.current.isFetching).toBe(true));
361-
await waitFor(() => expect(result.current.isFetching).toBe(false));
362-
expect(result.current.data.length).toBe(2);
363-
expect(result.current.page).toBe(1);
364-
expect(result.current.pageCount).toBe(5); // ceil(10/2)
365-
366-
// simulate sign-out
367-
rerender({ signedIn: false });
368-
await waitFor(() => expect(result.current.data).toEqual([]));
369-
expect(result.current.count).toBe(0);
370-
expect(result.current.page).toBe(1);
371-
expect(result.current.pageCount).toBe(0);
372-
});
373-
});
374-
375298
describe('usePagesOrInfinite - pagination helpers', () => {
376299
it('computes pageCount/hasNext/hasPrevious correctly for initialPage>1', async () => {
377300
const totalCount = 37;

‎packages/shared/src/react/hooks/__tests__/usePlans.spec.tsx‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ describe('usePlans', () => {
6565

6666
expect(getPlansSpy).toHaveBeenCalledTimes(1);
6767
// ensure correct args passed: for: 'user' and limit/page (rest)
68-
expect(getPlansSpy.mock.calls[0][0]).toMatchObject({ for: 'user' });
69-
expect(getPlansSpy.mock.calls[0][0].orgId).toBeUndefined();
68+
expect(getPlansSpy.mock.calls[0][0]).toStrictEqual({ for: 'user', initialPage: 1, pageSize: 5 });
7069
expect(result.current.data.length).toBe(5);
7170
expect(result.current.count).toBe(25);
7271
});
@@ -79,9 +78,8 @@ describe('usePlans', () => {
7978
await waitFor(() => expect(result.current.isLoading).toBe(false));
8079

8180
expect(getPlansSpy).toHaveBeenCalledTimes(1);
82-
expect(getPlansSpy.mock.calls[0][0]).toMatchObject({ for: 'organization' });
8381
// orgId must not leak to fetcher
84-
expect(getPlansSpy.mock.calls[0][0].orgId).toBeUndefined();
82+
expect(getPlansSpy.mock.calls[0][0]).toStrictEqual({ for: 'organization', initialPage: 1, pageSize: 5 });
8583
expect(result.current.data.length).toBe(5);
8684
});
8785

@@ -95,7 +93,6 @@ describe('usePlans', () => {
9593

9694
expect(getPlansSpy).toHaveBeenCalledTimes(1);
9795
expect(getPlansSpy.mock.calls[0][0]).toStrictEqual({ for: 'user', pageSize: 4, initialPage: 1 });
98-
expect(getPlansSpy.mock.calls[0][0].orgId).toBeUndefined();
9996
await waitFor(() => expect(result.current.isLoading).toBe(false));
10097
expect(result.current.data.length).toBe(4);
10198
});
@@ -111,9 +108,8 @@ describe('usePlans', () => {
111108
await waitFor(() => expect(result.current.isLoading).toBe(true));
112109

113110
expect(getPlansSpy).toHaveBeenCalledTimes(1);
111+
// orgId must not leak to fetcher
114112
expect(getPlansSpy.mock.calls[0][0]).toStrictEqual({ for: 'organization', pageSize: 3, initialPage: 1 });
115-
// orgId should not leak to fetcher
116-
expect(getPlansSpy.mock.calls[0][0].orgId).toBeUndefined();
117113
expect(result.current.data.length).toBe(3);
118114
});
119115
});

‎packages/shared/src/react/hooks/createBillingPaginatedHook.tsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
8080
: ({
8181
initialPage: safeValues.initialPage,
8282
pageSize: safeValues.pageSize,
83-
...(_for === 'organization' ? { orgId: organization?.id } : {}),
83+
...(options?.unauthenticated ? {} : _for === 'organization' ? { orgId: organization?.id } : {}),
8484
} as TParams);
8585

8686
const isOrganization = _for === 'organization';
@@ -102,8 +102,13 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
102102
},
103103
{
104104
type: resourceType,
105-
userId: user?.id,
106-
...(_for === 'organization' ? { orgId: organization?.id } : {}),
105+
// userId: user?.id,
106+
...(options?.unauthenticated
107+
? {}
108+
: {
109+
userId: user?.id,
110+
...(_for === 'organization' ? { orgId: organization?.id } : {}),
111+
}),
107112
},
108113
);
109114

0 commit comments

Comments
 (0)