fix(shared): drop docsUrl for Enstall OAuth provider#8885
Conversation
🦋 Changeset detectedLatest commit: 805dd38 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 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.
|
📝 WalkthroughWalkthrough
ChangesOAuthProviderData.docsUrl optionality and enstall data update
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/types/oauth.ts (1)
5-10: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd JSDoc to document the public
OAuthProviderDatainterface.The
OAuthProviderDatainterface is a public, reference-facing API exported from@clerk/sharedthat will appear in generated customer documentation. Per the coding guidelines forpackages/**/src/**/*.tsfiles, all public APIs must be documented with comprehensive JSDoc comments.This interface lacks documentation explaining:
- The purpose of the interface (OAuth provider configuration metadata)
- Each property and its role
- That
docsUrlis now optional (not all providers have public documentation)This is especially important since this PR changes the API surface by making
docsUrloptional—customers using this interface need clear documentation of the change.📝 Suggested JSDoc for OAuthProviderData
import type { OAuthStrategy } from './strategies'; export type OAuthScope = string; +/** + * Represents the configuration metadata for an OAuth provider. + * + * This interface defines the structure of OAuth provider entries used by Clerk's OAuth integration layer. + * Each provider has a unique identifier, associated strategy, and optional documentation link. + * + * `@property` provider - The unique provider identifier (e.g., 'google', 'github', 'enstall') + * `@property` strategy - The OAuth strategy name for this provider + * `@property` name - Human-readable display name of the provider + * `@property` docsUrl - Optional URL to the provider's documentation. Not all providers expose public documentation. + */ export interface OAuthProviderData { provider: OAuthProvider; strategy: OAuthStrategy; name: string; docsUrl?: string; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/types/oauth.ts` around lines 5 - 10, The public OAuthProviderData interface is missing JSDoc documentation required for all public APIs in packages/shared. Add a comprehensive JSDoc comment block above the interface definition that explains: the purpose of the interface as OAuth provider configuration metadata, the role of each property (provider, strategy, name, and docsUrl), and specifically document that docsUrl is optional since not all providers have public documentation. This ensures the interface is properly documented for generated customer documentation and clarifies the API surface change.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/shared/src/types/oauth.ts`:
- Around line 5-10: The public OAuthProviderData interface is missing JSDoc
documentation required for all public APIs in packages/shared. Add a
comprehensive JSDoc comment block above the interface definition that explains:
the purpose of the interface as OAuth provider configuration metadata, the role
of each property (provider, strategy, name, and docsUrl), and specifically
document that docsUrl is optional since not all providers have public
documentation. This ensures the interface is properly documented for generated
customer documentation and clarifies the API surface change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 936623be-bf17-4884-ae4f-fe84ff5cda43
📒 Files selected for processing (4)
.changeset/fresh-lies-film.mdpackages/shared/src/oauth.tspackages/shared/src/types/oauth.tspackages/shared/src/types/runtime-values.ts
💤 Files with no reviewable changes (2)
- packages/shared/src/oauth.ts
- packages/shared/src/types/runtime-values.ts
API Changes Report
Summary
🔴 Breaking changes index (1)Every breaking change, up front. Full diffs are in the package sections below.
@clerk/sharedCurrent version: 4.18.0 Subpath
|
|
@manovotny a bit worried about this warning. Making Wanted to sanity-check whether we’re okay treating this as a real breaking change, or if we'd rather preserve the current type shape and use a safer fallback for Enstall instead? 
|
This is expected result with our new break check tool. But this type is only used by docs correct? Tagging @jacekradko here |
| provider: 'enstall', | ||
| strategy: 'oauth_enstall', | ||
| name: 'Enstall', | ||
| docsUrl: 'https://clerk.com/docs/authentication/social-connections/enstall', |
There was a problem hiding this comment.
I ran a full org-wide sweep of docsUrl / OAUTH_PROVIDERS consumers (in-repo + gh search code --owner clerk), and there is the one spot worth a second look — everything appears safe.
clerk/cli reads OAUTH_PROVIDERS[].docsUrl but defensively (?.docsUrl ?? fallback), and the in-repo UI consumers only read .strategy/.provider/.name, never .docsUrl, so we're good here.
The tricky one is clerk/dashboard. Its getSocialSettings.ts imports OAUTH_PROVIDERS from @clerk/shared/oauth and reads item.docsUrl with a name-based fallback:
item.docsUrl ?? `https://clerk.com/docs/guides/configure/auth-strategies/social-connections/${item.provider}`Then, it renders that as the provider's docs link on the SSO connection page.
With docsUrl gone, Enstall falls through to …/guides/configure/auth-strategies/social-connections/enstall, which currently 404s — whereas the URL being removed here 308-redirects to the live social-connections overview (via the redirect added in clerk-docs#3391). So on the Enstall-enabled instance, this swaps a working (redirected) link for a hard 404 on the surface that actually renders it.
Since the dashboard is what surfaces this link, should we pair this with a dashboard change that drops the link when docsUrl is absent (or skips the fallback for private providers), or add a clerk-docs redirect for the new …/auth-strategies/social-connections/enstall path so the fallback resolves?
There was a problem hiding this comment.
Nice catch! Made a sibling dashboard PR. @manovotny
Makes sense! But I should prob make it a major bump then? @wobsoriano |
There was a problem hiding this comment.
I am good with the changes here and in https://github.com/clerk/dashboard/pull/9497 (this PR needs to land first).
I would appreciate another set of eyes and approval before merge to ensure we're not overlooking anything else.
3 participants
This PR is paired with a
dashboardPR.Problem
The JS SDK was shipping a
docsUrlfor Enstall that points to a docs page that does not exist:https://clerk.com/docs/authentication/social-connections/enstall. That link surfaces to end users as a 404.Enstall is a private, single-customer OIDC provider rather than a general public social connection, so we should not ship a public docs link for it.
This PR removes the public
docsUrlfor the Enstall OAuth provider fromclerk/javascript.Changes
docsUrloptional onOAuthProviderDatadocsUrlfrom the Enstall provider entry in:packages/shared/src/oauth.tspackages/shared/src/types/runtime-values.tsdocsUrlis always present for OAuth providersNotes
clerk-docsfor already-shipped SDK versions that still contain the old URLSummary by CodeRabbit