feat(shared,types): Migrate @clerk/types to @clerk/shared/types #7022
Conversation
🦋 Changeset detectedLatest commit: 955dce8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
WalkthroughMigrates most type imports from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Adapter as langchain.adapter
participant ClerkTool as clerkTool
participant ClerkClient as clerkClient
participant LC_Tool as StructuredTool
Note over Adapter,ClerkTool: previous flow (inline call) replaced by two-step setup
Adapter->>ClerkTool: bindExecute(clerkClient, context) -> executeFn
Adapter->>Adapter: build toolConfig { name, description, schema }
Adapter->>LC_Tool: tool(executeFn, toolConfig) -> returns StructuredTool
LC_Tool-->>Adapter: tool object (used by runtime)
Note right of Adapter: casts applied to satisfy typings
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
clerk-cookie
added
types
clerk-js
backend
chrome-extension
themes
elements
astro
vue
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
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/react/contexts.tsx (1)
26-26: OptionsContext default prevents the intended error path.createContext is initialized with {}, so useOptionsContext can never see undefined and won’t throw when provider is missing. Initialize with undefined to restore the guard.
-const OptionsContext = React.createContext<ClerkOptions>({}); +const OptionsContext = React.createContext<ClerkOptions | undefined>(undefined); function useOptionsContext(): ClerkOptions { const context = React.useContext(OptionsContext); - if (context === undefined) { + if (context === undefined) { throw new Error('useOptions must be used within an OptionsContext'); } return context; }As per coding guidelines.
Also applies to: 43-49
🧹 Nitpick comments (28)
packages/shared/src/react/hooks/useOrganizationList.tsx (1)
104-104: Avoid shadowing the type name with the parameter name.On line 104, the parameter is named
CreateOrganizationParams, which shadows the imported type of the same name. This creates confusion and violates naming conventions. Use a lowercase, descriptive parameter name instead.- createOrganization: (CreateOrganizationParams: CreateOrganizationParams) => Promise<OrganizationResource>; + createOrganization: (params: CreateOrganizationParams) => Promise<OrganizationResource>;packages/shared/tsconfig.json (1)
5-5: Minor: Consider broadeningtypesif package is meant for browser environments.Currently
types: ["node"]restricts ambient types to Node only. Sincelibincludes"DOM"and"WebWorker", this is functionally sufficient (those libs provide necessary types), but if the package is meant to be used in both Node and browser contexts, you may want to reconsider this restriction or document it clearly. This is a minor point if usage is Node-only.packages/shared/src/types/utils.ts (4)
92-103: AvoidFunctionand the ESLint override; detect callables safely.Use a callable signature instead of
Function, remove the disable, and keep “no any” where possible.-// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type -type IsSerializable<T> = T extends Function ? false : true; +type IsSerializable<T> = T extends (...args: unknown[]) => unknown ? false : true;As per coding guidelines.
40-46: DeepPartial/DeepRequired should respect arrays, dates, errors, and functions.Current
extends objectrecurses into arrays/functions and mutates built-ins. Guard common built-ins and handle arrays.-export type DeepPartial<T> = { - [P in keyof T]?: T[P] extends object ? DeepPartial<T[P]> : T[P]; -}; +export type DeepPartial<T> = + T extends (...args: unknown[]) => unknown ? T : + T extends Date | RegExp | Error ? T : + T extends readonly (infer U)[] ? readonly DeepPartial<U>[] : + T extends (infer U)[] ? DeepPartial<U>[] : + T extends object ? { [P in keyof T]?: DeepPartial<T[P]> } : + T; -export type DeepRequired<T> = Required<{ - [P in keyof T]: T[P] extends object | undefined ? DeepRequired<Required<T[P]>> : T[P]; -}>; +export type DeepRequired<T> = + T extends (...args: unknown[]) => unknown ? T : + T extends Date | RegExp | Error ? T : + T extends readonly (infer U)[] ? readonly DeepRequired<U>[] : + T extends (infer U)[] ? DeepRequired<U>[] : + T extends object ? { [P in keyof T]-?: DeepRequired<NonNullable<T[P]>> } : + T;As per coding guidelines.
21-35: Camel→snake currently prefixes an underscore for leading uppercase and splits every uppercase run.
'FooBar'becomes'_foo_bar'. Prefer a head/tail approach to avoid a leading underscore and only underscore on transitions.Add helpers (place near these types):
type SnakeFromRest<S extends string> = S extends `${infer C}${infer R}` ? C extends Lowercase<C> ? `${C}${SnakeFromRest<R>}` : `_${Lowercase<C>}${SnakeFromRest<R>}` : ''; type CamelToSnakeString<S extends string> = S extends `${infer First}${infer Rest}` ? `${Lowercase<First>}${SnakeFromRest<Rest>}` : S;Then update:
-export type DeepCamelToSnake<T> = T extends `${infer C0}${infer R}` - ? `${C0 extends Uppercase<C0> ? '_' : ''}${Lowercase<C0>}${DeepCamelToSnake<R>}` - : T extends object - ? { - [K in keyof T as DeepCamelToSnake<Extract<K, string>>]: DeepCamelToSnake<T[K]>; - } - : T; +export type DeepCamelToSnake<T> = + T extends string + ? CamelToSnakeString<T> + : T extends object + ? { [K in keyof T as K extends string ? CamelToSnakeString<K> : K]: DeepCamelToSnake<T[K]> } + : T; -export type CamelToSnake<T> = T extends `${infer C0}${infer R}` - ? `${C0 extends Uppercase<C0> ? '_' : ''}${Lowercase<C0>}${CamelToSnake<R>}` - : T extends object - ? { - [K in keyof T as CamelToSnake<Extract<K, string>>]: T[K]; - } - : T; +export type CamelToSnake<T> = + T extends string + ? CamelToSnakeString<T> + : T extends object + ? { [K in keyof T as K extends string ? CamelToSnakeString<K> : K]: T[K] } + : T;As per coding guidelines.
9-19: Constrain key remapping to strings and add array handling for deep variant.Prevents accidental remapping of symbol/number keys and improves consistency.
-export type SnakeToCamel<T> = T extends `${infer A}_${infer B}` - ? `${Uncapitalize<A>}${Capitalize<SnakeToCamel<B>>}` - : T extends object - ? { [K in keyof T as SnakeToCamel<K>]: T[K] } - : T; +export type SnakeToCamel<T> = + T extends string + ? (T extends `${infer A}_${infer B}` ? `${Uncapitalize<A>}${Capitalize<SnakeToCamel<B>>}` : T) + : T extends object + ? { [K in keyof T as K extends string ? SnakeToCamel<K> : K]: T[K] } + : T; -export type DeepSnakeToCamel<T> = T extends `${infer A}_${infer B}` - ? `${Uncapitalize<A>}${Capitalize<DeepSnakeToCamel<B>>}` - : T extends object - ? { [K in keyof T as DeepSnakeToCamel<K>]: DeepSnakeToCamel<T[K]> } - : T; +export type DeepSnakeToCamel<T> = + T extends string + ? (T extends `${infer A}_${infer B}` ? `${Uncapitalize<A>}${Capitalize<DeepSnakeToCamel<B>>}` : T) + : T extends readonly (infer U)[] ? readonly DeepSnakeToCamel<U>[] : + T extends (infer U)[] ? DeepSnakeToCamel<U>[] : + T extends object ? { [K in keyof T as K extends string ? DeepSnakeToCamel<K> : K]: DeepSnakeToCamel<T[K]> } : + T;As per coding guidelines.
pnpm-workspace.yaml (1)
28-28: v1.0.0-beta.43 is currently the latest stable release, but consider caution due to recency.v1.0.0-beta.43 is the current stable release (published Oct 13, 2025). While it's officially stable, it was released only 4 days ago. The original recommendations remain prudent:
- Review the rolldown changelog for fixes and changes in this version
- Set up monitoring for build failures during the rollout
- Have a rollback plan if unforeseen issues arise
packages/shared/src/types/organizationInvitation.ts (1)
21-21: Minor JSDoc formatting change - no functional impact.The added blank line in the JSDoc comment is a formatting-only change. While it doesn't affect functionality, ensure JSDoc formatting is consistent across the codebase.
packages/themes/src/createTheme.ts (1)
1-3: Type import migrated successfully. Consider updating the comment.The type imports have been correctly migrated to
@clerk/shared/types. However, the comment on lines 1-2 mentions this is a "temp way to import the type" and references extracting theming into its own package. With the type migration now complete, consider updating or removing this comment if it's no longer accurate.packages/shared/src/authorization.ts (1)
344-344: Export order changed - non-breaking but worth noting.The exported functions have been reordered (from
createCheckAuthorization, validateReverificationConfig, resolveAuthState, splitByScopetocreateCheckAuthorization, resolveAuthState, splitByScope, validateReverificationConfig). Since these are named exports, this change is non-breaking and has no runtime impact. The reordering appears intentional but consider whether a consistent ordering convention (alphabetical, by importance, etc.) should be documented.packages/shared/src/types/appearance.ts (1)
657-856: Excellent documentation improvements.The enhanced JSDoc comments with
@defaultvalues and@deprecatedannotations significantly improve the developer experience. The deprecation guidance properly directs developers to new property names.Consider spot-checking a few default values to ensure accuracy, especially for color values like:
- Line 658:
colorPrimarydefault '#2F3037'- Line 680:
colorDangerdefault '#EF4444'- Line 686:
colorSuccessdefault '#22C543'packages/shared/package.json (2)
155-170: Consider marking package as side-effect free.If modules are pure, add
"sideEffects": falseto improve tree-shaking."peerDependenciesMeta": { @@ "engines": { "node": ">=18.17.0" }, + "sideEffects": false, "publishConfig": {
139-143: Remove unnecessarysrcargument from dev script for consistency.The
tsdown.config.mtsalready explicitly defines entry points (entry: ['./src/types/index.ts']), making thesrcargument in the npm script redundant. All other packages in the monorepo use--watchwithout a path argument, relying on their configuration files to define entry points. Removing this argument aligns with the established pattern.- "dev": "tsdown --watch src", + "dev": "tsdown --watch",packages/shared/src/types/billing.ts (2)
620-671: Mark money amount object fields readonly (optional).Immutability reduces accidental mutation and aligns with value-object semantics.
-export interface BillingMoneyAmount { +export interface BillingMoneyAmount { /** * The raw amount as a number, usually in the smallest unit of the currency (like cents for USD). For example, `1000` for $10.00. */ - amount: number; + readonly amount: number; @@ - amountFormatted: string; + readonly amountFormatted: string; @@ - currency: string; + readonly currency: string; @@ - currencySymbol: string; + readonly currencySymbol: string; }As per coding guidelines.
281-332: Return type could be void instead of null (optional).
makeDefaultreturnsPromise<null>. ConsiderPromise<void>unlessnullis significant to callers.- makeDefault: (params?: MakeDefaultPaymentMethodParams) => Promise<null>; + makeDefault: (params?: MakeDefaultPaymentMethodParams) => Promise<void>;packages/themes/tsconfig.json (1)
3-21: Align TypeScript config: lib/target mismatch, enable unused locals check, disable unnecessary JS compilation.The package contains only TypeScript files and has tslib properly declared. Three improvements are recommended:
libshould matchtarget(use es2020 instead of es6)noUnusedLocalsshould be enabled for consistency withnoUnusedParametersallowJsis unnecessary; disable it for a TS-only package- "lib": ["es6", "dom"], + "lib": ["es2020", "dom"], @@ - "noUnusedLocals": false, + "noUnusedLocals": true, @@ - "allowJs": true, + "allowJs": falsepackages/shared/src/types/json copy.ts (3)
133-133: Replaceanywithunknown(and narrow where needed).
external_account: any→unknown(or a precise union if known).claims: Record<string, any>→Record<string, unknown>.Apply:
- external_account: any; + external_account: unknown; @@ - claims: Record<string, any> | null; + claims: Record<string, unknown> | null;As per coding guidelines (avoid any).
Also applies to: 849-849
799-801: Prefertypealias over empty interface with eslint-disable.This avoids disabling lint and is clearer.
Apply:
-// eslint-disable-next-line @typescript-eslint/no-empty-object-type -export interface BillingStatementTotalsJSON extends Omit<BillingCheckoutTotalsJSON, 'total_due_now'> {} +export type BillingStatementTotalsJSON = Omit<BillingCheckoutTotalsJSON, 'total_due_now'>;As per coding guidelines.
194-197: Consider narrowingobjectto a string literal for discriminated unions.Several resources extend ClerkResourceJSON but don’t narrow
object(e.g., IdentificationLinkJSON, SignUpEnterpriseConnectionJSON, EnterpriseAccountConnectionJSON, ApiKeyJSON). Narrowing improves type safety and switches.Please confirm the canonical
objectvalues and update accordingly. Example:export interface IdentificationLinkJSON extends ClerkResourceJSON { - id: string; - type: string; + object: 'identification_link'; + id: string; + type: string; }As per coding guidelines.
Also applies to: 147-151, 260-274, 843-859
packages/astro/src/stores/external.ts (1)
55-55: Narrow the session store type (drop undefined or justify)Unless
$authStorecan produceundefined, preferSignedInSessionResource | nullto reduce consumer nullability.-export const $sessionStore = computed([$authStore], auth => auth.session as SignedInSessionResource | null | undefined); +export const $sessionStore = computed([$authStore], auth => auth.session as SignedInSessionResource | null);If
undefinedis expected before load, consider typing viacomputed< SignedInSessionResource | null | undefined >(...)and document the pre‑load state.packages/shared/src/types/json.ts (3)
133-141: Avoidanyin public JSON typesUse
unknownforexternal_accountto comply with TS guidelines and avoid unsafe any.- external_account: any; + external_account: unknown;As per coding guidelines.
849-851: Preferunknownoveranyfor claims
Record<string, any>weakens type safety. Useunknown.- claims: Record<string, any> | null; + claims: Record<string, unknown> | null;As per coding guidelines.
351-362:VerificationJSON.errorlikely optionalErrors exist only on failed verifications; make
erroroptional to model API reality.- error: ClerkAPIErrorJSON; + error?: ClerkAPIErrorJSON;packages/backend/src/util/shared.ts (1)
12-15: Stale build-tool note.Comment references tsup; repo is moving to tsdown/rolldown. Update the TODO to match current toolchain to avoid confusion.
packages/shared/src/workerTimers/workerTimers.built.ts (2)
1-19: Mark as generated and exclude from lint/coverage.Add standard headers so tools don’t “fix” this file and coverage ignores it.
/** * * This is the minified string output of transforming workerTimers.worker.ts @@ * }; * */ +/* @generated File is auto-produced from workerTimers.worker.ts — do not edit by hand. */ +/* eslint-disable */ +/* istanbul ignore file */
20-20: Worker timer logic: OK; note CSP requirement (operational).The blob URL worker pattern requires allowing blob: in worker-src/connect-src in strict CSPs. Document this where createWorkerTimers consumes this string.
packages/shared/src/react/contexts.tsx (2)
36-38: Add explicit return type for public component.Annotate __experimental_CheckoutProvider return type.
-const __experimental_CheckoutProvider = ({ children, ...rest }: PropsWithChildren<UseCheckoutOptions>) => { +const __experimental_CheckoutProvider = ( + { children, ...rest }: PropsWithChildren<UseCheckoutOptions> +): React.JSX.Element => { return <CheckoutContext.Provider value={{ value: rest }}>{children}</CheckoutContext.Provider>; };As per coding guidelines.
62-67: Avoid any; prefer unknown (or a minimal local type) for swrConfig.Using any here leaks unsoundness. If importing SWR types breaks dts, use unknown instead.
- swrConfig?: any; + swrConfig?: unknown;As per coding guidelines.
| "rolldown": "catalog:repo", | ||
| "statuses": "^1.5.0", | ||
| "tree-kill": "^1.2.2", | ||
| "ts-jest": "29.2.5", | ||
| "tsdown": "catalog:repo", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the new build tooling is properly configured across all packages.
Two new build tools have been added as dev dependencies:
rolldown(line 138) - a Rust-based bundlertsdown(line 142) - a TypeScript declaration bundler
Since this is part of the migration from tsup to tsdown/rolldown, ensure that:
- All packages using these tools have appropriate configuration files (e.g.,
tsdown.config.mts) - Build scripts in individual packages are updated to use the new tooling
- The build pipeline works end-to-end with the new tools
Run the following to verify the build tooling setup:
🏁 Script executed:
#!/bin/bash
# Check for tsdown config files
echo "=== Checking for tsdown configuration files ==="
fd -t f "tsdown.config" packages/
# Check if tsup configs still exist (should be removed/replaced)
echo -e "\n=== Checking for remaining tsup configs (should be minimal) ==="
fd -t f "tsup.config" packages/
# Check package.json build scripts that reference the new tools
echo -e "\n=== Checking for build script references ==="
fd -t f "package.json" packages/ --exec grep -l "tsdown\|rolldown" {}
# Verify the new tools are in the catalog
echo -e "\n=== Checking pnpm workspace catalog ==="
cat pnpm-workspace.yaml | grep -A 2 "tsdown\|rolldown"Length of output: 986
I need to verify if this partial migration is intentional and check the build scripts in packages that still use tsup.
Let me check the build scripts to see which tools are being used in the packages:
Let me examine the actual build scripts in the packages to understand the state better:
🏁 Script executed:
#!/bin/bash
# Check build scripts in shared package (already migrated)
echo "=== shared package.json build scripts ==="
cat packages/shared/package.json | grep -A 20 '"scripts"'
# Check one package still using tsup
echo -e "\n=== One example: agent-toolkit package.json build scripts ==="
cat packages/agent-toolkit/package.json | grep -A 20 '"scripts"'
# Count packages and their tooling
echo -e "\n=== Build tool references in package.json files ==="
echo "Packages with tsdown reference:"
fd -t f "package.json" packages/ --exec grep -l "tsdown" {} | wc -l
echo "Packages with tsup reference:"
fd -t f "package.json" packages/ --exec grep -l "tsup" {} | wc -l
echo "Packages with neither (might use rolldown directly):"
fd -t f "package.json" packages/ -not -exec grep -l "tsdown\|tsup\|rolldown" {} \; | wc -lLength of output: 2357
Build tooling migration is incomplete—only 1 of ~20 packages has been migrated to tsdown.
The verification confirms the review comment's concern: the migration from tsup to tsdown/rolldown is not yet complete across all packages.
Current state:
- 1 package migrated:
packages/sharedhastsdown.config.mtsand usestsdownin build scripts - 19 packages unmigrated: Still have
tsup.config.tsfiles andtsupcommands in build scripts (agent-toolkit,astro,backend,chrome-extension,elements,expo,expo-passkeys,express,fastify,localizations,nextjs,nuxt,react,react-router,remix,tanstack-react-start,testing,themes,vue)
Both tooling versions remain in the pnpm workspace catalog (tsup: 8.5.0, tsdown: 0.15.7, rolldown: 1.0.0-beta.43).
Either complete the migration across remaining packages or document why the partial migration is intentional.
🤖 Prompt for AI Agents
package.json lines 138-142: the repo shows only one package migrated to
tsdown/rolldown while ~19 still use tsup; either finish migration or document
the intentional partial state. To fix, either (A) complete migration across the
listed packages by converting each package's tsup.config.ts to tsdown (create
tsdown.config.mts where needed), update package.json build scripts to use
tsdown/rolldown commands, update the pnpm workspace catalog to remove/replace
tsup with the chosen tsdown/rolldown versions and ensure lockfile is refreshed,
and remove tsup entries from package.json; or (B) if partial migration is
intentional, add a top-level migration note in the repo (README or MIGRATION.md)
listing migrated packages, those pending, rationale, and an action
plan/timeline, and keep package.json entries consistent with that documentation.
| @@ -1,5 +1,6 @@ | |||
| import { deriveState } from '@clerk/shared/deriveState'; | |||
| import { eventMethodCalled } from '@clerk/shared/telemetry'; | |||
| import type { SignedInSessionResource } from '@clerk/types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
Leftover import from @clerk/types—migrate to @clerk/shared/types
This file still pulls SignedInSessionResource from @clerk/types. Switch to the shared path to align with the migration.
-import type { SignedInSessionResource } from '@clerk/types';
+import type { SignedInSessionResource } from '@clerk/shared/types';🏁 Script executed:
#!/usr/bin/env bash
# Audit repo for lingering @clerk/types imports
rg -nP --type=ts --type=tsx --type=js --type=jsx -g '!**/node_modules/**' "\@clerk/types"Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Audit repo for lingering @clerk/types imports with corrected syntax
rg -nP "@clerk/types" --type ts --type js -g '!**/node_modules/**'
Length of output: 68479
🏁 Script executed:
#!/bin/bash
# Verify the specific file mentioned in the review
cat packages/astro/src/stores/external.tsLength of output: 5381
🏁 Script executed:
#!/bin/bash
# Check if @clerk/shared/types exists and what it exports
find . -path '*/node_modules' -prune -o -type f -name 'index.ts' -o -name 'types.ts' | grep -i "clerk/shared"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find the @clerk/shared package structure
find packages/shared -type f -name "*.ts" -o -name "*.tsx" | head -20Length of output: 992
🏁 Script executed:
#!/bin/bash
# Check package.json exports for @clerk/shared
cat packages/shared/package.json 2>/dev/null | grep -A 20 '"exports"'Length of output: 512
🏁 Script executed:
#!/bin/bash
# Search for SignedInSessionResource export in @clerk/shared
rg -n "SignedInSessionResource" packages/shared --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Search for SignedInSessionResource in packages/shared (corrected syntax)
rg -n "SignedInSessionResource" packages/shared --type tsLength of output: 1544
🏁 Script executed:
#!/bin/bash
# Check packages/types to see where SignedInSessionResource is defined
rg -n "SignedInSessionResource" packages/types/src --type ts -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# List top-level source files in packages/shared
ls -la packages/shared/src/*.ts 2>/dev/null | head -30Length of output: 2575
🏁 Script executed:
#!/bin/bash
# Check if packages/shared has a types.ts or similar
find packages/shared/src -maxdepth 1 -name "*types*" -o -name "*index*"Length of output: 54
🏁 Script executed:
#!/bin/bash
# Verify packages/types re-exports from @clerk/shared
cat packages/types/src/index.d.ts 2>/dev/null | head -30Length of output: 313
🏁 Script executed:
#!/bin/bash
# Check if other astro files still use @clerk/types for comparison
rg -n "@clerk/types" packages/astro/src --type tsLength of output: 1984
Migrate import to @clerk/shared/types for consistency with package deprecation
The review comment is correct. The packages/types package documentation explicitly states that "@clerk/types is now merged with @clerk/shared" and indicates "Please import from @clerk/shared/types instead." The SignedInSessionResource type is available from @clerk/shared/types, and this file already imports from @clerk/shared (lines 1–2), so the import should be migrated.
-import type { SignedInSessionResource } from '@clerk/types';
+import type { SignedInSessionResource } from '@clerk/shared/types';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { SignedInSessionResource } from '@clerk/types'; | |
| import type { SignedInSessionResource } from '@clerk/shared/types'; |
🤖 Prompt for AI Agents
In packages/astro/src/stores/external.ts around line 3, the file imports
SignedInSessionResource from '@clerk/types' but the package has been merged and
types should be imported from '@clerk/shared/types'; update the import to pull
SignedInSessionResource from '@clerk/shared/types' (keeping any existing imports
from '@clerk/shared') and remove the '@clerk/types' import to maintain
consistency with the deprecation.
|
|
||
| export class BillingSubscriptionItem extends BaseResource implements BillingSubscriptionItemResource { | ||
| id!: string; | ||
| paymentMethodId!: string; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that paymentMethodId should be a required field.
The field is marked as required with the ! operator, which means it cannot be null or undefined. Please ensure that the backend always provides payment_method_id in the JSON response.
Run the following script to check if there are any existing tests or usage patterns that might expect this field to be optional:
🏁 Script executed:
#!/bin/bash
# Search for BillingSubscriptionItem usage and check if paymentMethodId is always present
echo "Checking for BillingSubscriptionItem usage patterns..."
rg -n -C3 "BillingSubscriptionItem" --type ts --type tsx -g '!**/dist/**' -g '!**/node_modules/**'
echo -e "\nChecking for payment_method_id in test fixtures or mocks..."
rg -n "payment_method_id" --type ts --type tsx --type json -g '!**/dist/**' -g '!**/node_modules/**'Length of output: 171
🏁 Script executed:
#!/bin/bash
# Find and examine the BillingSubscription.ts file
echo "=== Finding BillingSubscription.ts ==="
fd -n "BillingSubscription.ts" --type f
# Look at the file content if it exists
echo -e "\n=== Content of BillingSubscription.ts ==="
find . -name "BillingSubscription.ts" -type f ! -path "*/node_modules/*" ! -path "*/dist/*" -exec cat {} \;
# Search for paymentMethodId usage more broadly
echo -e "\n=== Searching for paymentMethodId usage ==="
rg -n "paymentMethodId" -g '!**/dist/**' -g '!**/node_modules/**'
# Search for payment_method_id (snake_case) in all file types
echo -e "\n=== Searching for payment_method_id ==="
rg -n "payment_method_id" -g '!**/dist/**' -g '!**/node_modules/**'Length of output: 5883
🏁 Script executed:
#!/bin/bash
# Find and examine the BillingSubscription.ts file
echo "=== Finding BillingSubscription.ts ==="
fd -n "BillingSubscription.ts" --type f
# Look at the file content if it exists
echo -e "\n=== Content of BillingSubscription.ts ==="
find . -name "BillingSubscription.ts" -type f ! -path "*/node_modules/*" ! -path "*/dist/*" -exec cat {} \;
# Search for paymentMethodId usage more broadly
echo -e "\n=== Searching for paymentMethodId usage ==="
rg -n "paymentMethodId" -g '!**/dist/**' -g '!**/node_modules/**'
# Search for payment_method_id (snake_case) in all file types
echo -e "\n=== Searching for payment_method_id ==="
rg -n "payment_method_id" -g '!**/dist/**' -g '!**/node_modules/**'
# Search for BillingSubscriptionItem definition
echo -e "\n=== Searching for BillingSubscriptionItem ==="
rg -n "BillingSubscriptionItem" -g '!**/dist/**' -g '!**/node_modules/**'Length of output: 17657
🏁 Script executed:
#!/bin/bash
# Check the BillingSubscriptionItemJSON interface definition
echo "=== BillingSubscriptionItemJSON interface in packages/types/src/json.ts ==="
rg -n -A 30 "interface BillingSubscriptionItemJSON" packages/types/src/json.ts
echo -e "\n=== BillingSubscriptionItemJSON interface in packages/shared/src/types/json.ts ==="
rg -n -A 30 "interface BillingSubscriptionItemJSON" packages/shared/src/types/json.ts
# Check if there's any code path that could leave paymentMethodId undefined
echo -e "\n=== Check for optional handling or null checks on paymentMethodId ==="
rg -n -B2 -A2 "paymentMethodId\s*=" packages/clerk-js/src/core/resources/BillingSubscription.tsLength of output: 3003
🏁 Script executed:
#!/bin/bash
# Get full BillingSubscriptionItemJSON interface from packages/types
echo "=== Full BillingSubscriptionItemJSON from packages/types/src/json.ts ==="
rg -n -A 50 "interface BillingSubscriptionItemJSON" packages/types/src/json.ts | head -60
# Check if there's any mention of payment_method_id in packages/types/src/json.ts
echo -e "\n=== Looking for payment_method_id in packages/types/src/json.ts ==="
rg -n "payment_method_id" packages/types/src/json.ts
# Check package resolution - what does @clerk/types point to
echo -e "\n=== Check @clerk/types resolution ==="
find . -name "package.json" -path "*/packages/types/*" ! -path "*/node_modules/*" -exec grep -l "@clerk/types\|clerk-js" {} \;Length of output: 2364
Add missing payment_method_id field to BillingSubscriptionItemJSON interface in packages/types/src/json.ts.
The type definition is incomplete. While packages/shared/src/types/json.ts includes payment_method_id: string; in the BillingSubscriptionItemJSON interface at line 733, packages/types/src/json.ts is missing this required field. The implementation at packages/clerk-js/src/core/resources/BillingSubscription.ts line 90 assigns data.payment_method_id, but this field doesn't exist in the imported type definition, creating a type safety gap.
The field should be added to packages/types/src/json.ts after the credit property (around line 732):
payment_method_id: string;This ensures consistency across type definitions and maintains type safety for the required field marked with ! at line 63 of BillingSubscription.ts.
🤖 Prompt for AI Agents
In packages/types/src/json.ts around line ~732, the BillingSubscriptionItemJSON
interface is missing the required payment_method_id field referenced in
packages/clerk-js/src/core/resources/BillingSubscription.ts (line 63) and used
elsewhere; add payment_method_id: string; immediately after the credit property
in that interface so the type matches packages/shared/src/types/json.ts and the
implementation that reads data.payment_method_id compiles with proper type
safety.
| // @ts-expect-error - ignore error for now | ||
| clerk, |
There was a problem hiding this comment.
Address type mismatch before merging.
The @ts-expect-error suppression indicates that the clerk property doesn't match the SignInRouterInitEvent type signature. This technical debt should be resolved before the PR is merged out of WIP status.
Consider either:
- Updating the
SignInRouterInitEventtype definition to include theclerkproperty, or - Removing the
clerkproperty if it's not needed
To verify the current type definition:
#!/bin/bash
# Find the SignInRouterInitEvent type definition
ast-grep --pattern 'type SignInRouterInitEvent = {
$$$
}'🤖 Prompt for AI Agents
In packages/elements/src/react/sign-in/root.tsx around lines 44 to 45, the clerk
property is being passed despite a TypeScript mismatch (suppressed with
@ts-expect-error); resolve by either updating the SignInRouterInitEvent type to
include clerk (extend the interface/type with the correct clerk shape) or remove
the clerk property where it’s unused, and then remove the @ts-expect-error
comment; ensure any changes are mirrored in the type definition file found by
the project’s type declarations so the compiler no longer reports a mismatch.
| // @ts-expect-error - ignore error for now | ||
| clerk, |
There was a problem hiding this comment.
Address type mismatch before merging.
This has the same issue as packages/elements/src/react/sign-in/root.tsx (Lines 44-45). The @ts-expect-error suppression indicates a type mismatch that should be resolved before merging.
Update the SignUpRouterInitEvent type definition to properly include the clerk property:
#!/bin/bash
# Find the SignUpRouterInitEvent type definition
ast-grep --pattern 'type SignUpRouterInitEvent = {
$$$
}'
packages/shared/vitest.config.mts
Outdated
| }, | ||
| environment: 'jsdom', | ||
| include: ['**/*.{spec,test}.{js,ts,jsx,tsx}'], | ||
| include: ['**/*..{js,ts,jsx,tsx}'], |
There was a problem hiding this comment.
🧩 Analysis chain
Broken test include glob stops Vitest from discovering tests.
Current pattern '**/*..{js,ts,jsx,tsx}' is invalid. Use standard spec/test glob.
- include: ['**/*..{js,ts,jsx,tsx}'],
+ include: ['**/*.{spec,test}.{js,ts,jsx,tsx}'],🏁 Script executed:
#!/bin/bash
# List matched tests with the new pattern to confirm discovery.
fd -t f -a -g '**/*.{spec,test}.{js,ts,jsx,tsx}' packages/shared | sed 's/^/test file: /'Length of output: 3222
This will show me the actual file content to verify if the broken pattern exists and what the current state is.
🏁 Script executed:
#!/bin/bash
# Read the vitest.config.mts file, specifically around line 16
sed -n '10,25p' packages/shared/vitest.config.mts | head -20Length of output: 224
This will verify the proposed pattern captures all test files and nothing is missed.
🏁 Script executed:
#!/bin/bash
# Check if there are any test files using .test. naming (not .spec.)
fd -t f -g '**/*.test.{js,ts,jsx,tsx}' packages/sharedLength of output: 0
Fix broken Vitest include glob to restore test discovery.
The current pattern **/*..{js,ts,jsx,tsx} contains invalid glob syntax and matches no files. Update to the standard pattern:
- include: ['**/*..{js,ts,jsx,tsx}'],
+ include: ['**/*.{spec,test}.{js,ts,jsx,tsx}'],This fix enables discovery of all 37 test files in packages/shared/src.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| include: ['**/*..{js,ts,jsx,tsx}'], | |
| include: ['**/*.{spec,test}.{js,ts,jsx,tsx}'], |
🤖 Prompt for AI Agents
In packages/shared/vitest.config.mts around line 16 the include glob is invalid
(`**/*..{js,ts,jsx,tsx}`) and matches nothing; replace it with a correct
file-extension glob (for example `**/*.{js,ts,jsx,tsx}`) so Vitest can discover
test files.
| "exports": { | ||
| ".": { | ||
| "import": { | ||
| "types": "./src/index.d.mts", | ||
| "default": "./src/index.mjs" | ||
| }, | ||
| "require": { | ||
| "types": "./src/index.d.ts", | ||
| "default": "./src/index.js" | ||
| } | ||
| } | ||
| }, | ||
| "main": "src/index.js", | ||
| "module": "src/index.js", | ||
| "types": "src/index.d.ts", | ||
| "files": [ |
There was a problem hiding this comment.
🧩 Analysis chain
Exports map: prefer canonical 'types' mapping and verify files exist.
Placing types nested under import/require is non-standard and may confuse TS resolvers/publint. Use a top-level types entry for the subpath.
"exports": {
".": {
- "import": {
- "types": "./src/index.d.mts",
- "default": "./src/index.mjs"
- },
- "require": {
- "types": "./src/index.d.ts",
- "default": "./src/index.js"
- }
+ "types": "./src/index.d.ts",
+ "import": "./src/index.mjs",
+ "require": "./src/index.js",
+ "default": "./src/index.js"
}
},As per coding guidelines.
🏁 Script executed:
#!/bin/bash
# Ensure exported files actually exist before publishing.
for f in packages/types/src/index.js packages/types/src/index.mjs packages/types/src/index.d.ts; do
[[ -f "$f" ]] || { echo "Missing $f" >&2; fi
done
# Spot-check nested 'types' usage (should be absent after change)
jq -r '.exports["."].import.types? // empty' packages/types/package.json
jq -r '.exports["."].require.types? // empty' packages/types/package.jsonLength of output: 134
🏁 Script executed:
#!/bin/bash
# Ensure exported files actually exist before publishing.
for f in packages/types/src/index.js packages/types/src/index.mjs packages/types/src/index.d.ts; do
if [[ -f "$f" ]]; then
echo "Found: $f"
else
echo "Missing: $f" >&2
fi
done
# Check current exports structure
echo "=== Current exports structure ==="
jq '.exports' packages/types/package.json
# Spot-check nested 'types' usage in import/require
echo "=== Nested types in import ==="
jq -r '.exports["."].import.types? // "not found"' packages/types/package.json
echo "=== Nested types in require ==="
jq -r '.exports["."].require.types? // "not found"' packages/types/package.jsonLength of output: 442
🏁 Script executed:
#!/bin/bash
# Check specifically for index.d.mts
if [[ -f "packages/types/src/index.d.mts" ]]; then
echo "Found: packages/types/src/index.d.mts"
else
echo "Missing: packages/types/src/index.d.mts" >&2
fi
# List all files in packages/types/src to see what's actually there
echo "=== Files in packages/types/src/ ==="
ls -la packages/types/src/Length of output: 549
Exports map: prefer canonical 'types' mapping - verified.
All exported files exist (index.js, index.mjs, index.d.mts, index.d.ts). The current nested types structure under import/require is non-standard and should be flattened to a top-level types entry for proper TypeScript and tool resolution.
"exports": {
".": {
- "import": {
- "types": "./src/index.d.mts",
- "default": "./src/index.mjs"
- },
- "require": {
- "types": "./src/index.d.ts",
- "default": "./src/index.js"
- }
+ "types": "./src/index.d.ts",
+ "import": "./src/index.mjs",
+ "require": "./src/index.js",
+ "default": "./src/index.js"
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "exports": { | |
| ".": { | |
| "import": { | |
| "types": "./src/index.d.mts", | |
| "default": "./src/index.mjs" | |
| }, | |
| "require": { | |
| "types": "./src/index.d.ts", | |
| "default": "./src/index.js" | |
| } | |
| } | |
| }, | |
| "main": "src/index.js", | |
| "module": "src/index.js", | |
| "types": "src/index.d.ts", | |
| "files": [ | |
| "exports": { | |
| ".": { | |
| "types": "./src/index.d.ts", | |
| "import": "./src/index.mjs", | |
| "require": "./src/index.js", | |
| "default": "./src/index.js" | |
| } | |
| }, | |
| "main": "src/index.js", | |
| "module": "src/index.js", | |
| "types": "src/index.d.ts", | |
| "files": [ |
🤖 Prompt for AI Agents
In packages/types/package.json around lines 26 to 41, the exports map currently
nests "types" under the "import" and "require" conditionals which is
non-standard; remove the nested "types" properties and add a single top-level
"types" entry (pointing to the canonical declaration file, e.g. src/index.d.ts)
so TypeScript and bundlers resolve typings correctly, and ensure the exports
keys keep only the runtime defaults (default/import/require) while relying on
the top-level "types" field for type resolution.
| "src" | ||
| ], | ||
| "scripts": { | ||
| "build": "tsup --env.NODE_ENV production", | ||
| "build:declarations": "tsc -p tsconfig.declarations.json", | ||
| "clean": "rimraf ./dist", | ||
| "dev": "tsup --watch", | ||
| "format": "node ../../scripts/format-package.mjs", | ||
| "format:check": "node ../../scripts/format-package.mjs --check", | ||
| "lint": "eslint src", | ||
| "lint:attw": "attw --pack . --profile node16", | ||
| "publish:local": "pnpm yalc push --replace --sig" | ||
| "lint:attw": "attw --pack . --profile node16" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Publishing only "src": confirm no build step is required.
With "files": ["src"] and main/module pointing to src, consumers will load source files. Ensure these are plain JS (.js/.mjs) and not TS, and that sourcemaps are not needed.
🏁 Script executed:
#!/bin/bash
# Verify there are no .ts in the published "src" for this package.
fd -t f -a -g 'packages/types/src/**/*' | rg -n '\.ts$' && echo "Found TS sources in published files"Length of output: 0
I need to verify the package setup more thoroughly since the initial script was inconclusive. Let me check the actual package.json entries and directly inspect the src directory structure.
🏁 Script executed:
#!/bin/bash
# Check the main/module entries and files array in packages/types/package.json
echo "=== package.json entries ==="
jq '.main, .module, .exports, .files' packages/types/package.json
echo ""
echo "=== Files in packages/types/src with extensions ==="
fd -t f . packages/types/src --max-depth 3 | head -20Length of output: 501
Remove .ts source files from published "src" directory or implement a build step.
The package currently publishes TypeScript source files to npm. With "files": ["src"], consumers will receive billing.ts and json.ts as-is. Either:
- Compile
.ts→.js/.mjsbefore publishing, or - Exclude
.tsfiles from the files array:["src", "!src/**/*.ts"], or - Use a dist/ directory from a build step
Affected files:
- packages/types/src/billing.ts
- packages/types/src/json.ts
🤖 Prompt for AI Agents
In packages/types/package.json around lines 42 to 49, the package currently
publishes raw TypeScript sources via "files": ["src"] which exposes .ts files
(billing.ts, json.ts); fix by either adding a build step that compiles TS to JS
into a dist/ (or lib/) folder and updating "files" to include only that output
(and add prepare/build scripts to run before publish), or exclude .ts sources by
changing "files" to exclude TS (for example keep "src" but add an exclusion
pattern for "**/*.ts"), and ensure package.json's "main"/"module"/"types" fields
point to the compiled outputs or .d.ts files; implement the chosen change and
update npm scripts so consumers receive only compiled artifacts.
| import type { | ||
| Clerk, | ||
| ClientResource, | ||
| InitialState, | ||
| MultiDomainAndOrProxy, | ||
| Resources, | ||
| Without, | ||
| } from '@clerk/shared/types'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify migration completeness across the entire codebase.
Since this is a large-scale type migration marked as WIP, ensure all affected files have been updated and the build configuration supports the new import paths.
🏁 Script executed:
#!/bin/bash
# Description: Comprehensive verification of the type migration
# 1. Find any remaining @clerk/types imports in shared and vue packages
echo "=== Scanning for remaining @clerk/types imports ==="
rg -n "from ['\"]@clerk/types['\"]" packages/shared/src/ packages/vue/src/ \
-g '!*.spec.ts' -g '!*.test.ts' \
|| echo "✓ No remaining @clerk/types imports in source files"
# 2. Check TypeScript configuration for path mappings
echo -e "\n=== Checking TypeScript path configurations ==="
fd -e json "tsconfig" packages/shared packages/vue --exec cat {} \; | \
jq -r 'select(.compilerOptions.paths != null) | .compilerOptions.paths'
# 3. Check if @clerk/types is still listed as a dependency
echo -e "\n=== Checking package.json dependencies ==="
echo "Shared package @clerk/types dependency:"
cat packages/shared/package.json | jq -r '.dependencies["@clerk/types"] // .devDependencies["@clerk/types"] // "Not found"'
echo "Vue package @clerk/types dependency:"
cat packages/vue/package.json | jq -r '.dependencies["@clerk/types"] // .devDependencies["@clerk/types"] // "Not found"'
# 4. Verify @clerk/shared types are properly exported
echo -e "\n=== Verifying @clerk/shared/types export in package.json ==="
cat packages/shared/package.json | jq -r '.exports."./types"'Length of output: 559
I need to verify whether the @clerk/types dependency in the vue package should be removed, and ensure the migration is truly complete across all code paths.
Now I need to run a direct verification on the codebase to understand the complete state of the migration.
🏁 Script executed:
#!/bin/bash
# Check if @clerk/types package still exists as a workspace package
echo "=== Searching for @clerk/types package definition ==="
find packages -name package.json -exec grep -l '"name".*@clerk/types' {} \;
echo -e "\n=== Looking for packages/ directories ==="
ls -la packages/ | grep -E "^d" | awk '{print $NF}'
echo -e "\n=== Full vue package.json to understand dependencies ==="
cat packages/vue/package.json | jq '.dependencies, .devDependencies' | head -40
echo -e "\n=== Checking shared package exports more carefully ==="
cat packages/shared/package.json | jq '.exports'Length of output: 2535
I need to check what's in the @clerk/types package and whether it's actually being used anywhere.
🏁 Script executed:
#!/bin/bash
# Investigate @clerk/types package and its usage
echo "=== @clerk/types package.json details ==="
cat packages/types/package.json | jq '.name, .main, .exports, .description' | head -30
echo -e "\n=== Searching for ALL @clerk/types imports in the entire codebase ==="
rg "from ['\"]@clerk/types['\"]" packages/ -n || echo "✓ No @clerk/types imports found"
echo -e "\n=== Checking if @clerk/types is referenced as a peer dependency or in any configs ==="
rg "@clerk/types" packages/vue/ -n -g '!node_modules' || echo "✓ No @clerk/types references in vue"
echo -e "\n=== Check if any packages declare @clerk/types as peer or optional dependency ==="
find packages -name package.json -exec sh -c 'echo "=== {} ===" && cat "$1" | jq ".peerDependencies, .optionalDependencies" 2>/dev/null | grep -A5 "@clerk/types" && echo "$1"' _ {} \;Length of output: 79029
Remove @clerk/types dependency from vue/package.json to complete the migration.
The code change correctly imports from @clerk/shared/types, but vue's package.json still declares @clerk/types as a workspace dependency. Since no files in the vue package import from @clerk/types, this dependency should be removed. Additionally, note that @clerk/types remains heavily used across many other packages in the codebase (elements, testing, nextjs, react, clerk-js, etc.), suggesting this is a work-in-progress migration that may require similar cleanup elsewhere.
🤖 Prompt for AI Agents
In packages/vue/src/plugin.ts around lines 4 to 11 the imports have been
switched to @clerk/shared/types but packages/vue/package.json still declares
@clerk/types as a workspace dependency; remove the @clerk/types entry from
packages/vue/package.json (from dependencies/devDependencies/peerDependencies
where it appears), update any workspace references if present, and run the
package manager (yarn/pnpm/npm install) to refresh the lockfile and ensure no
remaining references to @clerk/types remain in the vue package.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/shared/tsdown.config.mts (1)
26-34: Remove the empty comment on line 29.The standalone
//comment serves no purpose and can be removed.{ ...common, entry: [ - // './src/types/index.ts', ], unbundle: false, outDir: './dist/types', },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/shared/src/pathToRegexp.ts(1 hunks)packages/shared/tsdown.config.mts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/pathToRegexp.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/tsdown.config.mts (1)
packages/dev-cli/src/commands/watch.js (1)
watch(16-61)
⏰ 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). (2)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/shared/tsdown.config.mts (2)
7-23: LGTM! Well-structured build configuration.The common options are appropriate for a library package:
- Dual CJS/ESM output ensures broad compatibility
- React externalization prevents bundling peer dependencies
- Define block correctly injects package metadata and dev flag
- Target es2022 balances modern features with reasonable browser support
35-47: Runtime build configuration looks correct.The explicit entry patterns successfully target the known module structure (react, utils, workerTimers, dom) and exclude test files. While a wildcard pattern like
'./src/**/*.{ts,tsx}'with exclusions would be more maintainable for future additions, the current explicit approach provides clear control over build inputs and aligns with the package.json exports structure.
nikosdouvlis
force-pushed
the
nikos/migrate-types-to-shared
branch
from
5520fb7 to
083ef3d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/shared/tsdown.config.mts (1)
4-5: Node engine constraint mismatch remains unresolved.This critical issue was flagged in a previous review but has not yet been addressed. The JSON import syntax
with { type: 'json' }requires Node >=18.20.0, but 25 package.json files still declare"engines.node": ">=18.17.0", creating a breaking incompatibility on Node 18.17.x through 18.19.x.
📜 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 (2)
packages/shared/src/pathToRegexp.ts(1 hunks)packages/shared/tsdown.config.mts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/shared/src/pathToRegexp.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/shared/tsdown.config.mts (1)
packages/dev-cli/src/commands/watch.js (1)
watch(16-61)
⏰ 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). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/shared/tsdown.config.mts (1)
25-34: Types build configuration looks correct.The types build properly targets a single entry point with bundling disabled, which is appropriate for generating consolidated type declarations at
dist/types.
| { | ||
| ...common, | ||
| entry: [ | ||
| './src/*.{ts,tsx}', | ||
| './src/react/index.ts', | ||
| './src/utils/index.ts', | ||
| './src/workerTimers/index.ts', | ||
| './src/types/index.ts', | ||
| './src/dom/*.ts', | ||
| '!./src/**/*.{test,spec}.{ts,tsx}', | ||
| ], | ||
| outDir: './dist/runtime', | ||
| unbundle: true, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify intentional duplication of types entry and consider maintenance of explicit entry patterns.
Two observations:
-
Types entry duplication:
./src/types/index.tsis included in both the types build (line 30) and the runtime build (line 42). If this is intentional—to make types available at runtime—please confirm. Otherwise, this could lead to output duplication or confusion about which build produces the canonical types. -
Explicit entry pattern maintenance: The current approach uses explicit paths (lines 38-43) rather than a broad glob pattern with negations. While this provides precise control, it requires manual updates when adding new modules. Consider whether the maintenance overhead is acceptable or if a pattern like
'./src/**/*.{ts,tsx}'with explicit negations would be more maintainable.
Run this script to verify the intended structure and check for any missing source files:
🏁 Script executed:
#!/bin/bash
# Verify source structure matches entry configuration
echo "=== Source files that match glob patterns ==="
fd -e ts -e tsx . packages/shared/src --max-depth 1
echo -e "\n=== Subdirectories with index files ==="
fd -t f '^index\.(ts|tsx)$' packages/shared/src --max-depth 2
echo -e "\n=== Verify types/index.ts exists ==="
ls -la packages/shared/src/types/index.ts 2>&1
echo -e "\n=== Check for modules not covered by explicit entries ==="
fd -e ts -e tsx . packages/shared/src --max-depth 2 -x echo {} | grep -v test | grep -v specLength of output: 8576
Critical: Runtime build entry configuration is missing entire subdirectories (errors, router, telemetry).
The script reveals a significant gap in the entry configuration. While the current setup includes:
- Root-level files via
./src/*.{ts,tsx} - Specific index.ts entries for
react,utils,workerTimers,types - DOM files via
./src/dom/*.ts
It does not cover:
./src/errors/(6+ files including errorThrower.ts, clerkError.ts, etc.)./src/router/(3 files including router.ts, react.tsx)./src/telemetry/(3 files including collector.ts, throttler.ts)
These nested subdirectories contain actual source files that are not accessible through index entry points. They will be excluded from the runtime build bundle, likely breaking exports or creating missing dependencies.
Secondary concern: ./src/types/index.ts appears in both the types build and runtime build entries. Clarify whether this duplication is intentional or should be removed from one build.
Required actions:
- Add explicit entries for
./src/errors/*.ts,./src/router/*.ts,./src/telemetry/*.ts(or use./src/**/*.{ts,tsx}pattern with exclusions if comprehensive coverage is intended) - Confirm the types entry duplication is intentional; remove if not
🤖 Prompt for AI Agents
In packages/shared/tsdown.config.mts around lines 35 to 48, the runtime entry
list omits entire subdirectories (./src/errors, ./src/router, ./src/telemetry)
so those source files won't be included in the runtime bundle; update the entry
configuration to include either explicit globs './src/errors/*.ts',
'./src/router/*.ts', './src/telemetry/*.ts' (or replace the current entries with
a broader pattern like './src/**/*.{ts,tsx}' plus the existing exclusions) so
all nested files are picked up, and also review the duplicate
'./src/types/index.ts' entry—confirm if it should remain in both runtime and
types builds or remove it from one to avoid redundancy.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/shared/src/pathToRegexp.ts (1)
3-85: Consider reordering type declarations to improve readability.While TypeScript handles forward references through hoisting, several types are referenced before they're declared:
Key(referenced at line 17, declared at lines 45-51)TokensToRegexpOptions(referenced at line 40, declared at lines 52-81)Path(referenced at line 39, declared at lines 82-85)Declaring types before they're referenced would improve code readability and make the file easier to navigate.
Consider this ordering:
Keyinterface (currently lines 45-51)Pathtype (currently lines 82-85)ParseOptionsinterface (currently lines 3-12)RegexpToFunctionOptionsinterface (currently lines 13-18)TokensToRegexpOptionsinterface (currently lines 52-81)MatchResult,Match,MatchFunctiontypes (currently lines 19-34)
📜 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 (2)
packages/shared/src/compiled/path-to-regexp/index.d.ts(0 hunks)packages/shared/src/pathToRegexp.ts(2 hunks)
💤 Files with no reviewable changes (1)
- packages/shared/src/compiled/path-to-regexp/index.d.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/shared/src/pathToRegexp.ts
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/shared/src/pathToRegexp.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/shared/src/pathToRegexp.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/shared/src/pathToRegexp.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{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
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/shared/src/pathToRegexp.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/shared/src/pathToRegexp.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
Add pnpm overrides for React 18.3.1 to prevent version mismatches in the monorepo. The lockfile was resolving peer-react catalog to 19.2.0, causing tests to fail with "Invalid hook call" errors due to React/React DOM version mismatch between packages. The override ensures all workspace packages use React 18.3.1 during development, matching the catalog:react standard, while peer dependencies still support both React 18 and 19 for consumers.
nikosdouvlis
force-pushed
the
nikos/migrate-types-to-shared
branch
from
f2f69c5 to
8b82759
Compare
nikosdouvlis
force-pushed
the
nikos/migrate-types-to-shared
branch
from
f4e8b3f to
c169593
Compare
nikosdouvlis
force-pushed
the
nikos/migrate-types-to-shared
branch
from
c169593 to
bc83234
Compare
nikosdouvlis
force-pushed
the
nikos/migrate-types-to-shared
branch
from
bc83234 to
3c50200
Compare
nikosdouvlis
force-pushed
the
nikos/migrate-types-to-shared
branch
from
340ac89 to
b21da21
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
packages/clerk-js/src/ui/components/SignIn/SignInAlternativePhoneCodePhoneNumberCard.tsx (2)
13-18: Fix type name mismatch: "SignUp" vs "SignIn".The props type is named
SignUpAlternativePhoneCodePhoneNumberCardPropsbut the component isSignInAlternativePhoneCodePhoneNumberCard. This naming inconsistency could cause confusion.Apply this diff to fix the type name:
-type SignUpAlternativePhoneCodePhoneNumberCardProps = { +type SignInAlternativePhoneCodePhoneNumberCardProps = { handleSubmit: React.FormEventHandler; phoneNumberFormState: FormControlState<any>; onUseAnotherMethod: () => void; phoneCodeProvider: PhoneCodeChannelData; }; -export const SignInAlternativePhoneCodePhoneNumberCard = (props: SignUpAlternativePhoneCodePhoneNumberCardProps) => { +export const SignInAlternativePhoneCodePhoneNumberCard = (props: SignInAlternativePhoneCodePhoneNumberCardProps) => {
36-37: Add optional chaining for safe property access.Line 36 safely uses optional chaining (
providerToDisplayData[channel]?.iconUrl), but line 37 accessesstrategyToDisplayData[channel].namewithout optional chaining. This inconsistency could cause a runtime error ifchannelis not a valid key instrategyToDisplayData.Apply this diff to add defensive coding:
<Image src={providerToDisplayData[channel]?.iconUrl} - alt={`${strategyToDisplayData[channel].name} logo`} + alt={`${strategyToDisplayData[channel]?.name} logo`} sx={theme => ({packages/clerk-js/src/ui/components/SignIn/SignInFactorOnePasskey.tsx (1)
17-20: Props interface has naming inconsistency and unused prop—both require fixes.The interface
SignInFactorOnePasswordPropsis copy-pasted from the PasswordCard component but used bySignInFactorOnePasskey. Additionally, theonFactorPrepareprop is defined but never called or referenced anywhere in the component implementation.Fixes needed:
- Rename interface to
SignInFactorOnePasskeyPropsto match the component name- Remove the unused
onFactorPrepareprop from the interface (lines 19)- Remove the unused
ResetPasswordCodeFactorimport (line 1)packages/clerk-js/src/ui/components/PaymentMethods/PaymentMethods.tsx (1)
111-116: Guard against undefined data, avoid in-place mutation, and make sort stable.
paymentMethods.sort(...)mutates source and may throw ifdatais undefined. Use a default empty array, copy before sorting, and return0when equal.- const { data: paymentMethods, isLoading, revalidate: revalidatePaymentMethods } = usePaymentMethods(); + const { data: paymentMethods = [], isLoading, revalidate: revalidatePaymentMethods } = usePaymentMethods(); - const sortedPaymentMethods = useMemo( - () => paymentMethods.sort((a, b) => (a.isDefault && !b.isDefault ? -1 : 1)), - [paymentMethods], - ); + const sortedPaymentMethods = useMemo( + () => + [...paymentMethods].sort((a, b) => + a.isDefault === b.isDefault ? 0 : a.isDefault ? -1 : 1, + ), + [paymentMethods], + );packages/clerk-js/src/core/resources/Organization.ts (1)
1-27: Missing import: OrganizationPublicMetadata is used but not imported.
publicMetadata: OrganizationPublicMetadata = {}(Line 45) references a type that isn’t imported. Add it from@clerk/shared/types.import type { AddMemberParams, ClerkPaginatedResponse, ClerkResourceReloadParams, CreateOrganizationParams, GetDomainsParams, GetInvitationsParams, GetMembershipRequestParams, GetMemberships, GetRolesParams, InviteMemberParams, InviteMembersParams, + OrganizationPublicMetadata, OrganizationDomainJSON, OrganizationDomainResource, OrganizationInvitationJSON, OrganizationInvitationResource, OrganizationJSON, OrganizationJSONSnapshot, OrganizationMembershipJSON, OrganizationMembershipRequestJSON, OrganizationMembershipRequestResource, OrganizationResource, RoleJSON, SetOrganizationLogoParams, UpdateMembershipParams, UpdateOrganizationParams, } from '@clerk/shared/types';packages/clerk-js/src/ui/components/OrganizationProfile/VerifiedDomainForm.tsx (2)
41-49: Fix suggestion count — wrong variable used.
calloutSuggestionCountLabelshould usetotalSuggestions, nottotalInvitations.- localizationKeys(`organizationProfile.verifiedDomainPage.enrollmentTab.calloutSuggestionCountLabel`, { - count: totalInvitations, - }), + localizationKeys(`organizationProfile.verifiedDomainPage.enrollmentTab.calloutSuggestionCountLabel`, { + count: totalSuggestions, + }),
126-131: Avoid side-effects during render; moveonResetbehind an effect and short-circuit render.
CallingonReset()in render can loop or cause double-invocations.useEffect(() => { if (domain) { enrollmentMode.setValue(domain.enrollmentMode); } }, [domain?.id]); + + // Reset when domain becomes unverified + useEffect(() => { + if (domain && domain.verification?.status !== 'verified') { + onReset?.(); + } + }, [domain?.verification?.status]); @@ - if (!(domain.verification && domain.verification.status === 'verified')) { - onReset(); - } + if (!(domain.verification && domain.verification.status === 'verified')) { + return null; + }Also make callback usage defensive:
- onSuccess(); + onSuccess?.();Also applies to: 185-189
packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts (1)
1-8: Add missingOrganizationInvitationPublicMetadatato type imports.The type-only import path migration looks correct, and no lingering
@clerk/typesimports remain in the codebase. However, the code at line 15 usesOrganizationInvitationPublicMetadatawhich is not included in the import statement. Add this type to the import list:import type { ClerkPaginatedResponse, GetUserOrganizationInvitationsParams, OrganizationCustomRoleKey, OrganizationInvitationPublicMetadata, OrganizationInvitationStatus, UserOrganizationInvitationJSON, UserOrganizationInvitationResource, } from '@clerk/shared/types';
♻️ Duplicate comments (2)
package.json (1)
138-142: Build tooling migration remains incomplete.The previous review correctly identified that only 1 of ~20 packages has migrated to tsdown/rolldown. This concern remains valid.
packages/clerk-js/src/core/resources/Web3Wallet.ts (1)
1-8: Type import migration LGTM.
import type { ... } from '@clerk/shared/types'matches the migration goal and keeps bundlers clean. Ensure@clerk/sharedexports the./typessubpath (see script in test file comment).
🧹 Nitpick comments (22)
packages/clerk-js/src/ui/components/SignIn/SignInAlternativePhoneCodePhoneNumberCard.tsx (1)
15-15: Replaceanytype with proper generic constraint.The
phoneNumberFormStateusesFormControlState<any>, which violates the coding guideline to avoidanytypes. Consider using a more specific type orunknownwith type guards.As per coding guidelines.
packages/clerk-js/src/core/resources/Waitlist.ts (2)
34-34: Removeas anytype assertion.The
body: params as anycast violates the coding guideline: "Noanytypes without justification." If there's a type mismatch betweenJoinWaitlistParamsand the expected body type, address it by either adjusting the types or usingunknownwith proper type narrowing.Consider this approach:
- body: params as any, + body: params,If type errors occur, verify that
BaseResource._fetchaccepts the correct body type or adjustJoinWaitlistParamsaccordingly.
36-36: Consider refining the double type assertion.The pattern
?.response as unknown as WaitlistJSONsuggests potential type friction. While double assertions are sometimes necessary, verify whether theBaseResource._fetchreturn type can be improved to eliminate this cast.If
BaseResource._fetch<WaitlistJSON>already returns the correct type structure, the double assertion may be unnecessary. Otherwise, consider adding a type guard or adjusting the return type of_fetch.packages/clerk-js/src/core/modules/billing/payment-source-methods.ts (3)
16-26: Add explicit return type and avoidanytype assertion.This exported function lacks an explicit return type annotation and uses an
anytype assertion on line 22, both of which violate the coding guidelines.Apply this diff to add the return type and improve type safety:
-export const initializePaymentMethod = async (params: InitializePaymentMethodParams) => { +export const initializePaymentMethod = async ( + params: InitializePaymentMethodParams +): Promise<BillingInitializedPaymentMethod> => { const { orgId, ...rest } = params; const json = ( await BaseResource._fetch({ path: Billing.path(`${PAYMENT_METHODS_PATH}/initialize`, { orgId }), method: 'POST', - body: rest as any, + body: rest as unknown, }) )?.response as unknown as BillingInitializedPaymentMethodJSON; return new BillingInitializedPaymentMethod(json); };As per coding guidelines: "Always define explicit return types for functions, especially public APIs" and "Avoid
anytype - preferunknownwhen type is uncertain."
28-39: Add explicit return type and avoidanytype assertion.This exported function lacks an explicit return type annotation and uses an
anytype assertion on line 35, both of which violate the coding guidelines.Apply this diff to add the return type and improve type safety:
-export const addPaymentMethod = async (params: AddPaymentMethodParams) => { +export const addPaymentMethod = async (params: AddPaymentMethodParams): Promise<BillingPaymentMethod> => { const { orgId, ...rest } = params; const json = ( await BaseResource._fetch({ path: Billing.path(PAYMENT_METHODS_PATH, { orgId }), method: 'POST', - body: rest as any, + body: rest as unknown, }) )?.response as unknown as BillingPaymentMethodJSON; return new BillingPaymentMethod(json); };As per coding guidelines: "Always define explicit return types for functions, especially public APIs" and "Avoid
anytype - preferunknownwhen type is uncertain."
41-55: Add explicit return type annotation.This exported function lacks an explicit return type annotation, which violates the coding guidelines for public APIs.
Apply this diff to add the return type:
-export const getPaymentMethods = async (params: GetPaymentMethodsParams) => { +export const getPaymentMethods = async ( + params: GetPaymentMethodsParams +): Promise<{ total_count: number; data: BillingPaymentMethod[] }> => { const { orgId, ...rest } = params; return await BaseResource._fetch({As per coding guidelines: "Always define explicit return types for functions, especially public APIs."
packages/clerk-js/src/ui/components/SubscriptionDetails/index.tsx (1)
109-110: Consider type guards instead ofas anycasts.The
as anycasts bypass TypeScript's type safety. While the logic is correct (enforced by the runtime throw at line 102-104), the conditional return type can be properly narrowed using type predicates or assertion functions instead.Consider refactoring to use a type assertion function:
function useGuessableSubscription<Or extends 'throw' | undefined = undefined>(options?: { or?: Or; }): UseGuessableSubscriptionResult<Or> { const { subscriptionItems, isLoading } = useSubscription(); const activeSubscription = subscriptionItems?.find(sub => sub.status === 'active'); const upcomingSubscription = subscriptionItems?.find(sub => sub.status === 'upcoming'); const pastDueSubscription = subscriptionItems?.find(sub => sub.status === 'past_due'); if (options?.or === 'throw' && !activeSubscription && !pastDueSubscription) { throw new Error('No active or past due subscription found'); } + const anySubscription = upcomingSubscription || activeSubscription || pastDueSubscription; + + if (options?.or === 'throw') { + // TypeScript now knows activeSubscription is defined due to the throw above + return { + upcomingSubscription, + pastDueSubscription, + activeSubscription: activeSubscription!, + anySubscription: anySubscription!, + isLoading, + } as UseGuessableSubscriptionResult<Or>; + } + return { upcomingSubscription, pastDueSubscription, - activeSubscription: activeSubscription as any, - anySubscription: (upcomingSubscription || activeSubscription || pastDueSubscription) as any, + activeSubscription, + anySubscription, isLoading, - }; + } as UseGuessableSubscriptionResult<Or>; }Alternatively, split into two separate functions for better type safety.
packages/clerk-js/src/core/fapiClient.ts (1)
45-45: Update or remove outdated TODO comment.This TODO suggests moving
FapiResponseJSONto@clerk/types, but the PR is migrating types from@clerk/typesto@clerk/shared/types. Consider updating this comment to reference@clerk/shared/typesinstead, or remove it if this interface should remain local to this file.packages/clerk-js/src/ui/components/Subscriptions/badge.tsx (1)
20-40: Consider addressing the@ts-expect-errorsuppressions.The component has two
@ts-expect-errorcomments indicating that the'ended'status is possible but not handled in thekeysandcolorsobjects. Ifsubscription.statusis'ended', accessingcolors[subscription.status]andkeys[subscription.status]would returnundefined, potentially causing runtime issues.Consider adding the
'ended'status to both objects:const keys = { active: 'badge__activePlan', upcoming: 'badge__upcomingPlan', past_due: 'badge__pastDuePlan', free_trial: 'badge__freeTrial', + ended: 'badge__endedPlan', }; const colors = { active: 'secondary', upcoming: 'primary', past_due: 'warning', free_trial: 'secondary', + ended: 'neutral', };Then remove the
@ts-expect-errorcomments. This would make the code more robust and type-safe.packages/clerk-js/src/core/modules/billing/namespace.ts (1)
126-137: Consider removing theas anytype assertion.Line 132 uses
as anyto bypass type checking for the request body. This weakens type safety and could hide mismatches betweenCreateCheckoutParamsand the expected API payload.If
BaseResource._fetchaccepts a properly typed body parameter, remove the type assertion:const json = ( await BaseResource._fetch<BillingCheckoutJSON>({ path: Billing.path(`/checkouts`, { orgId }), method: 'POST', - body: rest as any, + body: rest, }) )?.response as unknown as BillingCheckoutJSON;If the fetch method requires additional type adjustments, consider defining an explicit type for the body that matches the API contract.
packages/clerk-js/src/ui/components/PaymentMethods/PaymentMethodRow.tsx (1)
24-26: Localize the payment method label (remove TODO).Avoid rendering raw values; map
paymentMethod.paymentTypeandpaymentMethod.cardTypeto localization keys (e.g., payment brand names) and render via your localization utilities.packages/clerk-js/src/ui/components/SignIn/SignInFactorOneForgotPasswordCard.tsx (1)
12-24: Add an explicit return type for the exported component.Keeps public APIs explicit per guidelines.
-export const SignInFactorOneForgotPasswordCard = (props: SignInForgotPasswordCardProps) => { +export const SignInFactorOneForgotPasswordCard = (props: SignInForgotPasswordCardProps): JSX.Element => {packages/clerk-js/src/core/signals.ts (1)
69-86: Defensive guard when mapping API error field names.If
paramNamemaps to an unknown key after the types migration, we currently create an extraneous property onfields. Guard and fallback toglobal.- if ('meta' in error && error.meta && 'paramName' in error.meta) { - const name = snakeToCamel(error.meta.paramName); - parsedErrors.fields[name as keyof typeof parsedErrors.fields] = error; - return; - } + if ('meta' in error && error.meta && 'paramName' in error.meta) { + const key = snakeToCamel(error.meta.paramName) as string; + if (key in parsedErrors.fields) { + parsedErrors.fields[key as keyof typeof parsedErrors.fields] = error; + } else { + parsedErrors.global = parsedErrors.global ? [...parsedErrors.global, error] : [error]; + } + return; + }packages/clerk-js/src/core/resources/Organization.ts (1)
53-66: UseJSON.stringify()for request bodies instead ofas anycasts to maintain type safety.The
as anypattern on lines 63, 206, and 222 bypasses TypeScript's type system. WhilefapiClienthandles object bodies at runtime, the type system treatsFapiRequestInit.bodyasBodyInit(string/Blob), creating a gap. Explicit serialization withJSON.stringify()keeps types precise and aligns with the pattern used inpackages/clerk-js/src/core/modules/apiKeys/index.ts:body: JSON.stringify({ userId, role })This applies to all three occurrences: line 63 (
CreateOrganizationParams), line 206 ({ userId, role }), and line 222 ({ role }).packages/clerk-js/src/core/resources/UserOrganizationInvitation.ts (1)
18-18: ImportOrganizationInvitationPublicMetadataexplicitly.
Avoid relying on ambient/global types; import it from shared types for clarity and stability.import type { ClerkPaginatedResponse, GetUserOrganizationInvitationsParams, OrganizationCustomRoleKey, OrganizationInvitationStatus, + OrganizationInvitationPublicMetadata, UserOrganizationInvitationJSON, UserOrganizationInvitationResource, } from '@clerk/shared/types';packages/clerk-js/src/ui/components/OrganizationProfile/VerifiedDomainForm.tsx (3)
25-50: Return proper types fromuseCalloutLabel(LocalizationKey[]).
Avoidstring[]cast; it can mask type issues withlocalizationKeyprop.-const useCalloutLabel = ( - domain: OrganizationDomainResource | undefined | null, - { - infoLabel: infoLabelKey, - }: { - infoLabel: LocalizationKey; - }, -) => { +const useCalloutLabel = ( + domain: OrganizationDomainResource | undefined | null, + { + infoLabel: infoLabelKey, + }: { + infoLabel: LocalizationKey; + }, +): LocalizationKey[] => { const totalInvitations = domain?.totalPendingInvitations || 0; const totalSuggestions = domain?.totalPendingSuggestions || 0; const totalPending = totalSuggestions + totalInvitations; if (totalPending === 0) { - return [] as string[]; + return [] as LocalizationKey[]; } return [ infoLabelKey, localizationKeys(`organizationProfile.verifiedDomainPage.enrollmentTab.calloutInvitationCountLabel`, { count: totalInvitations, }), localizationKeys(`organizationProfile.verifiedDomainPage.enrollmentTab.calloutSuggestionCountLabel`, { - count: totalInvitations, + count: totalSuggestions, }), ]; };
52-85: Strongly type enrollment options toOrganizationEnrollmentMode.
Prevents drift and removes downstream casts.-const buildEnrollmentOptions = (settings: OrganizationSettingsResource) => { - const _options = []; +const buildEnrollmentOptions = ( + settings: OrganizationSettingsResource, +): Array<{ value: OrganizationEnrollmentMode; label: LocalizationKey; description: LocalizationKey }> => { + const _options: Array<{ value: OrganizationEnrollmentMode; label: LocalizationKey; description: LocalizationKey }> = + []; if (settings.domains.enrollmentModes.includes('manual_invitation')) { _options.push({ value: 'manual_invitation', label: localizationKeys('organizationProfile.verifiedDomainPage.enrollmentTab.manualInvitationOption__label'), description: localizationKeys( 'organizationProfile.verifiedDomainPage.enrollmentTab.manualInvitationOption__description', ), }); } @@ return _options; };Optional follow-up: make
useFormControlgeneric soenrollmentMode.valueis typed and you can drop the cast at Line 150.
148-152: Drop the cast by typingenrollmentMode.
IfuseFormControlsupports generics, preferuseFormControl<OrganizationEnrollmentMode>(...)soenrollmentMode.valueis properly typed.- enrollmentMode: enrollmentMode.value as OrganizationEnrollmentMode, + enrollmentMode: enrollmentMode.value,packages/clerk-js/src/ui/components/OrganizationProfile/InviteMembersForm.tsx (1)
96-99: Sanitize email list before submit and when pruning invalids.
Trim and drop empties to reduce server-side errors.- .inviteMembers({ - emailAddresses: emailAddressField.value.split(','), + .inviteMembers({ + emailAddresses: emailAddressField.value + .split(',') + .map(e => e.trim()) + .filter(Boolean), role: submittedData.get('role') as string, })And in
removeInvalidEmails:- const emails = emailAddressField.value.split(','); - emailAddressField.setValue(emails.filter(e => !invalidEmails.has(e)).join(',')); + const emails = emailAddressField.value.split(',').map(e => e.trim()).filter(Boolean); + emailAddressField.setValue(emails.filter(e => !invalidEmails.has(e)).join(','));Also applies to: 155-159
packages/clerk-js/src/core/resources/__tests__/Web3Wallet.test.ts (2)
8-12: Prefersatisfies(or a narrower shape) overasin tests.Using
as Web3WalletJSONcan mask missing fields. Consider:-const web3WalletJSON = { - object: 'web3_wallet', - web3_wallet: '0x0000000000000000000000000000000000000000', -} as Web3WalletJSON; +const web3WalletJSON = { + object: 'web3_wallet', + web3_wallet: '0x0000000000000000000000000000000000000000', +} as const satisfies Partial<Web3WalletJSON>;This keeps tests honest while allowing minimal shapes.
13-15: Avoid@ts-ignoreon private internals in tests.Instead of overriding a private
_fetch, consider injecting an HTTP client or exposing a minimal test seam (e.g.,BaseResource.__test_setFetch(fn)guarded for tests). Keeps type safety and avoids ignoring errors.Also applies to: 19-21, 84-86, 90-94
packages/clerk-js/src/core/resources/Web3Wallet.ts (1)
17-19: Constructor overload vs implementation mismatch can cause unsoundness.You expose an overload accepting
Partial<...>, but the implementation requires fullWeb3WalletJSON | Web3WalletJSONSnapshot. Callers may pass partials that compile yet result inundefinedfields at runtime (e.g.,new Verification(data.verification)).Two safe options:
- Simplify: remove the
Partialoverload if not needed.- Or support partials: widen the implementation and guard in
fromJSON.Apply one of these diffs:
Option A — remove
Partialoverload- public constructor(data: Partial<Web3WalletJSON | Web3WalletJSONSnapshot>, pathRoot: string); + // Accept only complete JSON or snapshot to avoid runtime undefineds. + public constructor(data: Web3WalletJSON | Web3WalletJSONSnapshot, pathRoot: string);Option B — support partials end-to-end
- public constructor(data: Partial<Web3WalletJSON | Web3WalletJSONSnapshot>, pathRoot: string); - public constructor(data: Web3WalletJSON | Web3WalletJSONSnapshot, pathRoot: string) { + public constructor(data: Partial<Web3WalletJSON | Web3WalletJSONSnapshot>, pathRoot: string) { super(); this.pathRoot = pathRoot; this.fromJSON(data); } @@ - protected fromJSON(data: Web3WalletJSON | Web3WalletJSONSnapshot | null): this { + protected fromJSON(data: Partial<Web3WalletJSON | Web3WalletJSONSnapshot> | null): this { if (!data) { return this; } - - this.id = data.id; - this.web3Wallet = data.web3_wallet; - this.verification = new Verification(data.verification); + if ('id' in data && data.id !== undefined) { + this.id = data.id as string; + } + if ('web3_wallet' in data && data.web3_wallet !== undefined) { + this.web3Wallet = data.web3_wallet as string; + } + if ('verification' in data && data.verification !== undefined) { + this.verification = new Verification(data.verification as any); + } return this; }Also applies to: 53-61
| @@ -1,7 +1,7 @@ | |||
| import { isBrowserOnline } from '@clerk/shared/browser'; | |||
| import { retry } from '@clerk/shared/retry'; | |||
| import type { ClerkAPIErrorJSON, ClientJSON, InstanceType } from '@clerk/shared/types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Type import migration looks correct.
The migration from @clerk/types to @clerk/shared/types is properly implemented using a type-only import, which aligns with TypeScript best practices and the project's coding guidelines.
Please verify that ClerkAPIErrorJSON, ClientJSON, and InstanceType are properly exported from @clerk/shared/types:
🏁 Script executed:
#!/bin/bash
# Description: Verify that the imported types are exported from @clerk/shared/types
# Search for type exports in the shared/types package
fd -e ts -e tsx . packages/shared | xargs rg -n "export (type|interface) (ClerkAPIErrorJSON|ClientJSON|InstanceType)" -C2Length of output: 1148
🏁 Script executed:
#!/bin/bash
# Verify types are re-exported from @clerk/shared/types package entry point
rg -n "export.*ClerkAPIErrorJSON|export.*ClientJSON|export.*InstanceType" packages/shared/src/types/index.ts packages/shared/src/index.ts packages/shared/package.json -C2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the TODO comment on line 45 of fapiClient.ts for context
rg -n "TODO.*@clerk/types" packages/clerk-js/src/core/fapiClient.ts -C3Length of output: 233
🏁 Script executed:
#!/bin/bash
# Verify the actual imports in fapiClient.ts are using the new path
rg -n "^import.*from.*@clerk" packages/clerk-js/src/core/fapiClient.tsLength of output: 254
🏁 Script executed:
#!/bin/bash
# Check what is actually exported from @clerk/shared/types index file
cat packages/shared/src/types/index.ts | head -50Length of output: 1807
🏁 Script executed:
#!/bin/bash
# Verify the package.json exports or tsconfig paths for @clerk/shared/types
rg -n "types|exports" packages/shared/package.json -C2Length of output: 2492
Update TODO comment to reflect the migration direction.
The import migration to @clerk/shared/types is correctly implemented, and the types are properly exported from the package. However, the TODO comment on line 45 ("// TODO: Move to @clerk/types") contradicts the migration direction and should be removed or updated, as this code is now using the target location.
Line 45:
// TODO: Move to @clerk/types
Should be removed since the types are already migrated to @clerk/shared/types.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/fapiClient.ts around line 45, remove the outdated
"// TODO: Move to @clerk/types" comment (or replace it with a short note
reflecting that types have been migrated to @clerk/shared/types) so the comment
no longer contradicts the current import from @clerk/shared/types.
| @@ -1,4 +1,4 @@ | |||
| import type { MembershipRole } from '@clerk/types'; | |||
| import type { MembershipRole } from '@clerk/shared/types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Import path correctly updated for the types migration.
The import statement has been properly updated to use @clerk/shared/types as part of the broader types consolidation effort. The type-only import syntax is correctly used, following TypeScript best practices.
Run the following script to verify that MembershipRole is correctly exported from the new package and the tests pass:
🏁 Script executed:
#!/bin/bash
# Description: Verify MembershipRole export and test execution
# Check that MembershipRole is exported from @clerk/shared/types
echo "Checking @clerk/shared/types exports..."
fd -t f "index.*ts$" packages/shared | head -1 | xargs cat | grep -A 5 "MembershipRole"
# Verify the type definition exists
echo -e "\nSearching for MembershipRole type definition..."
rg -n -C3 "type MembershipRole|interface MembershipRole" packages/shared
# Run the specific test file to ensure it passes
echo -e "\nRunning OrganizationSwitcher tests..."
npm run test -- OrganizationSwitcher.test.tsxLength of output: 281
🏁 Script executed:
#!/bin/bash
# Find all files that export or define MembershipRole
echo "=== Searching for MembershipRole type definition ==="
rg -n "MembershipRole" packages/shared --type ts --type tsx
# Check if packages/shared exists and its structure
echo -e "\n=== Packages/shared directory structure ==="
fd -t d "^[^.]" packages/shared -d 1
# Look for index or types exports in packages/shared
echo -e "\n=== Checking packages/shared type exports ==="
fd -t f "\.(ts|d\.ts)$" packages/shared -x basename {} \;
# Check packages/@clerk directory structure to understand org
echo -e "\n=== Checking @clerk packages ==="
fd -t d "@clerk" packages -d 1
# Directly check if MembershipRole exists in any types file
echo -e "\n=== Searching for MembershipRole in all type files ==="
fd -t f "\.(ts|tsx|d\.ts)$" packages -x rg -l "type MembershipRole|interface MembershipRole|export.*MembershipRole" {}Length of output: 3820
🏁 Script executed:
#!/bin/bash
# Check the relevant files in packages/shared that might contain MembershipRole
echo "=== Checking packages/shared/src/role.ts ==="
cat packages/shared/src/role.ts | head -50
echo -e "\n=== Checking packages/shared/src/organizationMembership.ts ==="
cat packages/shared/src/organizationMembership.ts | head -50
echo -e "\n=== Checking packages/shared/src/types.ts ==="
cat packages/shared/src/types.ts | head -50
# Check the main index file
echo -e "\n=== Checking packages/shared/src/index.ts for MembershipRole export ==="
cat packages/shared/src/index.ts | grep -A 2 -B 2 "MembershipRole"
# Look at the test file that was changed
echo -e "\n=== Checking the actual test file context ==="
head -20 packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx
# Check if @clerk/types package exists
echo -e "\n=== Checking for @clerk/types package ==="
find packages -name "types" -type d | head -10
# Check what the test file imports currently
echo -e "\n=== All imports in OrganizationSwitcher.test.tsx ==="
rg "^import" packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx -nLength of output: 1735
🏁 Script executed:
#!/bin/bash
# Check packages/types structure
echo "=== Checking packages/types directory ==="
ls -la packages/types/src/ 2>/dev/null | head -20
# Check packages/shared/src/types structure
echo -e "\n=== Checking packages/shared/src/types directory ==="
ls -la packages/shared/src/types/ 2>/dev/null | head -20
# Search for MembershipRole in packages/types
echo -e "\n=== Searching MembershipRole in packages/types ==="
rg -n "MembershipRole" packages/types --type ts -A 2
# Search for MembershipRole in packages/shared/src/types
echo -e "\n=== Searching MembershipRole in packages/shared/src/types ==="
rg -n "MembershipRole" packages/shared/src/types --type ts -A 2
# Check package.json files to understand package names
echo -e "\n=== Checking packages/shared/package.json ==="
cat packages/shared/package.json | grep -A 5 '"name"'
echo -e "\n=== Checking packages/types/package.json ==="
cat packages/types/package.json | grep -A 5 '"name"'
# Check if there's a types index that re-exports from @clerk/shared
echo -e "\n=== Checking packages/shared/src/index.ts exports ==="
cat packages/shared/src/index.ts | tail -50Length of output: 3603
🏁 Script executed:
#!/bin/bash
# Find where packages/types source code actually is
echo "=== Looking for packages/types source files ==="
find packages/types -name "*.ts" -not -path "*/node_modules/*" 2>/dev/null | head -20
# Search for all MembershipRole definitions in the entire packages directory
echo -e "\n=== Finding all MembershipRole type definitions ==="
rg -n "type MembershipRole\s*=|interface MembershipRole\s*{" packages --type ts
# Look at the @clerk/types index file to see what it exports
echo -e "\n=== Checking packages/types/index.d.ts ==="
cat packages/types/src/index.d.ts
# Check if there's a packages/types with actual TypeScript source
echo -e "\n=== Checking package.json for @clerk/types export field ==="
cat packages/types/package.json | grep -A 20 '"exports"'
# Look for tsconfig to understand path mappings
echo -e "\n=== Checking tsconfig paths ==="
cat tsconfig.json 2>/dev/null | grep -A 30 '"paths"' | head -40
# Check if @clerk/shared/types is a subpath export
echo -e "\n=== Checking if @clerk/shared has types subpath ==="
cat packages/shared/package.json | grep -A 20 '"exports"'Length of output: 1769
🏁 Script executed:
#!/bin/bash
# Check if dist folder exists and has types file
echo "=== Checking packages/shared/dist structure ==="
find packages/shared/dist -name "types*" -type f 2>/dev/null | head -10
# Search MembershipRole with fixed regex
echo -e "\n=== Finding MembershipRole type definition ==="
rg -n "MembershipRole" packages/shared/src/types --type ts -A 3
# List all files in packages/shared/src/types
echo -e "\n=== All files in packages/shared/src/types ==="
ls -1 packages/shared/src/types/ | grep -E "^[a-z]"
# Check organization-related type files for MembershipRole
echo -e "\n=== Checking organization.ts for MembershipRole ==="
rg -n "MembershipRole" packages/shared/src/types/organization.ts -C 2
# Check in packages/backend where we found it earlier
echo -e "\n=== Checking packages/backend Enums for MembershipRole ==="
rg -n "MembershipRole" packages/backend/src/api/resources/Enums.ts -A 2 | head -20
# Search all of packages/shared/src for MembershipRole
echo -e "\n=== Searching all packages/shared/src for MembershipRole ==="
rg -n "MembershipRole" packages/shared/src -C 2Length of output: 1682
🏁 Script executed:
#!/bin/bash
# Check what's exported from packages/shared/src/types/index.ts
echo "=== Checking packages/shared/src/types/index.ts ==="
cat packages/shared/src/types/index.ts | head -100
# Search entire packages directory for MembershipRole definition
echo -e "\n=== Searching entire packages for MembershipRole ==="
rg "export.*MembershipRole|type MembershipRole|MembershipRole =" packages -n --max-count 5
# Check role.ts file since it's role-related
echo -e "\n=== Checking packages/shared/src/types/role.ts ==="
cat packages/shared/src/types/role.ts
# Check organizationMembership.ts
echo -e "\n=== Checking packages/shared/src/types/organizationMembership.ts ==="
cat packages/shared/src/types/organizationMembership.ts
# Look in backend packages for reference type definition
echo -e "\n=== Checking if MembershipRole might be in backend types ==="
find packages/backend/src -name "*.ts" -exec rg -l "type MembershipRole|export.*MembershipRole" {} \;Length of output: 7807
🏁 Script executed:
#!/bin/bash
# Check if MembershipRole exists in packages/clerk-js
echo "=== Searching packages/clerk-js for MembershipRole ==="
rg "MembershipRole" packages/clerk-js --type ts --type tsx -n -C 2
# Check the exact line 212 usage in the test file
echo -e "\n=== Checking line 212 context in test file ==="
sed -n '205,220p' packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx
# Check what was the original import if this is a recent change
echo -e "\n=== Checking git history for the import change ==="
cd packages/clerk-js && git log --oneline -n 20 -- src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx 2>/dev/null || echo "Git history not available"
# Search for where MembershipRole might have been defined
echo -e "\n=== Final comprehensive search for MembershipRole ==="
rg -n "^export.*type MembershipRole|^type MembershipRole|^interface MembershipRole" packages --type tsLength of output: 1556
🏁 Script executed:
#!/bin/bash
cd /tmp/repo
# Check git diff to see what changed in the import
echo "=== Git diff for the import change ==="
git diff HEAD~1..HEAD packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx 2>/dev/null || echo "Unable to get git diff"
# Check @clerk/types package before migration
echo -e "\n=== Checking @clerk/types index for MembershipRole ==="
rg "MembershipRole" packages/types --type ts -n
# Check if @clerk/shared/types actually exports MembershipRole by looking at index
echo -e "\n=== Checking @clerk/shared/types/index.ts re-export chain ==="
cat packages/shared/src/types/index.ts | grep -i "membership"
# Look for MembershipRole in any compiled output or type definition files
echo -e "\n=== Checking for any .d.ts files with MembershipRole ==="
find packages -name "*.d.ts" -exec grep -l "MembershipRole" {} \; 2>/dev/null | head -5
# Search entire codebase for where MembershipRole is actually defined
echo -e "\n=== Final search - where is MembershipRole defined or exported ==="
grep -r "export.*type MembershipRole\|export.*interface MembershipRole" packages --include="*.ts" --include="*.d.ts" 2>/dev/null || echo "MembershipRole not found in any export"Length of output: 483
Fix broken import: MembershipRole type does not exist in @clerk/shared/types.
The import references a type that isn't exported from the target package. packages/shared/src/types/organizationMembership.ts exports OrganizationCustomRoleKey, not MembershipRole.
Either add MembershipRole to @clerk/shared/types exports or replace the import with the existing OrganizationCustomRoleKey type used for organization membership roles.
🤖 Prompt for AI Agents
packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx
around line 1: the test imports a non-existent type MembershipRole from
@clerk/shared/types; replace that import with the correct exported type
OrganizationCustomRoleKey (or update the import path to where
OrganizationCustomRoleKey is exported) so the test references the actual
organization membership role type.
| @@ -1,4 +1,4 @@ | |||
| import type { PhoneCodeFactor } from '@clerk/types'; | |||
| import type { PhoneCodeFactor } from '@clerk/shared/types'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Verify type export and migration consistency.
The import path update correctly follows the type-only import syntax and aligns with the PR's migration objective.
Run the following script to verify that PhoneCodeFactor is properly exported from the new location and that all similar imports have been consistently updated:
🏁 Script executed:
#!/bin/bash
# Verify PhoneCodeFactor is exported from @clerk/shared/types
ast-grep --pattern $'export { $$$PhoneCodeFactor$$$ }'
# Verify no remaining imports of PhoneCodeFactor from old @clerk/types location
rg -n --type=ts --type=tsx "from ['\"]@clerk/types['\"]" -C2 | rg -i "PhoneCodeFactor"
# Check all imports of PhoneCodeFactor to ensure consistency
rg -n --type=ts --type=tsx "PhoneCodeFactor" -C1 | rg "from ['\"]@clerk/"Length of output: 55
🏁 Script executed:
#!/bin/bash
# Corrected script to verify PhoneCodeFactor migration
# 1. Check if PhoneCodeFactor is exported from @clerk/shared/types
echo "=== Checking for PhoneCodeFactor export in @clerk/shared/types ==="
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "export.*PhoneCodeFactor" | grep -E "clerk/shared" | head -20
# 2. Check the actual import in the file under review
echo -e "\n=== Current import in SignInFactorTwoPhoneCodeCard.tsx ==="
head -10 packages/clerk-js/src/ui/components/SignIn/SignInFactorTwoPhoneCodeCard.tsx
# 3. Search for all PhoneCodeFactor imports to check consistency
echo -e "\n=== All PhoneCodeFactor imports ==="
grep -rn "PhoneCodeFactor" --include="*.ts" --include="*.tsx" | grep "from" | sort | uniq
# 4. Check for any remaining imports from old @clerk/types location that reference PhoneCodeFactor or related types
echo -e "\n=== Checking for imports from @clerk/types (old location) ==="
grep -rn "from ['\"]@clerk/types['\"]" --include="*.ts" --include="*.tsx" | head -30Length of output: 5632
Update import in testing package helpers-utils.ts for migration consistency.
The import change in the file under review is correct and follows proper TypeScript practices. However, verification revealed an inconsistency: packages/testing/src/common/helpers-utils.ts still imports PhoneCodeFactor from the old @clerk/types location, while all other files have been migrated to @clerk/shared/types.
Update packages/testing/src/common/helpers-utils.ts:1 to import from @clerk/shared/types alongside the other migrated files:
import type { EmailCodeFactor, PhoneCodeFactor, SignInFirstFactor } from '@clerk/shared/types';🤖 Prompt for AI Agents
In packages/testing/src/common/helpers-utils.ts around line 1, the file still
imports PhoneCodeFactor (and possibly other types) from the old @clerk/types
package; update the import to use @clerk/shared/types instead (for example,
import EmailCodeFactor, PhoneCodeFactor, SignInFirstFactor from
'@clerk/shared/types') so the testing helpers match the migrated imports used
across the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
integration/templates/expo-web/metro.config.js (1)
2-2: Fix typo in comment."KNWO" should be "KNOW".
- * DO NOT EDIT THIS FILE UNLESS YOU DEFINITELY KNWO WHAT YOU ARE DOING. + * DO NOT EDIT THIS FILE UNLESS YOU DEFINITELY KNOW WHAT YOU ARE DOING.
📜 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)
integration/templates/expo-web/metro.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
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
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
integration/templates/expo-web/metro.config.js
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
integration/templates/expo-web/metro.config.js
integration/**
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
Framework integration templates and E2E tests should be placed under the integration/ directory
Files:
integration/templates/expo-web/metro.config.js
integration/**/*
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
End-to-end tests and integration templates must be located in the 'integration/' directory.
Files:
integration/templates/expo-web/metro.config.js
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
integration/templates/expo-web/metro.config.js
⏰ 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). (27)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
integration/templates/expo-web/metro.config.js (1)
22-24: LGTM!The addition of "link:" prefix handling is consistent with existing patterns and correctly implements support for npm/yarn/pnpm link syntax. The logic follows the same structure as the "file:" handler and integrates well with the existing control flow.
3 participants
Description
WIP
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores