chore(shared,clerk-js): Set defaultOptions to queryClient #7285
Conversation
🦋 Changeset detectedLatest commit: bb5028c The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a pnpm catalog for Changes
Sequence DiagramsequenceDiagram
participant Component as React Component
participant Hook as useClerkQueryClient
participant Clerk as Clerk Instance
participant QueryClient as `@tanstack/query-core`
Component->>Hook: mount / call useClerkQueryClient()
Hook->>Clerk: read __internal_queryClient / subscribe (if implemented)
Note over Clerk: Query client may be uninitialized
Clerk->>Clerk: __internal_queryClient -> `#initQueryClient`()
Clerk->>QueryClient: dynamic import & create QueryClient (clerkQueryClientConfig)
QueryClient-->>Clerk: QueryClient instance
Clerk->>Clerk: wrap with RQ_CLIENT_TAG and emit "queryClientStatus"
Clerk-->>Hook: queryClientStatus event / updated __internal_queryClient
Hook-->>Component: set state { client, isLoaded: true } / re-render
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts (1)
62-69: Consider strengthening the type guard to validate theclientproperty.The
isTaggedRQClienttype guard checks for the__tagproperty but doesn't verify that theclientproperty exists or is aQueryClient. While this is likely safe given internal usage, adding a validation for theclientproperty would make the guard more robust.Apply this diff to strengthen the type guard:
const isTaggedRQClient = (value: unknown): value is ClerkRQClient => { return ( typeof value === 'object' && value !== null && '__tag' in (value as Record<string, unknown>) && - (value as Record<string, unknown>).__tag === 'clerk-rq-client' + (value as Record<string, unknown>).__tag === 'clerk-rq-client' && + 'client' in (value as Record<string, unknown>) ); };packages/clerk-js/src/core/clerk.ts (1)
299-316: Consider adding error handling for query client initialization.The
#initQueryClientmethod uses a fire-and-forget pattern with dynamic import but doesn't handle initialization failures. If the import fails or theQueryClientconstructor throws, the#queryClientremainsundefinedand thequeryClientStatusevent is never emitted, leaving consumers in a perpetual loading state.Apply this diff to add error handling:
#initQueryClient = (): void => { if (this.#queryClient) { return; } void import('./query-core') .then(module => module.QueryClient) .then(QueryClientCtor => { if (this.#queryClient) { return; } this.#queryClient = new QueryClientCtor(clerkQueryClientConfig); // @ts-expect-error - queryClientStatus is not typed this.#publicEventBus.emit('queryClientStatus', 'ready'); + }) + .catch(error => { + // Log initialization failure but don't throw + console.error('Failed to initialize query client:', error); + // Optionally emit error event + // @ts-expect-error - queryClientStatus is not typed + this.#publicEventBus.emit('queryClientStatus', 'error'); }); };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/thirty-pears-reply.md(1 hunks)packages/clerk-js/package.json(1 hunks)packages/clerk-js/src/core/clerk.ts(4 hunks)packages/clerk-js/src/test/mock-helpers.ts(1 hunks)packages/shared/package.json(1 hunks)packages/shared/src/react/clerk-rq/use-clerk-query-client.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/core/clerk.ts (1)
packages/clerk-js/src/core/query-core.ts (1)
QueryClient(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (9)
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts (2)
71-79: LGTM!The
getQueryClientStatefunction correctly handles both the initialized and uninitialized query client states, providing a mock client when the real one isn't ready yet.
81-101: LGTM!The event-driven approach for tracking query client initialization is well-implemented:
- Initial state is correctly derived
- Event subscription properly updates state
- Cleanup prevents memory leaks
- Falls back to mock client when not loaded
packages/shared/package.json (1)
166-166: LGTM!The migration to catalog-based resolution for
@tanstack/query-corealigns with the new catalog entry inpnpm-workspace.yamland maintains version consistency.pnpm-workspace.yaml (1)
21-23: LGTM!The new catalog entry for TanStack Query provides centralized version management across the workspace, which is a best practice for monorepos.
packages/clerk-js/src/test/mock-helpers.ts (1)
58-60: LGTM!Disabling refetch behaviors in test configuration improves test determinism and prevents flaky tests caused by automatic query refetches. This aligns with the production query client configuration.
packages/clerk-js/package.json (1)
74-74: LGTM!The migration to catalog-based resolution maintains version consistency across the workspace and aligns with the catalog entry in
pnpm-workspace.yaml.packages/clerk-js/src/core/clerk.ts (3)
97-97: LGTM!The type-only imports and discriminated union pattern using
RQ_CLIENT_TAGare well-designed and enable proper tree-shaking while maintaining type safety.Also applies to: 201-203
205-217: LGTM!The query client configuration is well-thought-out:
- Disables retry to use fapiClient's existing retry logic
- Disables automatic refetch on window focus/reconnect to reduce unnecessary requests
- Enables refetch on mount for stale data
- Clear comments explain the design decisions
264-273: LGTM!The getter pattern ensures lazy initialization while protecting against re-initialization via the guard in
#initQueryClient. Returnsundefinedsafely when the client isn't ready yet.
panteliselef
force-pushed
the
elef/consolidate-query-client-settings
branch
from
fab9ef2 to
3bb19da
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/clerk-js/src/core/clerk.ts (2)
201-217: Centralized query client config is clear and aligned with intentExtracting
RQ_CLIENT_TAG,ClerkRQClient, andclerkQueryClientConfigmakes the internal query client shape and its defaults explicit and easy to audit. The chosen defaults (no internal retries, no focus/reconnect refetch, refetch-on-mount enabled) fit well with the comment about delegating retry behavior tofapiClient.If you anticipate needing per-environment tuning later, you could consider threading an optional override into
#initQueryClient(or intoClerkOptions) and shallow-merging it intoclerkQueryClientConfig, but that’s not necessary for this PR.
264-316: Lazy query client init flow looks correct; consider logging import failuresThe interaction between
__internal_queryClientand#initQueryClientis sound: the getter kicks off async initialization, returns a tagged wrapper only once the client exists, and guards against duplicate construction if multiple calls race.One small improvement for observability would be to surface failures of the dynamic import or constructor, so consumers have something concrete to debug instead of a silently missing client. You can, for example, log via the existing
debugLogger:#initQueryClient = (): void => { if (this.#queryClient) { return; } void import('./query-core') .then(module => module.QueryClient) .then(QueryClientCtor => { if (this.#queryClient) { return; } this.#queryClient = new QueryClientCtor(clerkQueryClientConfig); // @ts-expect-error - queryClientStatus is not typed this.#publicEventBus.emit('queryClientStatus', 'ready'); - }); + }) + .catch(error => { + debugLogger.error('Failed to initialize QueryClient', { error }, 'clerk'); + }); };This keeps behavior the same on success while giving a clear signal in dev/production logs when something goes wrong during initialization.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/thirty-pears-reply.md(1 hunks)packages/clerk-js/package.json(1 hunks)packages/clerk-js/src/core/clerk.ts(4 hunks)packages/clerk-js/src/test/mock-helpers.ts(1 hunks)packages/shared/package.json(1 hunks)packages/shared/src/react/clerk-rq/use-clerk-query-client.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
- packages/clerk-js/package.json
- packages/clerk-js/src/test/mock-helpers.ts
- pnpm-workspace.yaml
- .changeset/thirty-pears-reply.md
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/core/clerk.ts (1)
packages/clerk-js/src/core/query-core.ts (1)
QueryClient(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/clerk-js/src/core/clerk.ts (1)
97-97: Type-only import for QueryClient/QueryClientConfig is appropriateUsing a
typeimport here keeps runtime bundling tied to the local./query-coredynamic import while still giving the class strong typing for#queryClientandClerkRQClient. This looks correct and consistent with the lazy-initialization approach.
panteliselef
force-pushed
the
elef/consolidate-query-client-settings
branch
from
3bb19da to
9bd928b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/clerk.ts (1)
267-276: Document the lazy initialization behavior and event-driven pattern.The getter triggers asynchronous initialization but returns synchronously, which may return
undefineduntil the dynamic import completes. Consumers should listen to the'queryClientStatus'event to know when the client is ready.Add JSDoc to document this behavior:
+ /** + * Internal query client accessor with lazy initialization. + * + * @remarks + * - First access triggers asynchronous initialization via dynamic import + * - Returns undefined until initialization completes + * - Listen to the 'queryClientStatus' event (payload: 'ready') to know when available + * - Safe to call multiple times; initialization happens only once + * + * @returns ClerkRQClient wrapper with tag discriminator, or undefined if not yet initialized + * + * @internal + */ get __internal_queryClient(): ClerkRQClient | undefined {Based on coding guidelines: Document functions with JSDoc comments including @param, @returns, @throws, and @example tags.
🧹 Nitpick comments (1)
packages/clerk-js/src/core/clerk.ts (1)
316-317: Type thequeryClientStatusevent properly instead of using@ts-expect-error.The event emission uses
@ts-expect-errorbecause the event isn't typed in the event bus. This should be properly typed to maintain type safety and enable IntelliSense for event listeners.Consider adding the event type to the event bus interface definition. The event should have a payload type of
'ready' | 'error'(with error support if you add error handling per the previous comment).Based on coding guidelines: Avoid
anytypes without justification, and use proper TypeScript error types.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/thirty-pears-reply.md(1 hunks)packages/clerk-js/package.json(1 hunks)packages/clerk-js/src/core/clerk.ts(4 hunks)packages/clerk-js/src/test/mock-helpers.ts(1 hunks)packages/shared/package.json(1 hunks)packages/shared/src/react/clerk-rq/use-clerk-query-client.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- pnpm-workspace.yaml
- packages/shared/package.json
- packages/clerk-js/src/test/mock-helpers.ts
- packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/clerk-js/package.jsonpackages/clerk-js/src/core/clerk.ts
packages/*/package.json
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/*/package.json: Packages should export TypeScript types alongside runtime code
Follow semantic versioning for all packagesAll packages must be published under @clerk namespace
Files:
packages/clerk-js/package.json
**/package.json
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Use pnpm as the package manager for this monorepo
Files:
packages/clerk-js/package.json
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/clerk-js/src/core/clerk.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/clerk-js/src/core/clerk.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/clerk-js/src/core/clerk.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/clerk-js/src/core/clerk.ts
🧬 Code graph analysis (1)
packages/clerk-js/src/core/clerk.ts (1)
packages/clerk-js/src/core/query-core.ts (1)
QueryClient(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
.changeset/thirty-pears-reply.md (1)
1-6: LGTM! Changeset properly documents the change.The changeset correctly marks both affected packages with patch-level changes and provides a clear description.
packages/clerk-js/src/core/clerk.ts (1)
100-100: LGTM! Import addition for QueryClientConfig.The QueryClientConfig type import is properly added and used for typing the clerkQueryClientConfig constant.
packages/clerk-js/package.json (1)
73-73: Catalog entry verified and correctly configured.The
@tanstack/query-coredependency correctly referencescatalog:tanstack, which is defined inpnpm-workspace.yamlwith version 5.87.4. Bothpackages/clerk-jsandpackages/shareduse the same catalog reference, ensuring consistent versioning across the monorepo.
| const clerkQueryClientConfig: QueryClientConfig = { | ||
| defaultOptions: { | ||
| queries: { | ||
| // use the retry logic that fapiClient uses | ||
| retry: false, | ||
| // Note: to refetch onWindowFocus, you need to call `queryClient.mount()` | ||
| refetchOnWindowFocus: false, | ||
| refetchOnReconnect: false, | ||
| // the query will refetch on mount if the data is stale | ||
| refetchOnMount: true, | ||
| }, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add JSDoc documentation for the exported configuration.
The clerkQueryClientConfig constant is exported but lacks JSDoc documentation. Per coding guidelines, all public APIs must be documented with JSDoc including description, usage notes, and any important behavioral details.
Apply this diff to add documentation:
+/**
+ * Default configuration for the Clerk QueryClient instance.
+ *
+ * @remarks
+ * - Retries are disabled to use the retry logic from fapiClient
+ * - Window focus and reconnect refetching are disabled by default
+ * - Mount refetching is enabled to fetch stale data on component mount
+ *
+ * @internal
+ */
const clerkQueryClientConfig: QueryClientConfig = {
defaultOptions: {
queries: {
// use the retry logic that fapiClient uses
retry: false,
// Note: to refetch onWindowFocus, you need to call `queryClient.mount()`
refetchOnWindowFocus: false,
refetchOnReconnect: false,
// the query will refetch on mount if the data is stale
refetchOnMount: true,
},
},
};Based on coding guidelines: All public APIs must be documented with JSDoc.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const clerkQueryClientConfig: QueryClientConfig = { | |
| defaultOptions: { | |
| queries: { | |
| // use the retry logic that fapiClient uses | |
| retry: false, | |
| // Note: to refetch onWindowFocus, you need to call `queryClient.mount()` | |
| refetchOnWindowFocus: false, | |
| refetchOnReconnect: false, | |
| // the query will refetch on mount if the data is stale | |
| refetchOnMount: true, | |
| }, | |
| }, | |
| }; | |
| /** | |
| * Default configuration for the Clerk QueryClient instance. | |
| * | |
| * @remarks | |
| * - Retries are disabled to use the retry logic from fapiClient | |
| * - Window focus and reconnect refetching are disabled by default | |
| * - Mount refetching is enabled to fetch stale data on component mount | |
| * | |
| * @internal | |
| */ | |
| const clerkQueryClientConfig: QueryClientConfig = { | |
| defaultOptions: { | |
| queries: { | |
| // use the retry logic that fapiClient uses | |
| retry: false, | |
| // Note: to refetch onWindowFocus, you need to call `queryClient.mount()` | |
| refetchOnWindowFocus: false, | |
| refetchOnReconnect: false, | |
| // the query will refetch on mount if the data is stale | |
| refetchOnMount: true, | |
| }, | |
| }, | |
| }; |
| #initQueryClient = (): void => { | ||
| if (this.#queryClient) { | ||
| return; | ||
| } | ||
|
|
||
| void import('./query-core') | ||
| .then(module => module.QueryClient) | ||
| .then(QueryClientCtor => { | ||
| if (this.#queryClient) { | ||
| return; | ||
| } | ||
|
|
||
| this.#queryClient = new QueryClientCtor(clerkQueryClientConfig); | ||
|
|
||
| // @ts-expect-error - queryClientStatus is not typed | ||
| this.#publicEventBus.emit('queryClientStatus', 'ready'); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Add error handling for dynamic import failures.
The #initQueryClient method lacks error handling if the dynamic import fails. This could occur due to network issues, build problems, or missing modules, leading to silent failures where features depending on the query client won't work without any diagnostic information.
Apply this diff to add error handling:
#initQueryClient = (): void => {
if (this.#queryClient) {
return;
}
void import('./query-core')
.then(module => module.QueryClient)
.then(QueryClientCtor => {
if (this.#queryClient) {
return;
}
this.#queryClient = new QueryClientCtor(clerkQueryClientConfig);
// @ts-expect-error - queryClientStatus is not typed
this.#publicEventBus.emit('queryClientStatus', 'ready');
- });
+ })
+ .catch(err => {
+ console.error('Clerk: Failed to initialize QueryClient', err);
+ // @ts-expect-error - queryClientStatus is not typed
+ this.#publicEventBus.emit('queryClientStatus', 'error');
+ });
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #initQueryClient = (): void => { | |
| if (this.#queryClient) { | |
| return; | |
| } | |
| void import('./query-core') | |
| .then(module => module.QueryClient) | |
| .then(QueryClientCtor => { | |
| if (this.#queryClient) { | |
| return; | |
| } | |
| this.#queryClient = new QueryClientCtor(clerkQueryClientConfig); | |
| // @ts-expect-error - queryClientStatus is not typed | |
| this.#publicEventBus.emit('queryClientStatus', 'ready'); | |
| }); | |
| }; | |
| #initQueryClient = (): void => { | |
| if (this.#queryClient) { | |
| return; | |
| } | |
| void import('./query-core') | |
| .then(module => module.QueryClient) | |
| .then(QueryClientCtor => { | |
| if (this.#queryClient) { | |
| return; | |
| } | |
| this.#queryClient = new QueryClientCtor(clerkQueryClientConfig); | |
| // @ts-expect-error - queryClientStatus is not typed | |
| this.#publicEventBus.emit('queryClientStatus', 'ready'); | |
| }) | |
| .catch(err => { | |
| console.error('Clerk: Failed to initialize QueryClient', err); | |
| // @ts-expect-error - queryClientStatus is not typed | |
| this.#publicEventBus.emit('queryClientStatus', 'error'); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/clerk.ts around lines 302 to 319, the dynamic
import promise in #initQueryClient has no error handling so import failures are
silent; add a .catch handler to the import chain that logs the error (e.g.,
console.error or an existing logger) and emits a failure status on the public
event bus (e.g., this.#publicEventBus.emit('queryClientStatus', 'error', error)
or similar) so consumers can react, and ensure the handler prevents further
initialization when an error occurs.
…ttings' into elef/consolidate-query-client-settings
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts (2)
66-84: Extract type checking logic into a type guard function.The type checking logic
typeof queryClient === 'object' && '__tag' in queryClient && queryClient.__tag === 'clerk-rq-client'is duplicated on lines 68 and 81.Apply this refactor to improve maintainability and follow the type guard pattern:
+function isClerkRQClient(value: unknown): value is ClerkRQClient { + return ( + value !== null && + typeof value === 'object' && + '__tag' in value && + value.__tag === 'clerk-rq-client' + ); +} + const useClerkQueryClient = (): [QueryClient, boolean] => { const clerk = useClerkInstanceContext(); // @ts-expect-error - __internal_queryClient is not typed const queryClient = clerk.__internal_queryClient as ClerkRQClient | undefined; - const [, setQueryClientLoaded] = useState( - typeof queryClient === 'object' && '__tag' in queryClient && queryClient.__tag === 'clerk-rq-client', - ); + const [, setQueryClientLoaded] = useState(isClerkRQClient(queryClient)); useEffect(() => { const _setQueryClientLoaded = () => setQueryClientLoaded(true); // @ts-expect-error - queryClientStatus is not typed clerk.on('queryClientStatus', _setQueryClientLoaded); return () => { // @ts-expect-error - queryClientStatus is not typed clerk.off('queryClientStatus', _setQueryClientLoaded); }; }, [clerk, setQueryClientLoaded]); - const isLoaded = typeof queryClient === 'object' && '__tag' in queryClient && queryClient.__tag === 'clerk-rq-client'; + const isLoaded = isClerkRQClient(queryClient); return [queryClient?.client || mockQueryClient, isLoaded]; };Note: Added
value !== nullcheck for defensive programming, astypeof null === 'object'in JavaScript.As per coding guidelines, implement type guards for type checking patterns.
62-62: Add JSDoc documentation for the exported function.The
useClerkQueryClientfunction is exported but lacks JSDoc documentation. The return tuple's meaning (QueryClient and loading state) should be documented for developers using this API.Apply this diff to add documentation:
+/** + * Returns the Clerk-managed QueryClient instance and its loading state. + * + * @returns A tuple containing: + * - [0]: The QueryClient instance (or a mock if not loaded) + * - [1]: A boolean indicating whether the real QueryClient is loaded + * + * @example + * ```tsx + * const [queryClient, isLoaded] = useClerkQueryClient(); + * if (isLoaded) { + * // Use the real QueryClient + * } + * ``` + */ const useClerkQueryClient = (): [QueryClient, boolean] => {As per coding guidelines, all public APIs must be documented with JSDoc including @returns and @example tags.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Prefer importing types from
@clerk/shared/typesinstead of the deprecated@clerk/typesalias
Files:
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts
🧬 Code graph analysis (1)
packages/shared/src/react/clerk-rq/use-clerk-query-client.ts (2)
packages/clerk-js/src/core/query-core.ts (1)
QueryClient(3-3)packages/shared/src/react/contexts.tsx (1)
useClerkInstanceContext(128-128)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Bug Fixes
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.