-
Notifications
You must be signed in to change notification settings - Fork 406
refactor(clerk-js): Standardize API keys naming convention #7223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: ddcdb76 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.
|
WalkthroughStandardizes API keys naming and casing across packages (ApiKeys → APIKeys), renames public APIs from Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant ClerkCore as Clerk (core)
participant Feature as FeatureFlags
participant APIKeysComp as APIKeys Component
App->>ClerkCore: mountAPIKeys(node, props)
ClerkCore->>ClerkCore: assert component readiness
ClerkCore->>ClerkCore: log early-access warning
ClerkCore->>Feature: check disabledAllAPIKeysFeatures
alt all API keys disabled
Feature-->>ClerkCore: true
ClerkCore->>App: throw ClerkRuntimeError (CANNOT_RENDER_API_KEYS_DISABLED)
else
Feature-->>ClerkCore: false
ClerkCore->>Feature: check disabledOrganizationAPIKeysFeature (if org)
alt org API keys disabled
Feature-->>ClerkCore: true
ClerkCore->>App: throw ClerkRuntimeError (CANNOT_RENDER_API_KEYS_ORG_DISABLED)
else
Feature-->>ClerkCore: false
ClerkCore->>Feature: check disabledUserAPIKeysFeature
alt user API keys disabled
Feature-->>ClerkCore: true
ClerkCore->>App: throw ClerkRuntimeError (CANNOT_RENDER_API_KEYS_USER_DISABLED)
else
Feature-->>ClerkCore: false
ClerkCore->>APIKeysComp: mount component (render)
ClerkCore->>ClerkCore: record telemetry
APIKeysComp-->>App: mounted
end
end
end
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)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (2)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/clerk-js/src/core/clerk.ts (1)
1247-1263: Feature flag checks should be mutually exclusive based on context.The current logic checks both organization-specific and user-specific feature flags sequentially, which could cause incorrect behavior. When
this.organizationexists (org context), both the org check (lines 1247-1254) and user check (lines 1256-1263) can execute. This means API keys could be blocked due to user-level restrictions even when mounting in an organization context.Apply this diff to make the checks mutually exclusive:
if (this.organization && disabledOrganizationAPIKeysFeature(this, this.environment)) { if (this.#instanceType === 'development') { throw new ClerkRuntimeError(warnings.cannotRenderAPIKeysComponentForOrgWhenDisabled, { code: CANNOT_RENDER_API_KEYS_ORG_DISABLED_ERROR_CODE, }); } return; - } - - if (disabledUserAPIKeysFeature(this, this.environment)) { + } else if (!this.organization && disabledUserAPIKeysFeature(this, this.environment)) { if (this.#instanceType === 'development') { throw new ClerkRuntimeError(warnings.cannotRenderAPIKeysComponentForUserWhenDisabled, { code: CANNOT_RENDER_API_KEYS_USER_DISABLED_ERROR_CODE, }); } return; }
🧹 Nitpick comments (2)
packages/react/src/components/uiComponents.tsx (1)
641-641: Inconsistent component name casing.The component identifier uses
'ApiKeys'with lowercase 'i', while the rest of the naming convention standardization uses uppercase 'API'. Consider updating this to'APIKeys'for consistency.Apply this diff to align the component name:
- { component: 'ApiKeys', renderWhileLoading: true }, + { component: 'APIKeys', renderWhileLoading: true },packages/clerk-js/src/ui/components/ApiKeys/ApiKeys.tsx (1)
104-105: Inconsistent naming convention forapiKeyNameidentifiers.The refactor standardized
apiKeyIdtoapiKeyID(uppercase acronym), butapiKeyNameremains with lowercase "name". For consistency with theapiKeyIDpattern, consider updating toapiKeyName(title case "Name").Apply these changes for consistency:
- const [selectedAPIKeyID, setSelectedAPIKeyID] = useState(''); - const [selectedApiKeyName, setSelectedApiKeyName] = useState(''); + const [selectedAPIKeyID, setSelectedAPIKeyID] = useState(''); + const [selectedAPIKeyName, setSelectedAPIKeyName] = useState('');- const handleRevoke = (apiKeyID: string, apiKeyName: string) => { + const handleRevoke = (apiKeyID: string, apiKeyName: string) => { setSelectedAPIKeyID(apiKeyID); - setSelectedApiKeyName(apiKeyName); + setSelectedAPIKeyName(apiKeyName); setIsRevokeModalOpen(true); };onClose={() => { setSelectedAPIKeyID(''); - setSelectedApiKeyName(''); + setSelectedAPIKeyName(''); setIsRevokeModalOpen(false); }} apiKeyID={selectedAPIKeyID} - apiKeyName={selectedApiKeyName} + apiKeyName={selectedAPIKeyName} onRevokeSuccess={invalidateAll}Also applies to: 126-128, 223-224, 227-228
📜 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 (18)
.changeset/brown-bags-fold.md(1 hunks)packages/clerk-js/sandbox/app.ts(1 hunks)packages/clerk-js/src/core/clerk.ts(2 hunks)packages/clerk-js/src/ui/components/ApiKeys/ApiKeys.tsx(3 hunks)packages/clerk-js/src/ui/components/ApiKeys/RevokeAPIKeyConfirmationModal.tsx(3 hunks)packages/clerk-js/src/ui/components/ApiKeys/__tests__/ApiKeys.spec.tsx(1 hunks)packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationAPIKeysPage.tsx(1 hunks)packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsx(3 hunks)packages/clerk-js/src/ui/components/UserProfile/APIKeysPage.tsx(1 hunks)packages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx(1 hunks)packages/clerk-js/src/ui/contexts/components/OrganizationProfile.ts(3 hunks)packages/clerk-js/src/ui/contexts/components/index.ts(1 hunks)packages/clerk-js/src/ui/lazyModules/components.ts(1 hunks)packages/react/src/components/uiComponents.tsx(1 hunks)packages/react/src/isomorphicClerk.ts(3 hunks)packages/shared/src/react/hooks/index.ts(1 hunks)packages/shared/src/react/hooks/useAPIKeys.ts(6 hunks)packages/shared/src/types/clerk.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
packages/clerk-js/src/ui/contexts/components/OrganizationProfile.ts (1)
packages/clerk-js/src/ui/constants.ts (1)
ORGANIZATION_PROFILE_NAVBAR_ROUTE_ID(8-13)
packages/clerk-js/src/core/clerk.ts (1)
packages/shared/src/types/clerk.ts (1)
APIKeysProps(1959-1978)
packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsx (1)
packages/clerk-js/src/ui/contexts/components/OrganizationProfile.ts (1)
useOrganizationProfileContext(33-86)
packages/clerk-js/sandbox/app.ts (2)
packages/clerk-js/src/core/clerk.ts (1)
Clerk(200-3040)packages/shared/src/types/clerk.ts (1)
Clerk(169-939)
packages/react/src/isomorphicClerk.ts (1)
packages/shared/src/types/clerk.ts (1)
APIKeysProps(1959-1978)
packages/shared/src/react/hooks/useAPIKeys.ts (4)
packages/shared/src/react/types.ts (2)
PaginatedHookConfig(89-102)PaginatedResources(13-79)packages/shared/src/types/apiKeys.ts (1)
APIKeyResource(5-22)packages/shared/src/react/hooks/index.ts (1)
useAPIKeys(2-2)packages/shared/src/telemetry/events/method-called.ts (1)
eventMethodCalled(13-25)
⏰ 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 (17)
.changeset/brown-bags-fold.md (1)
1-7: LGTM!The changeset correctly documents the naming convention standardization with appropriate minor version bumps for the affected packages.
packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationAPIKeysPage.tsx (1)
8-8: LGTM!The import path has been correctly updated to align with the APIKeys directory rename. This change is consistent with similar updates across the codebase.
packages/clerk-js/src/ui/components/OrganizationProfile/OrganizationProfileRoutes.tsx (3)
16-20: LGTM!The lazy import path and chunk name are correctly configured for the OrganizationAPIKeysPage component.
134-134: LGTM!The route path condition correctly uses the renamed
isAPIKeysPageRootvariable.
46-46: Property rename verified successfully.The context property has been correctly renamed to
isAPIKeysPageRootand is properly exported from OrganizationProfileContext. The destructuring in OrganizationProfileRoutes.tsx (line 46) correctly uses the renamed property, matching the context type definition and export.packages/shared/src/react/hooks/index.ts (1)
2-2: No issues found with the renamed hook export.The verification confirms that
useAPIKeysis properly exported from the implementation file at line 69 ofpackages/shared/src/react/hooks/useAPIKeys.ts. The re-export inpackages/shared/src/react/hooks/index.tscorrectly references this export, so the naming change fromuseApiKeystouseAPIKeysis consistent across the codebase.packages/clerk-js/src/ui/components/UserProfile/UserProfileRoutes.tsx (1)
16-20: Lazy import path verification successful.The file
packages/clerk-js/src/ui/components/UserProfile/APIKeysPage.tsxexists at the expected location and correctly exportsAPIKeysPage. The updated import path is valid and the code change is correct.packages/react/src/components/uiComponents.tsx (1)
631-632: Method names verified as correct.Both
mountAPIKeysandunmountAPIKeysexist on the Clerk instance with the proper type signatures defined inpackages/shared/src/types/clerk.tsand implemented inpackages/clerk-js/src/core/clerk.ts. The usage at lines 631-632 is valid.packages/clerk-js/src/ui/components/ApiKeys/__tests__/ApiKeys.spec.tsx (1)
6-6: LGTM! Import path correctly updated.The import path has been updated to reflect the standardized naming convention for the APIKeys module.
packages/clerk-js/src/ui/contexts/components/index.ts (1)
1-1: LGTM! Export path correctly updated.The export path has been updated to match the standardized APIKeys module naming.
packages/clerk-js/sandbox/app.ts (1)
328-328: LGTM! Method call correctly updated.The sandbox app now correctly uses the renamed
mountAPIKeysmethod, aligning with the standardized API naming convention.packages/clerk-js/src/ui/components/ApiKeys/RevokeAPIKeyConfirmationModal.tsx (1)
17-17: LGTM! Prop renamed consistently.The
apiKeyIdprop has been correctly renamed toapiKeyIDthroughout the component, following the common convention of capitalizing "ID" as a suffix. All references (prop type, parameter destructuring, conditional check, and API call) have been updated consistently.Also applies to: 27-27, 51-51, 55-55
packages/shared/src/types/clerk.ts (1)
586-586: LGTM! Public API methods renamed consistently.The
mountApiKeysandunmountApiKeysmethods have been correctly renamed tomountAPIKeysandunmountAPIKeys, standardizing the naming convention. Since these methods are marked as@experimental, the breaking change is acceptable. The JSDoc comments remain accurate and appropriate.Also applies to: 598-598
packages/clerk-js/src/ui/contexts/components/OrganizationProfile.ts (1)
27-27: LGTM! Property renamed consistently.The
isApiKeysPageRootproperty has been correctly renamed toisAPIKeysPageRootthroughout the context type definition, variable assignment, and return object, maintaining consistency with the standardized naming convention.Also applies to: 70-70, 83-83
packages/react/src/isomorphicClerk.ts (1)
1149-1163: LGTM! Public methods renamed correctly.The
mountApiKeysandunmountApiKeysmethods have been correctly renamed tomountAPIKeysandunmountAPIKeys, and the internal calls toclerkjs.mountAPIKeysandclerkjs.unmountAPIKeysare properly updated.Note: The internal property references (
premountAPIKeysNode) should use plural form as noted in the previous comment.packages/shared/src/react/hooks/useAPIKeys.ts (1)
12-12: LGTM! Hook and types renamed consistently.The
useApiKeyshook and its associated types have been comprehensively renamed touseAPIKeys,UseAPIKeysParams, andUseAPIKeysReturn. All references including JSDoc comments, telemetry events, and type casts have been updated correctly. The changes maintain functionality while improving naming consistency.Also applies to: 26-26, 69-69, 84-84, 107-107
packages/clerk-js/src/core/clerk.ts (1)
1285-1288: LGTM!The
unmountAPIKeysmethod correctly handles unmounting without requiring feature flag checks. The implementation is consistent with other unmount methods in the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/clerk-js/src/ui/components/ApiKeys/APIKeys.tsx(3 hunks)packages/clerk-js/src/ui/components/ApiKeys/__tests__/APIKeys.spec.tsx(1 hunks)packages/react/src/isomorphicClerk.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react/src/isomorphicClerk.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). (3)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/clerk-js/src/ui/components/ApiKeys/__tests__/APIKeys.spec.tsx (1)
6-6: LGTM!Import path correctly updated to reflect the standardized module naming.
packages/clerk-js/src/ui/components/ApiKeys/APIKeys.tsx (2)
126-130: LGTM!Function parameter and state setter correctly updated to use
apiKeyIDwith uppercase conventions.
222-231: No issues found—props match the component interface perfectly.The
RevokeAPIKeyConfirmationModalcomponent's prop interface definesapiKeyID(line 14) andapiKeyName(line 16), which exactly match the props being passed in APIKeys.tsx lines 227–228. There is no inconsistency between the usage and the component's prop interface. While the naming pattern differs betweenapiKeyID(uppercase API) andapiKeyName(lowercase api), this pattern is consistent within the interface itself and was not changed by this PR snippet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/clerk-js/src/ui/components/APIKeys/APIKeys.tsx (1)
105-105: Incomplete standardization:selectedApiKeyNamestill uses lowercase "Api".Line 105 still uses lowercase "Api" in
selectedApiKeyName, which is inconsistent with line 104'sselectedAPIKeyID(uppercase "API"). This was flagged in a previous review and remains unresolved. To complete the standardization, rename toselectedAPIKeyNameand update all references on lines 128, 224, and 228.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/clerk-js/src/ui/components/APIKeys/APIKeys.tsx(3 hunks)packages/clerk-js/src/ui/components/APIKeys/RevokeAPIKeyConfirmationModal.tsx(3 hunks)packages/clerk-js/src/ui/components/APIKeys/__tests__/APIKeys.spec.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (2)
packages/clerk-js/src/ui/components/APIKeys/__tests__/APIKeys.spec.tsx (1)
6-6: LGTM!The import path correctly updated to match the renamed module directory structure.
packages/clerk-js/src/ui/components/APIKeys/RevokeAPIKeyConfirmationModal.tsx (1)
17-17: LGTM!All instances of
apiKeyIdhave been consistently renamed toapiKeyID(uppercase "ID"), properly standardizing the naming convention throughout the component.Also applies to: 27-27, 51-51, 55-55
@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: |
Description
This PR standardizes remaining API keys that is camel-cased like
mountApiKeysintomountAPIKeys(with "API" in all caps) across the codebase.This is technically a breaking change for devs that directly calls
mountApiKeysorunmountApiKeyson the Clerk instance. However:<APIKeys />AIO rather than calling mount methods directly@experimentaland in early accessChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores