-
Notifications
You must be signed in to change notification settings - Fork 431
feat(react,vue,astro): Replace clerkUiCtor prop with ui prop #7664
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 08064ce 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: |
📝 WalkthroughWalkthroughRenames 🚥 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 |
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: 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; }
6bbf272 to
5f6df9f
Compare
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
431aa50 to
c5ff0d1
Compare
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
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.
packages/vue/src/plugin.ts
Outdated
| const clerkUiCtorPromise = (async () => { | ||
| await loadClerkUiScript(options); | ||
| if (!window.__internal_ClerkUiCtor) { | ||
| throw new Error('Failed to download latest Clerk UI. Contact support@clerk.com.'); | ||
| } | ||
| return window.__internal_ClerkUiCtor; | ||
| })(); |
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.
ui.ctor is not being used when provided.
Same issue as in the Astro integration: the code always loads the UI from CDN via loadClerkUiScript and ignores pluginOptions.ui?.ctor. This contradicts the documented behavior that bundled UI via ui.ctor should be used instead of CDN loading.
Suggested fix to honor ui.ctor when provided
const clerkUiCtorPromise = (async () => {
+ // Use bundled UI constructor if provided
+ if (pluginOptions.ui?.ctor) {
+ return pluginOptions.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;
})();📝 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.
| const clerkUiCtorPromise = (async () => { | |
| await loadClerkUiScript(options); | |
| if (!window.__internal_ClerkUiCtor) { | |
| throw new Error('Failed to download latest Clerk UI. Contact support@clerk.com.'); | |
| } | |
| return window.__internal_ClerkUiCtor; | |
| })(); | |
| const clerkUiCtorPromise = (async () => { | |
| // Use bundled UI constructor if provided | |
| if (pluginOptions.ui?.ctor) { | |
| return pluginOptions.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; | |
| })(); |
🤖 Prompt for AI Agents
In `@packages/vue/src/plugin.ts` around lines 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Check for options.ui.ClerkUI before loading from CDN. If a bundled ClerkUIConstructor is provided, return it directly instead of calling loadClerkUiScript.
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
🤖 Fix all issues with AI agents
In `@packages/astro/src/internal/create-clerk-instance.ts`:
- Around line 127-134: The spread into loadClerkUiScript is being overwritten by
clerkUiUrl: options.ui?.url which sets clerkUiUrl to undefined when
options.ui?.url is absent and breaks custom/self-hosted UIs; change the override
in the call to preserve an existing fallback (e.g., use the existing
options.clerkUiUrl or null-coalescing: clerkUiUrl: options.ui?.url ??
options.clerkUiUrl) or simply omit the clerkUiUrl override when options.ui?.url
is undefined so loadClerkUiScript receives the original clerkUiUrl; update the
call site in create-clerk-instance.ts where loadClerkUiScript is invoked and add
a unit/integration test exercising the custom URL path to ensure a provided
custom/self-hosted UI URL is used.
Use null-coalescing to fall back to options.clerkUiUrl when options.ui?.url is not provided, preventing custom/self-hosted UI URLs from being overwritten with undefined. Also adds vitest setup and tests for the clerkUiUrl fallback behavior.
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
🤖 Fix all issues with AI agents
In `@packages/vue/src/plugin.ts`:
- Around line 83-87: The call to loadClerkUiScript is overwriting a valid
options.clerkUIUrl with undefined because pluginOptions.ui?.url may be absent;
update the args so clerkUIUrl is only set when pluginOptions.ui?.url is defined
(e.g., use a conditional or nullish-coalescing to fall back to
options.clerkUIUrl) and keep clerkUIVersion mapping as-is; locate the
loadClerkUiScript invocation and change the clerkUIUrl assignment to use
pluginOptions.ui?.url ?? options.clerkUIUrl or omit the property when undefined.
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
🤖 Fix all issues with AI agents
In `@packages/vue/src/plugin.ts`:
- Around line 83-87: The call to loadClerkUiScript is referencing nonexistent
properties options.clerkUIVersion and options.clerkUIUrl; update the invocation
so it does not fall back to those fields — either pass only
pluginOptions.ui?.version and pluginOptions.ui?.url (remove the options.*
fallbacks) or, if you intended to reuse the JS values, map
options.clerkJSVersion -> clerkUIVersion and options.clerkJSUrl -> clerkUIUrl
before calling loadClerkUiScript; change the arguments in the loadClerkUiScript
call accordingly (refer to loadClerkUiScript, pluginOptions, and options).
♻️ Duplicate comments (1)
packages/vue/src/plugin.ts (1)
81-92: Bundled UI constructor is ignored; code always loads from CDN.When
pluginOptions.ui?.ClerkUIis provided (bundled UI), the code should use it directly instead of loading from CDN. This was flagged in a previous review and remains unaddressed.
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
🤖 Fix all issues with AI agents
In `@integration/templates/react-router-library/src/main.tsx`:
- Around line 15-18: The template now uses the renamed ClerkProvider prop
clerkUIUrl; update or add tests to exercise this public API change by asserting
that ClerkProvider is rendered with clerkUIUrl (not the old prop) in
integration/template validation or unit tests—locate usages of ClerkProvider in
main.tsx and update any existing tests referencing the old prop name to pass
clerkUIUrl and verify the provider receives and uses that value (e.g., via
shallow render, snapshot, or template validation).
…opt-out for Next.js Replaces __internal_forceBundledUI with __internal_preferCDN to fix double-loading issue where importing @clerk/ui would bundle ClerkUI but still load it from CDN. - Default behavior now uses bundled UI when ui.ClerkUI is provided - Next.js sets __internal_preferCDN due to bundling issues - Chrome Extension simplified (no longer needs explicit flag)
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
🤖 Fix all issues with AI agents
In `@packages/react/src/isomorphicClerk.ts`:
- Around line 464-477: getClerkUiEntryChunk is async and is being called without
awaiting its Promise, so ClerkUI is a Promise instead of the resolved
constructor; change the call to await this.getClerkUiEntryChunk() so ClerkUI
contains the actual constructor before passing it into clerk.load({... ui: {
..., ClerkUI }}). Locate the usage around getClerkUiEntryChunk,
getClerkJsEntryChunk, beforeLoad, and clerk.load in isomorphicClerk.ts and
ensure ClerkUI is awaited (and handle any needed async context) before calling
clerk.load with the ui.ClerkUI value.
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 Changes
ui={ui}- the object is opaqueclerkUiCtorfrom publicIsomorphicClerkOptions(kept internally inClerkOptionsfor SDK use)__internal_forceBundledUIescape hatch for SDKs that must bundle the UIui.versionto pin the UI versionSDK Behavior
__internal_forceBundledUI: trueui.versionpinningui.versionpinningui.versionpinningUsage
The
uiexport is opaque - users just pass it to ClerkProvider without accessing internals. The SDK handles whether to useui.ctoror load from CDN.Test plan
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.