-
Notifications
You must be signed in to change notification settings - Fork 422
chore(clerk-js, shared): Remove SWR from clerk-js and create variants for existing internal hooks #7270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
chore(clerk-js, shared): Remove SWR from clerk-js and create variants for existing internal hooks #7270
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
2f87ceb
compat with internal hooks
panteliselef 5855d81
fix unit tests
panteliselef c10e94d
clean up
panteliselef 5c4823d
handle initData
panteliselef c258f7a
remove useSWRMutation from APIKeys.tsx
panteliselef a9359ad
rq compatibility for PaymentElement
panteliselef 7f118ff
run rq e2es for machine
panteliselef 86373fc
compatibility layer for revalidation of useAPIKeys
panteliselef 94ef80b
add tests for invalidations in api keys
panteliselef 42fda7b
drop SWRConfig from the compat rq variant
panteliselef bc36ccd
cleanup
panteliselef 121977e
computedValues in usePagesOrInfinite.rq.tsx
panteliselef 3b52ab4
patch tests
panteliselef 10c1b5f
add changeset
panteliselef 8141be7
fix useBillingHookEnabled.ts
panteliselef acbc638
fix tests
panteliselef dc4843d
remove comments
panteliselef d9a307c
Merge branch 'main' into elef/remove-swr-from-main
panteliselef f820c96
remove 'use client'
panteliselef File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /** | ||
| * @internal | ||
| */ | ||
| export function defineKeepPreviousDataFn(enabled: boolean) { | ||
| if (enabled) { | ||
| return function KeepPreviousDataFn<Data>(previousData: Data): Data { | ||
| return previousData; | ||
| }; | ||
| } | ||
| return undefined; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
62 changes: 62 additions & 0 deletions
62
packages/shared/src/react/hooks/usePaymentAttemptQuery.rq.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import { eventMethodCalled } from '../../telemetry/events'; | ||
| import { defineKeepPreviousDataFn } from '../clerk-rq/keep-previous-data'; | ||
| import { useClerkQuery } from '../clerk-rq/useQuery'; | ||
| import { | ||
| useAssertWrappedByClerkProvider, | ||
| useClerkInstanceContext, | ||
| useOrganizationContext, | ||
| useUserContext, | ||
| } from '../contexts'; | ||
| import { usePaymentAttemptQueryCacheKeys } from './usePaymentAttemptQuery.shared'; | ||
| import type { PaymentAttemptQueryResult, UsePaymentAttemptQueryParams } from './usePaymentAttemptQuery.types'; | ||
|
|
||
| const HOOK_NAME = 'usePaymentAttemptQuery'; | ||
|
|
||
| /** | ||
| * This is the new implementation of usePaymentAttemptQuery using React Query. | ||
| * It is exported only if the package is built with the `CLERK_USE_RQ` environment variable set to `true`. | ||
| * | ||
| * @internal | ||
| */ | ||
| function usePaymentAttemptQuery(params: UsePaymentAttemptQueryParams): PaymentAttemptQueryResult { | ||
| useAssertWrappedByClerkProvider(HOOK_NAME); | ||
|
|
||
| const { paymentAttemptId, enabled = true, keepPreviousData = false, for: forType = 'user' } = params; | ||
| const clerk = useClerkInstanceContext(); | ||
| const user = useUserContext(); | ||
| const { organization } = useOrganizationContext(); | ||
|
|
||
| clerk.telemetry?.record(eventMethodCalled(HOOK_NAME)); | ||
|
|
||
| const organizationId = forType === 'organization' ? (organization?.id ?? null) : null; | ||
| const userId = user?.id ?? null; | ||
|
|
||
| const { queryKey } = usePaymentAttemptQueryCacheKeys({ | ||
| paymentAttemptId, | ||
| userId, | ||
| orgId: organizationId, | ||
| for: forType, | ||
| }); | ||
|
|
||
| const queryEnabled = Boolean(paymentAttemptId) && enabled && (forType !== 'organization' || Boolean(organizationId)); | ||
|
|
||
| const query = useClerkQuery({ | ||
| queryKey, | ||
| queryFn: ({ queryKey }) => { | ||
| const args = queryKey[3].args; | ||
| return clerk.billing.getPaymentAttempt(args); | ||
| }, | ||
| enabled: queryEnabled, | ||
| placeholderData: defineKeepPreviousDataFn(keepPreviousData), | ||
| staleTime: 1_000 * 60, | ||
| }); | ||
|
|
||
| return { | ||
| data: query.data, | ||
| error: (query.error ?? null) as PaymentAttemptQueryResult['error'], | ||
| isLoading: query.isLoading, | ||
| isFetching: query.isFetching, | ||
| }; | ||
| } | ||
|
|
||
| export { usePaymentAttemptQuery as __internal_usePaymentAttemptQuery }; | ||
32 changes: 32 additions & 0 deletions
32
packages/shared/src/react/hooks/usePaymentAttemptQuery.shared.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import { useMemo } from 'react'; | ||
|
|
||
| import type { ForPayerType } from '../../types'; | ||
| import { INTERNAL_STABLE_KEYS } from '../stable-keys'; | ||
| import { createCacheKeys } from './createCacheKeys'; | ||
|
|
||
| export function usePaymentAttemptQueryCacheKeys(params: { | ||
| paymentAttemptId: string; | ||
| userId: string | null; | ||
| orgId: string | null; | ||
| for?: ForPayerType; | ||
| }) { | ||
| const { paymentAttemptId, userId, orgId, for: forType } = params; | ||
| return useMemo(() => { | ||
| return createCacheKeys({ | ||
| stablePrefix: INTERNAL_STABLE_KEYS.PAYMENT_ATTEMPT_KEY, | ||
| authenticated: true, | ||
| tracked: { | ||
| paymentAttemptId, | ||
| forType, | ||
| userId, | ||
| orgId, | ||
| }, | ||
| untracked: { | ||
| args: { | ||
| id: paymentAttemptId ?? undefined, | ||
| orgId: orgId ?? undefined, | ||
| }, | ||
| }, | ||
| }); | ||
| }, [paymentAttemptId, forType, userId, orgId]); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify error type compatibility.
Line 44 uses a type assertion to coerce
query.errortoPaymentAttemptQueryResult['error'](which isClerkAPIResponseError | null). This bypasses TypeScript's type checking and could hide type mismatches if the underlying query error type doesn't matchClerkAPIResponseError.Please verify that errors thrown by
clerk.billing.getPaymentAttemptare actually of typeClerkAPIResponseErrorand that the error handling in the query library preserves this type. Consider removing the type assertion if types are compatible, or add runtime validation if they might differ.🏁 Script executed:
Length of output: 2693
🏁 Script executed:
Length of output: 1248
🏁 Script executed:
Length of output: 1831
🏁 Script executed:
Length of output: 1329
🏁 Script executed:
Length of output: 24853
🏁 Script executed:
Length of output: 6438
Add explicit TError parameter to useClerkQuery or validate error types at runtime.
The type assertion on line 44 masks a real type mismatch.
useClerkQuerydefaults toTError = DefaultError(from @tanstack/query-core), but the function returnserror: ClerkAPIResponseError | null. This same pattern affectsuseStatementQueryandusePlanDetailsQuery.Fix by either:
TErrorparameter:useClerkQuery<..., ClerkAPIResponseError>({...})clerk.billing.getPaymentAttempterrors areClerkAPIResponseErrorSimilar hooks like
useSubscription.rq.tsxhandle this differently (converting toundefinedwithout type assertion), suggesting inconsistent error handling across the codebase.🤖 Prompt for AI Agents