feat(ui): Add unified ui prop with RSC support via conditional exports#7664
feat(ui): Add unified ui prop with RSC support via conditional exports#7664jacekradko merged 60 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: 3987c28 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
📝 WalkthroughWalkthroughAdds a public Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vue/src/plugin.ts (1)
81-91: Misleading comment on fallback behavior.The comment states "fall back to clerkUiCtor (deprecated)" but the actual fallback is loading from CDN via
loadClerkUiScript, not checking aclerkUiCtorproperty. Consider updating the comment to accurately reflect the behavior:- // Check for ui.ctor first (new API), then fall back to clerkUiCtor (deprecated) + // Check for ui.ctor first (bundled UI), then fall back to loading from CDN
🧹 Nitpick comments (1)
packages/astro/src/internal/create-clerk-instance.ts (1)
118-122: Misleading comment: fallback is CDN loading, not clerkUiCtor.The comment mentions "fall back to clerkUiCtor (deprecated)" but the actual fallback is loading from CDN via
loadClerkUiScript. TheclerkUiCtoroption path was removed from this function.Suggested fix
- // Check for ui.ctor first (new API), then fall back to clerkUiCtor (deprecated) + // If ui.ctor is provided (bundled UI), use it directly; otherwise load from CDN const ctorFromUi = options?.ui?.ctor; if (ctorFromUi) { return ctorFromUi; }
jacekradko
force-pushed
the
jrad/ui-prop-cleanup
branch
from
6bbf272 to
5f6df9f
Compare
jacekradko
force-pushed
the
jrad/ui-prop-cleanup
branch
from
5f6df9f to
431aa50
Compare
Adds `ui` prop to ClerkProvider for specifying UI metadata.
Each SDK decides whether to use `ui.ctor` based on support level.
- `@clerk/ui` exports only `{ ui }` with version and ctor
- Chrome extension uses `ui.ctor` (verified to work)
- React, Vue, Astro use CDN loading (not verified for bundling yet)
- Omit `clerkUiCtor` from public ClerkProviderProps type
jacekradko
force-pushed
the
jrad/ui-prop-cleanup
branch
from
431aa50 to
c5ff0d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/astro/src/internal/create-clerk-instance.ts (1)
111-124:ui.ctoris not being used when provided.The PR objective states that when
ui.ctoris provided, it should be used instead of loading from CDN. However,getClerkUiEntryChunkalways loads the UI script from CDN and ignoresoptions.ui?.ctor. This contradicts the JSDoc inpackages/astro/src/types.ts(lines 39-43) which states: "When provided with a bundled UI viaui.ctor, it will be used instead of loading from CDN."Suggested fix to honor ui.ctor when provided
async function getClerkUiEntryChunk<TUi extends Ui = Ui>( options?: AstroClerkCreateInstanceParams<TUi>, ): Promise<ClerkUiConstructor> { + // Use bundled UI constructor if provided + if (options?.ui?.ctor) { + return options.ui.ctor; + } + await loadClerkUiScript(options); if (!window.__internal_ClerkUiCtor) { throw new Error('Failed to download latest Clerk UI. Contact support@clerk.com.'); } return window.__internal_ClerkUiCtor; }
🤖 Fix all issues with AI agents
In `@packages/vue/src/plugin.ts`:
- Around line 81-87: The code always calls loadClerkUiScript and ignores a
provided bundled constructor; update the clerkUiCtorPromise logic to first check
pluginOptions.ui?.ctor (or options.ui?.ctor) and resolve to that ctor if
present, otherwise call loadClerkUiScript and fall back to
window.__internal_ClerkUiCtor; ensure clerkUiCtorPromise returns the provided
ctor when available, and preserve the existing error throw if neither the
provided ctor nor the downloaded window.__internal_ClerkUiCtor is present.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/chrome-extension/src/react/ClerkProvider.tsx`:
- Around line 38-40: The spread uses an unsafe cast ({...(rest as any)}) to
sneak in clerkUiCtor, hiding a type mismatch; replace this by declaring an
internal extended props type (e.g., InternalClerkProviderProps) that extends
ClerkProviderProps and includes clerkUiCtor with the correct type, update the
component signature to use that internal type, remove the as any cast, and pass
clerkUiCtor={ui.ctor} and Clerk={clerkInstance} with proper typing so TypeScript
enforces compatibility (locate symbols: ClerkProvider.tsx, rest, clerkUiCtor,
ui.ctor, ClerkReactProvider, ClerkProviderProps, clerkInstance).
♻️ Duplicate comments (1)
packages/vue/src/plugin.ts (1)
81-87:ui.ctoris not being used when provided.Same issue as in the Astro integration: the code always loads the UI from CDN via
loadClerkUiScriptand ignorespluginOptions.ui?.ctor. This contradicts the documented behavior that bundled UI viaui.ctorshould be used instead of CDN loading.
🧹 Nitpick comments (1)
.changeset/shiny-owls-dance.md (1)
10-23: Consider adding migration guidance.The changeset describes the new
uiprop but doesn't mention the deprecation or removal ofclerkUiCtor. Since the PR objectives state "Omits clerkUiCtor from public ClerkProviderProps," consider adding explicit migration guidance to help users transition from the old API.📝 Suggested addition for migration guidance
Usage: ```tsx import { ui } from '@clerk/ui'; <ClerkProvider ui={ui}> ... </ClerkProvider>
+Migration note: The
clerkUiCtorprop has been removed from public types. Use theuiprop instead by importing theuiobject from@clerk/ui.</details> </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react/src/isomorphicClerk.ts (1)
511-535: Add tests for the new bundledui.ctorpath and CDN fallback.Line 511 introduces a new early-return path for
ui.ctor. I don’t see tests in this PR; please add (or point to) coverage that validates the bundled constructor path and the CDN fallback to prevent regressions.
…dledUI escape hatch
The dynamic import('@clerk/ui/entry') in @clerk/react caused tsup to bundle
the entire @clerk/ui package into the dist (30MB, 219 files). Externalizing it
broke Vite-based apps that don't have @clerk/ui installed.
Move the RSC resolution to @clerk/nextjs where bundle:false preserves the
import as-is, and it's only consumed by Next.js (webpack/turbopack) which
handles unresolvable dynamic imports gracefully.
Replace the duplicate __PKG_VERSION__ constant with PACKAGE_VERSION to match the naming convention used by all other packages in the monorepo. Remove redundant defines from rspack, tsdown, and vitest configs.
4 participants
Summary
Refactors the UI loading architecture so users only interact with an opaque
uiprop from@clerk/ui. Each SDK internally decides whether to use the bundled UI constructor or load from CDN.Key addition: Added
react-serverconditional export so theuiprop works directly in Next.js App Router server components without requiring a client wrapper.Key Changes
ui={ui}- the object is opaque and brandedclerkUiCtorfrom publicIsomorphicClerkOptions(kept internally asui.ClerkUIfor SDK use)ui.ClerkUI)react-serverconditional export that provides a server-safe markerui.versionto pin the UI versionHow It Works
The
@clerk/uipackage uses conditional exports:server.js{ __brand, version }index.js{ __brand, version, ClerkUI }When IsomorphicClerk receives the
uiprop:ui.ClerkUIexists → use directly (bundled)ui.__brand === '__clerkUI'but noClerkUI→ dynamic import from@clerk/ui/entrySDK Behavior
ui={ui}directlyui.ClerkUIdirectlyui.ClerkUIdirectlyui.ClerkUIdirectlyUsage
Next.js App Router (server component):
Other SDKs:
Test plan
Summary by CodeRabbit
New Features
Breaking / Public API
Tests