-
Notifications
You must be signed in to change notification settings - Fork 417
feat(clerk-js,shared,react,nextjs): Migrate to useSyncExternalStore #7411
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
base: main
Are you sure you want to change the base?
feat(clerk-js,shared,react,nextjs): Migrate to useSyncExternalStore #7411
Conversation
…uth instead of at the ClerkProvider level
… react package to use it
WalkthroughCentralizes Clerk state emission into a single Resources payload, adds listener options (skipInitialEmit), replaces several context exports with hook-based base hooks using useSyncExternalStore, introduces a shared ClerkContextProvider, removes PromisifiedAuthProvider/CoreClerkContextWrapper, and updates Next.js/Expo integrations and related typings. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / Consumer
participant Provider as ClerkContextProvider
participant Clerk as IsomorphicClerk (clerk-js)
participant Hook as use*Base hooks
participant UI as UI Consumers
App->>Provider: mount with clerk & initialState
Provider->>Clerk: ensure singleton & provide contexts
Provider->>Hook: render children with InitialStateProvider & ClerkInstanceContext
Hook->>Clerk: subscribe(listener, {skipInitialEmit: true})
Note over Clerk,Hook: Clerk state changes (session/user/org)
Clerk->>Clerk: `#updateAccessors`(...) -> build Resources
Clerk->>Clerk: __emit(resources) and cache __internal_lastEmittedResources
Clerk->>Hook: invoke listener with Resources
Hook->>UI: useSyncExternalStore supplies snapshot
UI->>UI: re-render with updated state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (25)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
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.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/types/clerk.ts (1)
690-701: Update JSDoc to document the new options parameter.The
addListenersignature has been updated to accept an optionaloptionsparameter, but the JSDoc doesn't document this new parameter or theskipInitialEmitflag behavior.Apply this diff to update the JSDoc:
/** * Register a listener that triggers a callback each time important Clerk resources are changed. * Allows to hook up at different steps in the sign up, sign in processes. * * Some important checkpoints: * When there is an active session, user === session.user. * When there is no active session, user and session will both be null. * When a session is loading, user and session will be undefined. * * @param callback - Callback function receiving the most updated Clerk resources after a change. + * @param options - Optional configuration for the listener + * @param options.skipInitialEmit - If true, the callback will not be invoked immediately with the current state * @returns - Unsubscribe callback */ addListener: (callback: ListenerCallback, options?: ListenerOptions) => UnsubscribeCallback;Based on coding guidelines: "All public APIs must be documented with JSDoc".
♻️ Duplicate comments (1)
packages/shared/src/react/__tests__/payment-element.test.tsx (1)
265-269: Repeated provider tree in locale‑normalization testSame provider wiring as previous cases and correctly parameterized with per‑test
options; behavior is consistent and clear. Any future refactor to DRY the render tree (e.g., a smallrenderPaymentElementhelper) would apply here as well but is optional.
🧹 Nitpick comments (10)
packages/ui/src/contexts/CoreClientContext.tsx (2)
6-7: Consider updating the assertion message in the same PRThe TODO is accurate: if
useClientContextno longer reads from an actual React context, the'ClientContext'label inassertContextExistsmay be misleading. Consider updating the message (or assertion helper) now to reflect the new source ofctx, or at least referencing the new provider surface in the eventual wording.
13-14: Mirror the updated assertion/message behavior between sign‑in and sign‑upWhatever change you make to the assertion/message in
useCoreSignInshould also be applied here so both hooks report consistent guidance whenctxis missing or invalid.packages/react/src/contexts/IsomorphicClerkContext.tsx (1)
1-5: Typed hook alias looks good; consider JSDoc and tightening the castThis achieves the goal of exposing a typed
useIsomorphicClerkContextalias overuseClerkInstanceContext, and the type-only import is in line with the guidelines.Two small follow-ups to consider:
Add JSDoc for the public hook
Per the guidelines, public APIs should be documented. A brief JSDoc describing that this hook returns the currentIsomorphicClerkinstance (and any caveats about calling it outside a provider) would help:/** * Returns the current IsomorphicClerk instance from context. * @returns The active IsomorphicClerk instance. */ export const useIsomorphicClerkContext = …Revisit the
unknowndouble-cast
TheuseClerkInstanceContext as unknown as () => IsomorphicClerkpattern is an unchecked narrowing; if the underlying hook’s return type ever diverges fromIsomorphicClerk, TypeScript won’t flag it. If the types are actually compatible, you can potentially remove theunknownhop and/or make the intent clearer by giving the const an explicit type:export const useIsomorphicClerkContext: () => IsomorphicClerk = useClerkInstanceContext as unknown as () => IsomorphicClerk;or, if assignable:
export const useIsomorphicClerkContext: () => IsomorphicClerk = useClerkInstanceContext as () => IsomorphicClerk;Worth double-checking the
useClerkInstanceContextsignature to see if a direct or generic typing there would avoid the need forunknown.packages/shared/src/react/__tests__/payment-element.test.tsx (1)
227-231: Inline provider usage for default-locale case is fine; consider minor DRYingThis inline
OptionsContext.Providertree is correct and matches the helper above; the emptyoptionsobject is appropriate for the “no localization provided” case the test covers. If you touch this file again, you might consider reusing a shared render helper that accepts anoptionsoverride to reduce duplication, but it’s not required.packages/nextjs/src/utils/mergeNextClerkPropsWithEnv.ts (2)
7-9: Add explicit return type for this exported util
mergeNextClerkPropsWithEnvis exported and currently relies on inference; to make the public surface more stable, consider annotating the return type explicitly while keeping thesatisfiescheck:-export const mergeNextClerkPropsWithEnv = <TUi extends Ui = Ui>( - props: Omit<NextClerkProviderProps<TUi>, 'children'>, -) => { +export const mergeNextClerkPropsWithEnv = <TUi extends Ui = Ui>( + props: Omit<NextClerkProviderProps<TUi>, 'children'>, +): Omit<NextClerkProviderProps<TUi>, 'children'> => {This locks in the intended API shape without changing runtime behavior. As per coding guidelines, explicit return types are preferred for public APIs.
Also applies to: 36-36
14-14: Tightenanyusage and boolean fallback semanticsTwo small follow‑ups worth considering here:
- Line 14:
(props as any).clerkUiUrlbypasses the stronger typing you just introduced. Prefer extendingNextClerkProviderProps(or a dedicated internal type) to includeclerkUiUrl?: stringso you can drop theanycast and keep full type‑safety.- Line 18:
props.isSatellite || isTruthy(...)treats an explicitfalsethe same as “unset” and may unexpectedly let the env var override an intentionalfalse. If you want “explicit false wins”, switch to nullish coalescing:- isSatellite: props.isSatellite || isTruthy(process.env.NEXT_PUBLIC_CLERK_IS_SATELLITE), + isSatellite: props.isSatellite ?? isTruthy(process.env.NEXT_PUBLIC_CLERK_IS_SATELLITE),Both changes would better align this helper with the “no
any” and nullish‑coalescing preferences. As per coding guidelines, avoidanyand prefer??for this kind of config.Also applies to: 18-18
packages/react/src/hooks/__tests__/useAuth.test.tsx (1)
71-75: Consider providing more explicit test fixtures.The test setup uses
{} as anyfor theinitialStateprop. While this works for basic rendering tests, providing more explicit mock data would improve test isolation and clarity.Consider creating a helper for test fixtures:
const createMockInitialState = (overrides = {}) => ({ client: null, session: null, user: null, organization: null, ...overrides, });Then use it in the test:
-<InitialStateProvider initialState={{} as any}> +<InitialStateProvider initialState={createMockInitialState()}> <TestComponent /> </InitialStateProvider>This would make test expectations clearer and easier to maintain as the InitialState type evolves.
packages/react/src/contexts/ClerkProvider.tsx (1)
13-23: ClerkProvider re-wiring viaClerkContextProviderlooks right; address the@ts-expect-errorgapThe new flow:
- Build an
IsomorphicClerkinstance viauseLoadedIsomorphicClerk.- Feed
clerkandclerkStatusinto the sharedClerkContextProvider.- Let the shared provider handle InitialState, SWR, and the singleton instance context.
This is a solid direction and keeps React’s tree concerns in the shared layer.
The one concern here is the explicit
// @ts-expect-error - Fixme!on theclerkprop:<ClerkContextProvider initialState={initialState} // @ts-expect-error - Fixme! clerk={isomorphicClerk} clerkStatus={clerkStatus} >Relying on a permanent
@ts-expect-errorfor a core prop weakens guarantees for all downstream consumers ofClerkInstanceContext. It would be good to either:
- Align the types (e.g. make
IsomorphicClerkimplement/extend theClerkinterface or narrow the prop type to whatIsomorphicClerkactually provides), or- Introduce a dedicated
IsomorphicClerkLiketype that both the context andIsomorphicClerkshare, then use that forclerk.Even if you don’t fix it in this PR, a TODO with a concrete plan (or an issue link) would help ensure it gets addressed.
packages/shared/src/react/ClerkContextProvider.tsx (1)
12-46: Context composition looks solid; tightenswrConfigtyping and consider exporting props typeThe provider wiring (InitialStateProvider → ClerkInstanceContext → SWRConfigCompat → CheckoutProvider) and the memoization keyed by
clerkStatusare well aligned with the “mutable singleton + status‑driven referential integrity” design.Two refinements to keep this aligned with the project’s TS guidelines:
Avoid
anyonswrConfigtype ClerkContextProps = { clerk: Clerk; clerkStatus?: ClerkStatus; children: React.ReactNode; swrConfig?: any; initialState?: InitialState; };If possible, use the actual SWR config type (e.g.
SWRConfigurationor a narrowed subset) or fall back tounknownwith a runtime cast at the call site. This keeps the shared surface type‑safe.Public API typing / docs
Since
ClerkContextProvideris re‑exported from./index.ts, it’s effectively part of the shared public API. ExportingClerkContextPropsand adding a short JSDoc onClerkContextProvider(documentingclerk,clerkStatus,initialState, andswrConfig) would make the contract explicit for SDK consumers.packages/clerk-js/src/core/clerk.ts (1)
2978-2992: Document thedangerouslySkipEmitinvariant more prominently.The JSDoc comment explains the danger, but the name alone may not convey the invariant that callers must call
#emit()themselves. Consider adding an assertion or more explicit documentation about when this is safe to use./** * Updates the accessors for the Clerk singleton and emits. * If dangerouslySkipEmit is true, the emit will be skipped and you are responsible for calling #emit() yourself. This is dangerous because if there is a gap between setting these and emitting, library consumers that both read state directly and set up listeners could end up in a inconsistent state. + * + * @internal Only use dangerouslySkipEmit when you need to emit a different event (like TokenUpdate) before the state change emission. */ #updateAccessors = (session?: SignedInSessionResource | null, options?: { dangerouslySkipEmit?: boolean }) => {
📜 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 (36)
packages/clerk-js/src/core/clerk.ts(9 hunks)packages/expo/src/hooks/useAuth.ts(1 hunks)packages/nextjs/src/app-router/client/ClerkProvider.tsx(2 hunks)packages/nextjs/src/app-router/server/ClerkProvider.tsx(2 hunks)packages/nextjs/src/app-router/server/keyless-provider.tsx(2 hunks)packages/nextjs/src/client-boundary/PromisifiedAuthProvider.tsx(0 hunks)packages/nextjs/src/client-boundary/hooks.ts(1 hunks)packages/nextjs/src/utils/mergeNextClerkPropsWithEnv.ts(2 hunks)packages/react/src/contexts/AuthContext.ts(0 hunks)packages/react/src/contexts/AuthContext.tsx(1 hunks)packages/react/src/contexts/ClerkContextProvider.tsx(0 hunks)packages/react/src/contexts/ClerkProvider.tsx(2 hunks)packages/react/src/contexts/IsomorphicClerkContext.tsx(1 hunks)packages/react/src/contexts/OrganizationContext.tsx(1 hunks)packages/react/src/contexts/SessionContext.tsx(1 hunks)packages/react/src/contexts/UserContext.tsx(1 hunks)packages/react/src/hooks/__tests__/useAuth.test.tsx(2 hunks)packages/react/src/hooks/__tests__/useAuth.type.test.ts(0 hunks)packages/react/src/hooks/useAuth.ts(5 hunks)packages/react/src/isomorphicClerk.ts(5 hunks)packages/shared/src/deriveState.ts(2 hunks)packages/shared/src/react/ClerkContextProvider.tsx(1 hunks)packages/shared/src/react/__tests__/payment-element.test.tsx(4 hunks)packages/shared/src/react/contexts.tsx(2 hunks)packages/shared/src/react/hooks/base/useClientBase.ts(1 hunks)packages/shared/src/react/hooks/base/useOrganizationBase.ts(1 hunks)packages/shared/src/react/hooks/base/useSessionBase.ts(1 hunks)packages/shared/src/react/hooks/base/useUserBase.ts(1 hunks)packages/shared/src/react/index.ts(1 hunks)packages/shared/src/react/utils.ts(1 hunks)packages/shared/src/types/clerk.ts(3 hunks)packages/ui/src/contexts/CoreClerkContextWrapper.tsx(0 hunks)packages/ui/src/contexts/CoreClientContext.tsx(1 hunks)packages/ui/src/contexts/index.ts(1 hunks)packages/ui/src/contexts/utils.ts(1 hunks)packages/ui/src/lazyModules/providers.tsx(2 hunks)
💤 Files with no reviewable changes (5)
- packages/ui/src/contexts/CoreClerkContextWrapper.tsx
- packages/react/src/contexts/AuthContext.ts
- packages/react/src/hooks/tests/useAuth.type.test.ts
- packages/react/src/contexts/ClerkContextProvider.tsx
- packages/nextjs/src/client-boundary/PromisifiedAuthProvider.tsx
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{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/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/clerk-js/src/core/clerk.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/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/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/shared/src/react/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/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)
Files:
packages/shared/src/react/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/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/shared/src/react/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/clerk-js/src/core/clerk.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/shared/src/react/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/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/shared/src/react/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/shared/src/react/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/clerk-js/src/core/clerk.ts
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/shared/src/react/hooks/base/useOrganizationBase.tspackages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/shared/src/react/hooks/base/useClientBase.tspackages/react/src/contexts/UserContext.tsxpackages/ui/src/contexts/index.tspackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/shared/src/types/clerk.tspackages/shared/src/react/index.tspackages/ui/src/contexts/utils.tspackages/shared/src/react/hooks/base/useUserBase.tspackages/shared/src/react/hooks/base/useSessionBase.tspackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/nextjs/src/client-boundary/hooks.tspackages/shared/src/deriveState.tspackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/utils.tspackages/shared/src/react/__tests__/payment-element.test.tsxpackages/expo/src/hooks/useAuth.tspackages/nextjs/src/utils/mergeNextClerkPropsWithEnv.tspackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/react/src/isomorphicClerk.tspackages/shared/src/react/contexts.tsxpackages/react/src/hooks/useAuth.tspackages/clerk-js/src/core/clerk.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/shared/src/react/__tests__/payment-element.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/react/src/contexts/UserContext.tsxpackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/__tests__/payment-element.test.tsxpackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/shared/src/react/contexts.tsx
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/shared/src/react/__tests__/payment-element.test.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/react/src/contexts/UserContext.tsxpackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/__tests__/payment-element.test.tsxpackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/shared/src/react/contexts.tsx
**/*.test.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use React Testing Library for component testing
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/shared/src/react/__tests__/payment-element.test.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/ui/src/contexts/CoreClientContext.tsxpackages/react/src/contexts/UserContext.tsxpackages/react/src/contexts/OrganizationContext.tsxpackages/react/src/contexts/AuthContext.tsxpackages/nextjs/src/app-router/client/ClerkProvider.tsxpackages/react/src/contexts/IsomorphicClerkContext.tsxpackages/react/src/contexts/ClerkProvider.tsxpackages/nextjs/src/app-router/server/ClerkProvider.tsxpackages/shared/src/react/ClerkContextProvider.tsxpackages/react/src/contexts/SessionContext.tsxpackages/ui/src/lazyModules/providers.tsxpackages/shared/src/react/__tests__/payment-element.test.tsxpackages/nextjs/src/app-router/server/keyless-provider.tsxpackages/shared/src/react/contexts.tsx
**/*.{test,spec}.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{test,spec}.{jsx,tsx}: Use React Testing Library for unit testing React components
Test component behavior, not implementation details
Use proper test queries in React Testing Library tests
Implement proper test isolation in React component tests
Use proper test coverage in React component tests
Test component interactions in integration tests
Use proper test data in React component tests
Implement proper test setup in React component tests
Use proper test cleanup in React component tests
Implement proper test assertions in React component tests
Use proper test structure for React component tests
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/shared/src/react/__tests__/payment-element.test.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use React Testing Library for component testing
Files:
packages/react/src/hooks/__tests__/useAuth.test.tsxpackages/shared/src/react/__tests__/payment-element.test.tsx
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/ui/src/contexts/index.tspackages/shared/src/react/index.ts
🧬 Code graph analysis (12)
packages/react/src/contexts/AuthContext.tsx (5)
packages/shared/src/types/jwtv2.ts (3)
SessionStatusClaim(200-200)JwtPayload(185-185)ActClaim(192-195)packages/shared/src/types/organizationMembership.ts (2)
OrganizationCustomRoleKey(78-82)OrganizationCustomPermissionKey(65-69)packages/shared/src/react/index.ts (2)
useClerkInstanceContext(9-9)useInitialStateContext(17-17)packages/shared/src/deriveState.ts (3)
deriveFromSsrInitialState(31-61)deriveFromClientSideState(63-98)DeriveStateReturnType(14-14)packages/shared/src/types/clerk.ts (1)
Resources(1210-1215)
packages/nextjs/src/app-router/client/ClerkProvider.tsx (2)
packages/shared/src/react/contexts.tsx (1)
InitialStateProvider(26-40)packages/shared/src/react/index.ts (1)
InitialStateProvider(16-16)
packages/shared/src/react/hooks/base/useUserBase.ts (4)
packages/shared/src/react/contexts.tsx (3)
useUserBase(18-18)useClerkInstanceContext(126-126)useInitialStateContext(42-50)packages/shared/src/types/user.ts (1)
UserResource(63-135)packages/shared/src/react/index.ts (2)
useClerkInstanceContext(9-9)useInitialStateContext(17-17)packages/react/src/isomorphicClerk.ts (1)
user(734-740)
packages/shared/src/react/hooks/base/useSessionBase.ts (4)
packages/shared/src/react/contexts.tsx (3)
useSessionBase(20-20)useClerkInstanceContext(126-126)useInitialStateContext(42-50)packages/shared/src/types/session.ts (1)
SignedInSessionResource(287-287)packages/shared/src/react/index.ts (2)
useClerkInstanceContext(9-9)useInitialStateContext(17-17)packages/react/src/isomorphicClerk.ts (1)
session(726-732)
packages/react/src/contexts/ClerkProvider.tsx (4)
packages/react/src/types.ts (2)
ClerkProviderProps(39-61)IsomorphicClerkOptions(17-17)packages/shared/src/types/clerk.ts (1)
IsomorphicClerkOptions(2330-2362)packages/shared/src/react/ClerkContextProvider.tsx (1)
ClerkContextProvider(20-47)packages/react/src/isomorphicClerk.ts (1)
IsomorphicClerk(126-1545)
packages/shared/src/deriveState.ts (1)
packages/shared/src/types/clerk.ts (1)
Resources(1210-1215)
packages/shared/src/react/__tests__/payment-element.test.tsx (1)
packages/shared/src/react/index.ts (1)
OptionsContext(7-7)
packages/expo/src/hooks/useAuth.ts (1)
packages/shared/src/types/hooks.ts (1)
UseAuthReturn(26-118)
packages/nextjs/src/utils/mergeNextClerkPropsWithEnv.ts (1)
packages/nextjs/src/types.ts (1)
NextClerkProviderProps(5-26)
packages/react/src/isomorphicClerk.ts (1)
packages/shared/src/types/clerk.ts (4)
ListenerOptions(138-138)UnsubscribeCallback(139-139)Resources(1210-1215)ListenerCallback(137-137)
packages/shared/src/react/contexts.tsx (3)
packages/shared/src/types/clerk.ts (1)
LoadedClerk(2364-2366)packages/shared/src/types/organization.ts (1)
OrganizationResource(38-69)packages/shared/src/react/hooks/base/useOrganizationBase.ts (1)
useOrganizationBase(7-25)
packages/clerk-js/src/core/clerk.ts (5)
packages/shared/src/types/clerk.ts (4)
ListenerCallback(137-137)ListenerOptions(138-138)UnsubscribeCallback(139-139)Resources(1210-1215)packages/clerk-js/src/utils/memoizeStateListenerCallback.ts (1)
memoizeListenerCallback(139-146)packages/react/src/isomorphicClerk.ts (4)
session(726-732)organization(742-748)user(734-740)client(717-724)packages/shared/src/types/session.ts (1)
SignedInSessionResource(287-287)packages/react/src/stateProxy.ts (1)
client(325-331)
🔇 Additional comments (35)
packages/ui/src/contexts/CoreClientContext.tsx (1)
1-1: Import adjustment to dropClientContextlooks correctThe import list now matches usage in this file (
assertContextExistsanduseClientContextonly). No further changes needed here.packages/react/src/contexts/SessionContext.tsx (1)
1-1: This change aligns with the hook-based API and removes an unused export.The
SessionContextexport has been removed, leaving onlyuseSessionContext. This is appropriate because no code in the codebase importsSessionContextdirectly—all usages are already using theuseSessionContexthook pattern. The export from@clerk/shared/reactis properly configured and stable.packages/shared/src/react/__tests__/payment-element.test.tsx (2)
6-6: Context import matches new provider wiringImporting
ClerkInstanceContextandOptionsContextfrom the shared../contextsbarrel keeps the test aligned with runtime wiring; no issues here.
198-202: Provider tree correctly supplies Clerk + options to PaymentElementWiring
ClerkInstanceContext.ProviderwithOptionsContext.Provideraround__experimental_PaymentElementProvidermirrors the production composition and ensures locale is driven from the same options surface used at runtime. Looks good.packages/nextjs/src/utils/mergeNextClerkPropsWithEnv.ts (1)
1-1: Type-onlyUiimport is correct and tree-shaking friendlyUsing
import type { Ui }keeps this dependency type‑only at runtime and aligns with the tree‑shaking guidance. No changes needed here.packages/expo/src/hooks/useAuth.ts (1)
11-12: LGTM! Clean signature update.The updated signature correctly forwards the options object to the base hook, maintaining type consistency via
Parameters<typeof useAuthBase>[0]. This aligns with the broader migration to an options-based API pattern.packages/nextjs/src/client-boundary/hooks.ts (1)
3-15: LGTM! Simplified re-exports.The direct re-export of
useAuthfrom@clerk/reactis cleaner and aligns with the removal ofPromisifiedAuthProvider. Since the PR is marked as a breaking change, ensure the migration path fromusePromisifiedAuth(if it was previously exposed) is documented.packages/shared/src/react/hooks/base/useSessionBase.ts (1)
12-21: Well-implemented useSyncExternalStore pattern.The subscription correctly uses
skipInitialEmit: trueto avoid redundant updates, and the snapshot logic properly falls back to initial state when Clerk isn't loaded. The memoization withuseCallbackensures stable references for the store.packages/nextjs/src/app-router/server/ClerkProvider.tsx (2)
34-41: Clean simplification of async state handling.The inline conditional computation and direct await pattern is more readable than the previous approach with separate generator functions. The async/await usage is correct for this server component.
54-65: LGTM! Cleaner provider rendering logic.The direct return pattern eliminates the intermediate
outputvariable and makes the control flow immediately clear. The removal ofPromisifiedAuthProviderwrapping aligns with the architectural shift touseSyncExternalStore.packages/shared/src/deriveState.ts (1)
12-14: Good type inference pattern.Using
ReturnType<typeof deriveFromSsrInitialState>ensures the derived state type stays synchronized with the implementation automatically. The comment on lines 12-13 clearly documents this design choice.packages/nextjs/src/app-router/server/keyless-provider.tsx (2)
15-17: LGTM: Improved type precision.The parameter type now correctly excludes both
'__internal_invokeMiddlewareOnAuthStateChange'and'children', which aligns with the function's usage since it doesn't need the children prop.
34-76: ClientClerkProvider is correctly compatible with optional nonce and initialState props.The removal of
generateNonceandgenerateStatePromisefrom KeylessProvider is architecturally sound. The server-levelClerkProvider(inapp-router/server/ClerkProvider.tsx) extractsnoncefrom request headers and computesinitialStatefrom dynamic auth data, then passes both toClientClerkProviderbefore determining keyless status.ClientClerkProviderhandlesinitialStateas an optional prop and wraps it inInitialStateProvideronly when present (line 116-118 ofapp-router/client/ClerkProvider.tsx).KeylessProvidercorrectly does not regenerate these values—they flow through from the parentClerkProvidercontext.packages/react/src/contexts/UserContext.tsx (1)
1-1: LGTM: Context export removed in favor of hook-based access.This change removes the
UserContextexport, leaving onlyuseUserContextavailable. This is part of the broader architectural shift toward hook-based state access and away from exposing context objects directly.Note: This is a breaking change for consumers directly importing
UserContext.packages/react/src/contexts/OrganizationContext.tsx (1)
1-1: LGTM: Provider export removed, hook remains.This change removes the
OrganizationProviderexport while retaininguseOrganizationContext. This aligns with the broader refactoring to consolidate providers and expose hook-based access patterns.Note: This is a breaking change for consumers directly importing
OrganizationProvider.packages/ui/src/lazyModules/providers.tsx (2)
13-13: LGTM: Updated to use ClerkContextProvider from shared/react.The lazy import correctly references ClerkContextProvider from the contexts module, which now re-exports it from @clerk/shared/react.
52-56: LGTM: Component replacement maintains same props.The replacement of
CoreClerkContextWrapperwithClerkContextProvidermaintains the same prop structure (clerk={props.clerk}and children), ensuring compatibility with the rest of the provider tree.packages/shared/src/types/clerk.ts (2)
138-138: LGTM: ListenerOptions type correctly defined.The new
ListenerOptionstype with optionalskipInitialEmitflag is simple and well-typed.
249-254: LGTM: Internal property properly documented.The
__internal_lastEmittedResourcesproperty is correctly marked as internal and includes JSDoc documentation explaining its purpose.packages/ui/src/contexts/index.ts (1)
9-9: LGTM: ClerkContextProvider now re-exported from shared/react.This change removes
CoreClerkContextWrapperand addsClerkContextProviderre-exported from @clerk/shared/react, consolidating the context provider implementation in the shared package.Note: This is a breaking change for consumers directly importing
CoreClerkContextWrapperfrom the UI package.packages/react/src/hooks/__tests__/useAuth.test.tsx (1)
1-10: LGTM: Imports updated to reflect new API surface.The test correctly updates imports to use
InitialStateProviderfrom @clerk/shared/react instead of the removedAuthContext.packages/nextjs/src/app-router/client/ClerkProvider.tsx (1)
113-121: Nested-provider guard looks good; confirmuseClerkNextOptionsis safe without a providerThe
isNestedcheck plusInitialStateProviderwrapper for the dynamic-in-a-parent case is a clean way to avoid doubleClerkProviders while still exposing initial state to the subtree.The only thing to double‑check is that
useClerkNextOptions()behaves safely when there is no surroundingClerkNextOptionsProvider(i.e. returnsundefined/ a noop result rather than throwing), sinceClientClerkProvideris also used as the top‑level provider.packages/ui/src/contexts/utils.ts (1)
1-10: Context assertion wiring via shared error helper looks correctUsing
clerkCoreErrorContextProviderNotFoundinassertContextExistskeeps context‑missing errors consistent across packages and matches the intended semantics (throw when provider is absent).packages/shared/src/react/index.ts (1)
16-21: New exports fromshared/reactindex align with the refactorRe‑exporting
InitialStateProvider,useInitialStateContext, andClerkContextProviderhere matches the new shared React API surface and makes the initial‑state and context composition primitives available to downstream packages without adding new logic.packages/react/src/hooks/useAuth.ts (1)
13-13: LGTM! Clean migration from context to useSyncExternalStore-backed state.The refactoring from
useAuthContexttouseAuthStateproperly aligns with the useSyncExternalStore migration. The simplifiedUseAuthOptionstype (removingRecord<string, any>) improves type safety.Also applies to: 95-99, 109-109
packages/react/src/contexts/AuthContext.tsx (2)
41-68: Well-structured useSyncExternalStore integration.The hook correctly implements the useSyncExternalStore pattern:
skipInitialEmit: trueprevents double-rendering on mount- Proper fallback to
initialStateduring SSR/hydration- Memoized derivation avoids unnecessary recalculations
85-87: The type guard correctly distinguishes between the two types.Resourcesinterface requires aclient: ClientResourceproperty (line 1210 in packages/shared/src/types/clerk.ts), whileInitialStatetype does not include aclientproperty (defined in packages/shared/src/types/ssr.ts lines 34-47). The guard logic using!('client' in state)is sound and effectively narrows the union type.packages/react/src/isomorphicClerk.ts (2)
160-169: Good refactoring of premount listener tracking.The updated data shape properly tracks both the options and handler functions, enabling correct cleanup regardless of whether ClerkJS has loaded.
810-812: LGTM! Clean proxy getter for last emitted resources.The getter correctly delegates to the underlying ClerkJS instance and returns
undefinedwhen ClerkJS is not available.packages/clerk-js/src/core/clerk.ts (3)
1584-1601: LGTM! Proper implementation of skipInitialEmit option.The
addListenermethod correctly respects theskipInitialEmitoption, only emitting the initial state when the option is not set or false. This prevents unnecessary double-renders when usinguseSyncExternalStore.
2931-2944: Good centralization of emission logic.The
#emitmethod now constructs a unifiedResourcesobject and caches it in__internal_lastEmittedResources. This provides a consistent snapshot foruseSyncExternalStore's getSnapshot.
2487-2512: Verify the emit order is correct for updateClient.The code sets accessors with
dangerouslySkipEmit: trueat line 2512, then emitsTokenUpdateat line 2522, and finally calls#emit()at line 2525. This order ensures the token update is processed before the state change, which seems intentional. Confirm this ordering is required for correct behavior.packages/shared/src/react/contexts.tsx (3)
26-40: Well-designed InitialStateProvider with proper immutability guarantees.The
useStatepattern to capture the initial snapshot is a clever way to enforce the "initialState cannot change" invariant. The comment explains the rationale clearly, and theeslint-disableis appropriately justified.
91-94: LGTM! Clean refactor of useOrganizationContext.The hook now delegates to
useOrganizationBaseand wraps the result in a memoized object for referential stability.
18-20: No breaking change found for the stated context exports.The review comment claims that
ClientContext,SessionContext, andUserContextwere "no longer exported directly," but these exports never existed in the public API. A search of the entire codebase reveals no usage or imports of these identifiers anywhere. The current exports inpackages/shared/src/react/index.tsshow:
- Hook-based exports:
useUserContext,useClientContext,useSessionContext(lines 18-20)- Other direct context exports:
ClerkInstanceContext,OptionsContext(which remain available)The lines in question simply export hook aliases from internal implementation files—this is the intended API design, not a replacement of previously exported contexts. Git blame shows these lines were newly added in commit e3b3d7e; no migration documentation or evidence of actual breaking changes exists.
Likely an incorrect or invalid review comment.
| export function useSessionBase(): SignedInSessionResource | null | undefined { | ||
| const clerk = useClerkInstanceContext(); | ||
| const initialState = useInitialStateContext(); | ||
| const getInitialState = useCallback(() => initialState?.session as SignedInSessionResource, [initialState?.session]); |
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
🏁 Script executed:
#!/bin/bash
# Find and examine the InitialState and SignedInSessionResource types
rg -n "interface InitialState|type InitialState" packages/shared/src --type=ts -A 10Repository: clerk/javascript
Length of output: 841
🏁 Script executed:
#!/bin/bash
# Search for SignedInSessionResource definition
rg -n "SignedInSessionResource" packages/shared/src --type=ts -B 2 -A 5 | head -100Repository: clerk/javascript
Length of output: 7053
🏁 Script executed:
#!/bin/bash
# Examine the useSessionBase.ts file around line 10
cat -n packages/shared/src/react/hooks/base/useSessionBase.tsRepository: clerk/javascript
Length of output: 1055
🏁 Script executed:
#!/bin/bash
# Check SessionResource definition and its relationship to SignedInSessionResource
rg -n "export type SessionResource|export interface SessionResource" packages/shared/src/types --type=ts -A 15 | head -80Repository: clerk/javascript
Length of output: 1248
🏁 Script executed:
#!/bin/bash
# Check if there's documentation or comments about why this cast is necessary
rg -B 5 "as SignedInSessionResource" packages/shared/src --type=ts | head -60Repository: clerk/javascript
Length of output: 1279
Unsafe type cast - add type guard or validate at source.
InitialState.session is typed as SessionResource | undefined (from packages/shared/src/types/ssr.ts), but the code casts it to SignedInSessionResource without validation. SignedInSessionResource is a union of ActiveSessionResource | PendingSessionResource—both subtypes of SessionResource. While the cast may be intentional (assuming session is always signed-in when present), it bypasses TypeScript's type checking and violates the guideline to avoid unsafe type assertions.
Either:
- Add a type guard to validate the session status before the cast, or
- Type
InitialState.sessionasSignedInSessionResource | undefinedat the source if the context guarantees it's always signed-in when present
🤖 Prompt for AI Agents
In packages/shared/src/react/hooks/base/useSessionBase.ts around line 10, the
code unsafely casts initialState?.session to SignedInSessionResource; replace
this with a safe narrowing: either (A) add and use a type guard (e.g.,
isSignedInSession(session) checking session !== undefined and
session.type/status matches ActiveSessionResource | PendingSessionResource) and
return only when the guard passes, or (B) if the source truly guarantees only
signed-in sessions, change the InitialState.session type in
packages/shared/src/types/ssr.ts to SignedInSessionResource | undefined so the
cast is unnecessary; implement one of these fixes and adjust the useCallback to
rely on the narrowed/updated type.
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/react/src/contexts/AuthContext.tsx (3)
15-39: Tighten typing and immutability fordefaultDerivedInitialStateThe
AuthStateValueshape anddefaultDerivedInitialStatekeys/values line up, but you can make this default safer and future‑proof by locking its shape and intent:
- Ensure it always stays in sync with
AuthStateValue.- Communicate that it’s an immutable fallback and not meant to be mutated by consumers.
A small tweak using
satisfies(andas const) would do this cleanly:-export const defaultDerivedInitialState = { - actor: undefined, - factorVerificationAge: null, - orgId: undefined, - orgPermissions: undefined, - orgRole: undefined, - orgSlug: undefined, - sessionClaims: undefined, - sessionId: undefined, - sessionStatus: undefined, - userId: undefined, -}; +export const defaultDerivedInitialState = { + actor: undefined, + factorVerificationAge: null, + orgId: undefined, + orgPermissions: undefined, + orgRole: undefined, + orgSlug: undefined, + sessionClaims: undefined, + sessionId: undefined, + sessionStatus: undefined, + userId: undefined, +} as const satisfies AuthStateValue;This enforces the contract at compile time while preserving a readonly default object.
41-67:useAuthStateSSR/client behavior and subscription look solid; consider documenting as a public hookThe
useSyncExternalStorewiring looks correct:
subscribedelegates toclerk.addListenerwithskipInitialEmit: true.getSnapshotreturns SSR initial state untilclerk.loadedand__internal_lastEmittedResourcesare available, avoiding hydration mismatches.getServerSnapshotuses the samegetInitialState, aligning server and client initial renders.- The memoized
authStatenormalization viaderiveFromSsrInitialState/deriveFromClientSideStateis clean, and the!stateguard safely falls back todefaultDerivedInitialState.Two small follow‑ups:
Public API docs – If
useAuthStateis (or will become) part of the public React surface (re‑exported from the package entry), it deserves a short JSDoc explaining:
- That it reconciles SSR initial state and live client state.
- What fields are guaranteed on the returned
AuthStateValue.
This matches the package’s guideline that public APIs be documented with JSDoc. As per coding guidelines, public APIs should have JSDoc.Double‑check
addListenercontract –useSyncExternalStoreexpectssubscribeto return an unsubscribe function. This is very likely already true forclerk.addListener, but it’s worth confirming its signature stayslistener => () => voidso the hook remains safe across refactors.No structural changes to the hook itself are necessary.
69-82: Add an explicit return type forauthStateFromFullThe mapping from
DeriveStateReturnTypeto the publicAuthStateValueslice is straightforward and correct. To keep this helper tightly coupled to the public shape (and catch any future drift between the two), it’s useful to give it an explicit return type:-function authStateFromFull(derivedState: DeriveStateReturnType) { +function authStateFromFull(derivedState: DeriveStateReturnType): AuthStateValue { return { sessionId: derivedState.sessionId, sessionStatus: derivedState.sessionStatus, sessionClaims: derivedState.sessionClaims, userId: derivedState.userId, actor: derivedState.actor, orgId: derivedState.orgId, orgRole: derivedState.orgRole, orgSlug: derivedState.orgSlug, orgPermissions: derivedState.orgPermissions, factorVerificationAge: derivedState.factorVerificationAge, }; }This aligns with the guideline to use explicit return types for functions and helps ensure any added/removed fields are surfaced at compile time. As per coding guidelines, explicit return types are preferred.
📜 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/react/src/contexts/AuthContext.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/react/src/contexts/AuthContext.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/react/src/contexts/AuthContext.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/react/src/contexts/AuthContext.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/react/src/contexts/AuthContext.tsx
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/react/src/contexts/AuthContext.tsx
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/react/src/contexts/AuthContext.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
packages/react/src/contexts/AuthContext.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
packages/react/src/contexts/AuthContext.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
packages/react/src/contexts/AuthContext.tsx
**/*.{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/react/src/contexts/AuthContext.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/react/src/contexts/AuthContext.tsx
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/react/src/contexts/AuthContext.tsx
🧬 Code graph analysis (1)
packages/react/src/contexts/AuthContext.tsx (5)
packages/shared/src/types/jwtv2.ts (3)
SessionStatusClaim(200-200)JwtPayload(185-185)ActClaim(192-195)packages/shared/src/types/organizationMembership.ts (2)
OrganizationCustomRoleKey(78-82)OrganizationCustomPermissionKey(65-69)packages/shared/src/react/contexts.tsx (2)
useClerkInstanceContext(126-126)useInitialStateContext(42-50)packages/shared/src/react/index.ts (2)
useClerkInstanceContext(9-9)useInitialStateContext(17-17)packages/shared/src/deriveState.ts (3)
deriveFromSsrInitialState(31-61)deriveFromClientSideState(63-98)DeriveStateReturnType(14-14)
⏰ 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). (6)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/react/src/contexts/AuthContext.tsx (2)
1-13: Imports and shared hooks usage look consistentType-only imports from
@clerk/shared/deriveStateand@clerk/shared/typesplus value imports and React hooks are all idiomatic and tree‑shaking friendly. No changes requested here.
84-86:isInitialStatetype guard is reasonable; verifyResourcesalways carriesclientUsing the absence of a
clientproperty to distinguishInitialStatefromResources:function isInitialState(state: Resources | InitialState): state is InitialState { return !('client' in state); }is a simple and efficient discriminator, and it matches how
Resourcesis typically shaped in the shared layer.Just ensure that:
Resourcesalways includes aclientfield (even if optional), andInitialStatenever has aclientfield,so this remains a sound and future‑proof narrowing strategy.
No code changes required; this is more of an invariant to keep in mind as
Resources/InitialStateevolve.
…s-from-provider-and-rely-on
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
integration/testUtils/usersService.ts (1)
206-220: Consider using ES6 property shorthand notation.Line 210 uses
name: name,which can be simplified to justname,using ES6 shorthand property notation when the property name and variable name are identical.Apply this diff to simplify the syntax:
const organization = await withErrorLogging('createOrganization', () => clerkClient.organizations.createOrganization({ - name: name, + name, createdBy: userId, }), );integration/templates/next-app-router/src/app/transitions/page.tsx (1)
7-23: Promise cache never clears - acceptable for test code only.The
cachedPromisesMap grows unbounded since entries are never removed. This is fine for a test fixture but shouldn't be used in production code.The composite key
${key}-${value}-${delay}is a reasonable approach to cache unique promise instances per combination.For better type safety, consider a typed wrapper instead of
as any:type SuspensePromise<T> = Promise<T> & { status?: 'fulfilled'; value?: T };However, this is test code, so current implementation is acceptable.
📜 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 (3)
integration/templates/next-app-router/src/app/transitions/page.tsx(1 hunks)integration/testUtils/usersService.ts(1 hunks)integration/tests/transitions.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
integration/testUtils/usersService.tsintegration/tests/transitions.test.tsintegration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/testUtils/usersService.tsintegration/tests/transitions.test.tsintegration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
integration/testUtils/usersService.tsintegration/tests/transitions.test.tsintegration/templates/next-app-router/src/app/transitions/page.tsx
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
integration/testUtils/usersService.tsintegration/tests/transitions.test.tsintegration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{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:
integration/testUtils/usersService.tsintegration/tests/transitions.test.tsintegration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
integration/testUtils/usersService.tsintegration/tests/transitions.test.tsintegration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
integration/testUtils/usersService.tsintegration/tests/transitions.test.tsintegration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
integration/tests/transitions.test.ts
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
integration/tests/transitions.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use React Testing Library for component testing
Files:
integration/tests/transitions.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
integration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
integration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
integration/templates/next-app-router/src/app/transitions/page.tsx
🧬 Code graph analysis (2)
integration/tests/transitions.test.ts (4)
integration/testUtils/index.ts (1)
testAgainstRunningApps(88-88)integration/presets/index.ts (1)
appConfigs(14-30)integration/testUtils/usersService.ts (2)
FakeUser(57-65)FakeOrganization(69-73)packages/shared/src/keys.ts (1)
parsePublishableKey(87-136)
integration/templates/next-app-router/src/app/transitions/page.tsx (2)
packages/shared/src/types/organizationMembership.ts (1)
OrganizationMembershipResource(47-63)packages/shared/src/types/clerk.ts (1)
SetActive(1351-1351)
⏰ 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). (28)
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (**)
- GitHub Check: Unit Tests (shared, clerk-js, RQ)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
integration/tests/transitions.test.ts (6)
1-8: LGTM - Clean imports.Imports are well-organized with external dependencies first, followed by local utilities. Type imports are properly used for
FakeUserandFakeOrganization.
9-24: Good documentation of test scope and limitations.The comment block clearly explains the purpose of these tests, acknowledges they document current behavior rather than desired behavior, and notes potential limitations around cookie handling in transitions. This is helpful context for future maintainers.
56-61: LGTM - Proper cleanup in afterAll.Resources are cleaned up in correct order: organizations first, then user, then app teardown.
70-100: LGTM - First test covers the baseline behavior.The test properly validates:
- Initial undefined state during page load
- Organization ID appearing after hydration
- Suspense fallback showing during fetch
- Fetch result matching the organization ID
- Organization switch triggering re-suspend and new fetch
Good use of test IDs and exact text matching for organization names.
109-150: LGTM - Second test validates transition interruption behavior.This test correctly verifies that auth state changes interrupt an in-progress unrelated transition. The flow is well-structured: start transition → switch org → verify immediate update → finish original transition.
158-189: LGTM - Third test covers setActive within a transition.The test properly validates behavior when
setActiveis triggered inside a transition, including the "Switching..." pending state UI.integration/templates/next-app-router/src/app/transitions/page.tsx (7)
1-6: LGTM - Appropriate imports for the test page.The
'use client'directive is correct for this interactive component. Type imports from@clerk/shared/typesfollow the coding guidelines.
25-53: LGTM - Well-structured page layout.The component composition is clean:
TransitionControllerfor manual transition control,TransitionSwitcherfor membership buttons,OrganizationSwitcherfor standard Clerk UI, andFetcherwrapped inSuspense. The test IDs (fetcher-fallback) align with the integration test expectations.
55-87: Acknowledged hack for controllable transitions.The pattern of attaching
resolveto the promise object ((promise as any).resolve = resolve) is an intentional hack as documented. The@ts-expect-errorfor async transitions is appropriate since this targets React 19 behavior testing.
89-107: LGTM - TransitionSwitcher properly guards loading state.The early return for
!isLoaded || !userMemberships.datacorrectly handles loading and empty states before rendering membership buttons.
109-132: LGTM - TransitionSwitcherButton demonstrates current setActive behavior.The comment on lines 123-124 clearly documents that
setActivedoesn't currently support transitions and the test verifies existing behavior. The@ts-expect-erroris appropriate.
134-151: LGTM - AuthStatePresenter with proper test IDs.The test IDs (
session-id,user-id,org-id) match the integration test assertions. UsingString()handles undefined/null values gracefully.
153-171: Fetcher uses valid Suspense throw pattern.The pattern of checking
promise.status !== 'fulfilled'and throwing the promise is a standard Suspense data-fetching approach. The early return for!orgIdprevents unnecessary cache entries.Note: In production code, consider using React's
use()hook (React 19+) or a data-fetching library with built-in Suspense support instead of manual promise throwing.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/shared/src/react/hooks/base/useSessionBase.ts (1)
10-12: Unsafe type cast persists - add type guard or fix source type.This issue was previously flagged:
initialState?.sessionis typed asSessionResource | undefinedbut is cast toSignedInSessionResourcewithout validation. This bypasses TypeScript's type checking and violates the guideline to avoid unsafe type assertions.Solutions:
Option A (Recommended): Add a type guard
+function isSignedInSession(session: SessionResource | undefined): session is SignedInSessionResource { + return session !== undefined && (session.status === 'active' || session.status === 'pending'); +} + export function useSessionBase(): SignedInSessionResource | null | undefined { const clerk = useClerkInstanceContext(); const initialState = useInitialStateContext(); const getInitialState = useCallback(() => { - return initialState?.session as SignedInSessionResource | undefined; + const session = initialState?.session; + return isSignedInSession(session) ? session : undefined; }, [initialState?.session]);Option B: Update source type (if the context truly guarantees only signed-in sessions)
Change
InitialState.sessiontype inpackages/shared/src/types/ssr.tstoSignedInSessionResource | undefined.
🧹 Nitpick comments (4)
packages/shared/src/react/hooks/base/useSessionBase.ts (1)
10-21: Consider adding explicit return types to callbacks.Per coding guidelines, "Always define explicit return types for functions, especially public APIs." While TypeScript can infer these types, explicit annotations improve clarity and catch potential type errors earlier.
- const getInitialState = useCallback(() => { + const getInitialState = useCallback((): SignedInSessionResource | undefined => { return initialState?.session as SignedInSessionResource | undefined; }, [initialState?.session]); const session = useSyncExternalStore( - useCallback(callback => clerk.addListener(callback, { skipInitialEmit: true }), [clerk]), - useCallback(() => { + useCallback((callback: () => void): (() => void) => clerk.addListener(callback, { skipInitialEmit: true }), [clerk]), + useCallback((): SignedInSessionResource | null | undefined => { if (!clerk.loaded) { return getInitialState(); } return clerk.session; }, [clerk, getInitialState]), getInitialState, );integration/templates/next-app-router/src/app/transitions/page.tsx (3)
3-5: Prefer type-only imports for Clerk shared typesTo align with the guideline about type-only imports and keep the runtime bundle clean, consider importing
OrganizationMembershipResourceandSetActiveas types:-import { OrganizationSwitcher, useAuth, useOrganizationList } from '@clerk/nextjs'; -import { OrganizationMembershipResource, SetActive } from '@clerk/shared/types'; -import { Suspense, useState, useTransition } from 'react'; +import { OrganizationSwitcher, useAuth, useOrganizationList } from '@clerk/nextjs'; +import type { OrganizationMembershipResource, SetActive } from '@clerk/shared/types'; +import { Suspense, useState, useTransition } from 'react';
7-23: Strongly type the Suspense cache entries instead of usinganyThe current pattern relies on
(promise as any).statusand(promise as any).valuein bothgetCachedPromiseandFetcher. This works at runtime but bypasses type safety and conflicts with the “noany” guideline.You can introduce a small cache entry type and remove the
anyassertions:-// Quick and dirty promise cache to enable Suspense "fetching" -const cachedPromises = new Map<string, Promise<unknown>>(); -const getCachedPromise = (key: string, value: string | undefined | null, delay: number = 1000) => { +// Quick and dirty promise cache to enable Suspense "fetching" +type SuspenseCacheEntry = Promise<string> & { + status?: 'fulfilled'; + value?: string; +}; + +const cachedPromises = new Map<string, SuspenseCacheEntry>(); +const getCachedPromise = ( + key: string, + value: string | undefined | null, + delay: number = 1000, +): SuspenseCacheEntry => { if (cachedPromises.has(`${key}-${value}-${delay}`)) { - return cachedPromises.get(`${key}-${value}-${delay}`)!; + return cachedPromises.get(`${key}-${value}-${delay}`)!; } - const promise = new Promise(resolve => { + const promise = new Promise<string>(resolve => { setTimeout(() => { const returnValue = `Fetched value: ${value}`; - (promise as any).status = 'fulfilled'; - (promise as any).value = returnValue; + (promise as SuspenseCacheEntry).status = 'fulfilled'; + (promise as SuspenseCacheEntry).value = returnValue; resolve(returnValue); }, delay); - }); + }) as SuspenseCacheEntry; cachedPromises.set(`${key}-${value}-${delay}`, promise); return promise; };And in
Fetcher:- const promise = getCachedPromise('fetcher', orgId, 1000); - if (promise && (promise as any).status !== 'fulfilled') { + const promise = getCachedPromise('fetcher', orgId, 1000); + if (promise.status !== 'fulfilled') { throw promise; } return ( <div> <h1>Fetcher</h1> - <div data-testid='fetcher-result'>{(promise as any).value}</div> + <div data-testid='fetcher-result'>{promise.value}</div> </div> );This keeps the Suspense demo behavior but removes all
anyusage around the cache.Also applies to: 149-167
55-85: Type the controlled transition promise instead of usingany
TransitionControllercurrently stores a promise with an ad‑hoc.resolveproperty and calls(transitionPromise as any).resolve(). This is safe in practice here but loses type information and could be accidentally misused.You can introduce a small helper type and avoid
any:-function TransitionController() { - const [transitionPromise, setTransitionPromise] = useState<Promise<unknown> | null>(null); +type TransitionPromise = Promise<void> & { + resolve?: () => void; +}; + +function TransitionController() { + const [transitionPromise, setTransitionPromise] = useState<TransitionPromise | null>(null); const [pending, startTransition] = useTransition(); return ( <div> <button onClick={() => { - if (pending) { - (transitionPromise as any).resolve(); - setTransitionPromise(null); - } else { - let resolve; - const promise = new Promise(r => { - resolve = r; - }); - // We save the resolve on the promise itself so we can later resolve it manually - (promise as any).resolve = resolve; + if (pending && transitionPromise?.resolve) { + transitionPromise.resolve(); + setTransitionPromise(null); + } else { + let resolve!: () => void; + const promise = new Promise<void>(r => { + resolve = r; + }) as TransitionPromise; + // We save the resolve on the promise itself so we can later resolve it manually + promise.resolve = resolve; setTransitionPromise(promise); startTransition(async () => { await promise; });This keeps the “manual transition control” behavior but makes the contract explicit and removes
any.
📜 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 (3)
integration/templates/next-app-router/src/app/transitions/page.tsx(1 hunks)packages/shared/src/react/hooks/base/useClientBase.ts(1 hunks)packages/shared/src/react/hooks/base/useSessionBase.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/react/hooks/base/useClientBase.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
integration/templates/next-app-router/src/app/transitions/page.tsxpackages/shared/src/react/hooks/base/useSessionBase.ts
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/templates/next-app-router/src/app/transitions/page.tsxpackages/shared/src/react/hooks/base/useSessionBase.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
integration/templates/next-app-router/src/app/transitions/page.tsxpackages/shared/src/react/hooks/base/useSessionBase.ts
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
integration/templates/next-app-router/src/app/transitions/page.tsxpackages/shared/src/react/hooks/base/useSessionBase.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
integration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
integration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
integration/templates/next-app-router/src/app/transitions/page.tsx
**/*.{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:
integration/templates/next-app-router/src/app/transitions/page.tsxpackages/shared/src/react/hooks/base/useSessionBase.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
integration/templates/next-app-router/src/app/transitions/page.tsxpackages/shared/src/react/hooks/base/useSessionBase.ts
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
integration/templates/next-app-router/src/app/transitions/page.tsxpackages/shared/src/react/hooks/base/useSessionBase.ts
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/react/hooks/base/useSessionBase.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/hooks/base/useSessionBase.ts
🧬 Code graph analysis (1)
packages/shared/src/react/hooks/base/useSessionBase.ts (4)
packages/shared/src/react/contexts.tsx (3)
useSessionBase(20-20)useClerkInstanceContext(126-126)useInitialStateContext(42-50)packages/shared/src/types/session.ts (1)
SignedInSessionResource(287-287)packages/shared/src/react/index.ts (2)
useClerkInstanceContext(9-9)useInitialStateContext(17-17)packages/react/src/isomorphicClerk.ts (1)
session(726-732)
⏰ 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). (26)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (5)
integration/templates/next-app-router/src/app/transitions/page.tsx (5)
25-53: TransitionsPage component looks correct for the intended demoThe top-level page wiring (TransitionController, TransitionSwitcher, OrganizationSwitcher, AuthStatePresenter, Suspense + Fetcher) is straightforward and matches the transition/Suspense test scenario; no logic or rendering issues stand out.
If you want to tighten typing further in line with the guidelines, you could add an explicit return type for the page component (e.g.
(): React.JSX.Elementwith atypeimport fromreact), but that’s optional for this demo page.
87-105: Organization list loading and rendering flow looks goodThe
TransitionSwitcherhook usage and guards (!isLoaded || !userMemberships.data) before mapping memberships are sound and avoid rendering until data is ready. The component is small and focused on its single responsibility.
107-128: TransitionSwitcherButton correctly wrapssetActivein a transitionThe button’s
useTransitionusage, pending label (“Switching...”), and call tosetActive({ organization: membership.organization.id })look consistent with the intent to exercise current transition behavior. For this test/demo component it’s reasonable not to add additional error handling aroundsetActive, since failures would be surfaced via the integration tests.
130-147: AuthStatePresenter is simple and test-friendlyReading
orgId,sessionId, anduserIdfromuseAuthand rendering them withdata-testidhooks is straightforward and keeps this component focused on observability for tests. No issues from a logic or typing standpoint.
149-167: Fetcher Suspense usage is appropriate for this demoThe Fetcher correctly gates on
orgId, uses the cached promise to drive Suspense (throwing while unresolved, rendering value afterwards), and exposes a deterministicdata-testidfor tests. Aside from the earlier note about typing the cache to removeany, the control flow is solid.
| export { useUserBase as useUserContext } from './hooks/base/useUserBase'; | ||
| export { useClientBase as useClientContext } from './hooks/base/useClientBase'; | ||
| export { useSessionBase as useSessionContext } from './hooks/base/useSessionBase'; |
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.
The new base hooks return the same thing as the contexts hook did, so I reexported to keep changes minimal and this part non-breaking for a first pass.
Do we want to instead remove the useContext hooks? If so, we should also decide if we keep these new hooks internal or export them.
Also, what do we think about naming, useUserBase etc? Not sure what I think, so I just went with something.
| ClientContext, | ||
| OptionsContext, | ||
| OrganizationProvider, | ||
| SessionContext, | ||
| useAssertWrappedByClerkProvider, | ||
| useCheckoutContext, | ||
| useClerkInstanceContext, | ||
| useClientContext, | ||
| useOptionsContext, | ||
| useOrganizationContext, | ||
| UserContext, |
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.
The contexts being removed is a breaking change, but we don't think anyone is using them, and if they do it's kind of unavoidable.
| 'InitialStateContext', | ||
| ); | ||
|
|
||
| export function InitialStateProvider({ |
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.
I just noticed I had forgotten to write a JSDoc comment for this. This is a new public API community SDK packages could use.
| * ["Initialization", "Signed out", "Signed in (no active organization)", "Signed in (with active organization)"] | ||
| * | ||
| * @param [initialAuthStateOrOptions] - An object containing the initial authentication state or options for the `useAuth()` hook. If not provided, the hook will attempt to derive the state from the context. `treatPendingAsSignedOut` is a boolean that indicates whether pending sessions are considered as signed out or not. Defaults to `true`. | ||
| * @param [options] - An object containing options for the `useAuth()` hook. `treatPendingAsSignedOut` is a boolean that indicates whether pending sessions are considered as signed out or not. Defaults to `true`. |
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.
Removing initialAuthState here is technically a breaking change, but we don't think anyone is, or should be using this as it was added for internal use.
| if (authContext.sessionId === undefined && authContext.userId === undefined) { | ||
| authContext = initialAuthState != null ? initialAuthState : {}; | ||
| } |
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.
These lines have a bug that wrongly returns the initial auth state during the "transitive state". This can cause you to appear signed out right after sign in for example.
Removing this also fixes the bug.
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.
Note for self: This is no longer a context, rename this file and hook to match the other hooks.
| const state = useSyncExternalStore( | ||
| useCallback(callback => clerk.addListener(callback, { skipInitialEmit: true }), [clerk]), | ||
| useCallback(() => { | ||
| if (!clerk.loaded || !clerk.__internal_lastEmittedResources) { | ||
| return getInitialState(); | ||
| } | ||
|
|
||
| return clerk.__internal_lastEmittedResources; | ||
| }, [clerk, getInitialState]), | ||
| getInitialState, | ||
| ); |
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.
Using useSyncExternalStore in all hooks should hopefully fix the partial hydration mismatches we've seen at times.
| // If we want to support passing initialState as a promise, this is where we would handle that. | ||
| // Note that we can not use(promise) as long as we support React 18, so we'll need to have some extra handling | ||
| // and throw the promise instead. |
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.
The idea is to implement this as a quick follow up. The reason we want this is to stop awaiting the initialState at the top level provider as one thing we should do to enable better PPR support.
I tried this with use() just to verify it worked, but an implementation with throwing is slightly trickier so wanted to do that as a separate PR.
| user: this.user, | ||
| organization: this.organization, | ||
| }; | ||
| this.__internal_lastEmittedResources = resources; |
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.
clever!
…s-from-provider-and-rely-on
…s-from-provider-and-rely-on
…s-from-provider-and-rely-on
Description
This picks up where #7194 and #7267 left off.
I'll update this description as this gets closer to being review-ready, for now there's a bunch of context in the old PRs for the curious.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.