Skip to content

Commit 791ff19

Browse files
authored
fix(shared): Add for in cache key for unauthed hooks (#7067)
1 parent d1a186c commit 791ff19

File tree

3 files changed

+87
-9
lines changed

3 files changed

+87
-9
lines changed

.changeset/bitter-paths-march.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@clerk/shared': patch
3+
---
4+
5+
Fixes a bug where `usePlans()` would display stale data even if the `for` property has changed.

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

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { renderHook, waitFor } from '@testing-library/react';
1+
import { render, renderHook, screen, waitFor } from '@testing-library/react';
2+
import React from 'react';
23
import { beforeEach, describe, expect, it, vi } from 'vitest';
34

45
const mockUser: any = { id: 'user_1' };
@@ -7,7 +8,10 @@ const mockOrganization: any = { id: 'org_1' };
78
const getPlansSpy = vi.fn((args: any) =>
89
Promise.resolve({
910
// pageSize maps to limit; default to 10 if missing
10-
data: Array.from({ length: args.limit ?? args.pageSize ?? 10 }, (_, i) => ({ id: `plan_${i + 1}` })),
11+
data: Array.from<Partial<BillingPlanResource>, Partial<BillingPlanResource>>(
12+
{ length: args.limit ?? args.pageSize ?? 10 },
13+
(_, i) => ({ id: `plan_${i + 1}`, forPayerType: args.for }),
14+
),
1115
total_count: 25,
1216
}),
1317
);
@@ -37,6 +41,8 @@ vi.mock('../../contexts', () => {
3741
};
3842
});
3943

44+
import type { BillingPlanResource } from '@clerk/types';
45+
4046
import { usePlans } from '../usePlans';
4147
import { wrapper } from './wrapper';
4248

@@ -112,4 +118,69 @@ describe('usePlans', () => {
112118
expect(getPlansSpy.mock.calls[0][0]).toStrictEqual({ for: 'organization', pageSize: 3, initialPage: 1 });
113119
expect(result.current.data.length).toBe(3);
114120
});
121+
122+
it('mounts user and organization hooks together and renders their respective data', async () => {
123+
const DualPlans = () => {
124+
const userPlans = usePlans({ initialPage: 1, pageSize: 2 });
125+
const orgPlans = usePlans({ initialPage: 1, pageSize: 2, for: 'organization' } as any);
126+
127+
return (
128+
<>
129+
<div data-testid='user-count'>{userPlans.data.length}</div>
130+
<div data-testid='org-count'>{orgPlans.data.length}</div>
131+
</>
132+
);
133+
};
134+
135+
render(<DualPlans />, { wrapper });
136+
137+
await waitFor(() => expect(screen.getByTestId('user-count').textContent).toBe('2'));
138+
await waitFor(() => expect(screen.getByTestId('org-count').textContent).toBe('2'));
139+
140+
expect(getPlansSpy).toHaveBeenCalledTimes(2);
141+
const calls = getPlansSpy.mock.calls.map(c => c[0]);
142+
expect(calls).toEqual(
143+
expect.arrayContaining([
144+
{ for: 'user', initialPage: 1, pageSize: 2 },
145+
{ for: 'organization', initialPage: 1, pageSize: 2 },
146+
]),
147+
);
148+
149+
// Ensure orgId does not leak into the fetcher params
150+
for (const call of calls) {
151+
expect(call).not.toHaveProperty('orgId');
152+
}
153+
});
154+
155+
it('conditionally renders hooks based on prop passed to render', async () => {
156+
const UserPlansCount = () => {
157+
const userPlans = usePlans({ initialPage: 1, pageSize: 2 });
158+
return <div data-testid='user-type'>{userPlans.data.map(p => p.forPayerType)[0]}</div>;
159+
};
160+
161+
const OrgPlansCount = () => {
162+
const orgPlans = usePlans({ initialPage: 1, pageSize: 2, for: 'organization' } as any);
163+
return <div data-testid='org-type'>{orgPlans.data.map(p => p.forPayerType)[0]}</div>;
164+
};
165+
166+
const Conditional = ({ showOrg }: { showOrg: boolean }) => (showOrg ? <OrgPlansCount /> : <UserPlansCount />);
167+
168+
const { rerender } = render(<Conditional showOrg={false} />, { wrapper });
169+
170+
await waitFor(() => expect(screen.getByTestId('user-type').textContent).toBe('user'));
171+
expect(getPlansSpy).toHaveBeenCalledTimes(1);
172+
expect(getPlansSpy.mock.calls[0][0]).toStrictEqual({ for: 'user', initialPage: 1, pageSize: 2 });
173+
174+
rerender(<Conditional showOrg />);
175+
176+
await waitFor(() => expect(screen.getByTestId('org-type').textContent).toBe('organization'));
177+
expect(getPlansSpy).toHaveBeenCalledTimes(2);
178+
const calls = getPlansSpy.mock.calls.map(c => c[0]);
179+
expect(calls).toEqual(
180+
expect.arrayContaining([
181+
{ for: 'user', initialPage: 1, pageSize: 2 },
182+
{ for: 'organization', initialPage: 1, pageSize: 2 },
183+
]),
184+
);
185+
});
115186
});

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,11 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
5353
): PaginatedResources<TResource, T extends { infinite: true } ? true : false> {
5454
const { for: _for, ...paginationParams } = params || ({} as Partial<T>);
5555

56+
const safeFor = _for || 'user';
57+
5658
useAssertWrappedByClerkProvider(hookName);
5759

58-
const fetchFn = useFetcher(_for || 'user');
60+
const fetchFn = useFetcher(safeFor);
5961

6062
const safeValues = useWithSafeValues(paginationParams, {
6163
initialPage: 1,
@@ -74,17 +76,18 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
7476

7577
clerk.telemetry?.record(eventMethodCalled(hookName));
7678

79+
const isForOrganization = safeFor === 'organization';
80+
7781
const hookParams =
7882
typeof paginationParams === 'undefined'
7983
? undefined
8084
: ({
8185
initialPage: safeValues.initialPage,
8286
pageSize: safeValues.pageSize,
83-
...(options?.unauthenticated ? {} : _for === 'organization' ? { orgId: organization?.id } : {}),
87+
...(options?.unauthenticated ? {} : isForOrganization ? { orgId: organization?.id } : {}),
8488
} as TParams);
8589

86-
const isOrganization = _for === 'organization';
87-
const billingEnabled = isOrganization
90+
const billingEnabled = isForOrganization
8891
? environment?.commerceSettings.billing.organization.enabled
8992
: environment?.commerceSettings.billing.user.enabled;
9093

@@ -102,12 +105,11 @@ export function createBillingPaginatedHook<TResource extends ClerkResource, TPar
102105
},
103106
{
104107
type: resourceType,
105-
// userId: user?.id,
106108
...(options?.unauthenticated
107-
? {}
109+
? { for: safeFor }
108110
: {
109111
userId: user?.id,
110-
...(_for === 'organization' ? { orgId: organization?.id } : {}),
112+
...(isForOrganization ? { orgId: organization?.id } : {}),
111113
}),
112114
},
113115
);

0 commit comments

Comments
 (0)