-
Notifications
You must be signed in to change notification settings - Fork 407
feat(shared,types): Migrate @clerk/types to @clerk/shared/types #7022
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
c29586b
15551ec
c98b464
083ef3d
31dd221
4e9cdf0
73e31d0
a5e40af
053c07e
5d75555
40c4118
caf27cc
65cf7f6
72d97d2
0c72edb
42fce5e
99d20a4
8b82759
f00a0f0
3c50200
b21da21
20276ca
55c4907
955dce8
27850c1
a7f5d85
9545b8e
e32de45
bbc3738
4b299d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||
| import { deriveState } from '@clerk/shared/deriveState'; | ||||||
| import { eventMethodCalled } from '@clerk/shared/telemetry'; | ||||||
| import type { SignedInSessionResource } from '@clerk/types'; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainLeftover import from @clerk/types—migrate to @clerk/shared/types This file still pulls -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 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 -import type { SignedInSessionResource } from '@clerk/types';
+import type { SignedInSessionResource } from '@clerk/shared/types';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| import { batched, computed, onMount, type Store } from 'nanostores'; | ||||||
|
|
||||||
| import { $clerk, $csrState, $initialState } from './internal'; | ||||||
|
|
@@ -51,7 +52,7 @@ export const $userStore = computed([$authStore], auth => auth.user); | |||||
| * @example | ||||||
| * $sessionStore.subscribe((session) => console.log(session.id)) | ||||||
| */ | ||||||
| export const $sessionStore = computed([$authStore], auth => auth.session); | ||||||
| export const $sessionStore = computed([$authStore], auth => auth.session as SignedInSessionResource | null | undefined); | ||||||
|
|
||||||
| /** | ||||||
| * A client side store that is populated after clerk-js has loaded. | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,7 @@ export class BillingSubscription extends BaseResource implements BillingSubscrip | |
|
|
||
| export class BillingSubscriptionItem extends BaseResource implements BillingSubscriptionItemResource { | ||
| id!: string; | ||
| paymentMethodId!: string; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify that The field is marked as required with the 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 The type definition is incomplete. While The field should be added to payment_method_id: string;This ensures consistency across type definitions and maintains type safety for the required field marked with 🤖 Prompt for AI Agents |
||
| plan!: BillingPlan; | ||
| planPeriod!: BillingSubscriptionPlanPeriod; | ||
| status!: BillingSubscriptionStatus; | ||
|
|
@@ -86,6 +87,7 @@ export class BillingSubscriptionItem extends BaseResource implements BillingSubs | |
| } | ||
|
|
||
| this.id = data.id; | ||
| this.paymentMethodId = data.payment_method_id; | ||
| this.plan = new BillingPlan(data.plan); | ||
| this.planPeriod = data.plan_period; | ||
| this.status = data.status; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ function SignInFlowProvider({ children, exampleMode, fallback, isRootPath }: Sig | |
| const cb = () => { | ||
| const evt: SignInRouterInitEvent = { | ||
| type: 'INIT', | ||
| // @ts-expect-error - ignore error for now | ||
| clerk, | ||
|
Comment on lines
+44
to
45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address type mismatch before merging. The Consider either:
To verify the current type definition: #!/bin/bash
# Find the SignInRouterInitEvent type definition
ast-grep --pattern 'type SignInRouterInitEvent = {
$$$
}'🤖 Prompt for AI Agents |
||
| exampleMode, | ||
| formRef, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ function SignUpFlowProvider({ children, exampleMode, fallback, isRootPath }: Sig | |
| const cb = () => { | ||
| const evt: SignUpRouterInitEvent = { | ||
| type: 'INIT', | ||
| // @ts-expect-error - ignore error for now | ||
| clerk, | ||
|
Comment on lines
+44
to
45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Address type mismatch before merging. This has the same issue as Update the #!/bin/bash
# Find the SignUpRouterInitEvent type definition
ast-grep --pattern 'type SignUpRouterInitEvent = {
$$$
}' |
||
| exampleMode, | ||
| formRef, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import type { Color } from '@clerk/types'; | ||
| import type { Color } from '../types'; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainRemove backup file from version control. This Consider removing this file: 🏁 Script executed: #!/bin/bash
# Check if there's a non-backup version of this test file
fd -t f "color.test.ts$" packages/shared/src/__tests__/
# Check for other .bak files that might need cleanup
fd -e bak . packages/Length of output: 47 Remove the orphaned The file 🤖 Prompt for AI Agents |
||
|
|
||
| import { colorToSameTypeString, hexStringToRgbaColor, stringToHslaColor, stringToSameTypeColor } from '../color'; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify 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 bundlerSince this is part of the migration from
tsuptotsdown/rolldown, ensure that:tsdown.config.mts)Run the following to verify the build tooling setup:
🏁 Script executed:
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:
Length 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
tsuptotsdown/rolldownis not yet complete across all packages.Current state:
packages/sharedhastsdown.config.mtsand usestsdownin build scriptstsup.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