feat(react,vue,astro): Replace clerkUiCtor prop with ui prop #7664
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: c0d3b6f 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: |
📝 WalkthroughWalkthroughThe PR renames the UI constructor type to 🚥 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.
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.
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.
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
- Remove top-level ClerkUI from ClerkOptions, now only available via ui.ClerkUI
- SDKs construct ui: { version?, url?, ClerkUI } when loading from CDN
- Fix TypeScript declaration file issues by not spreading branded Ui type
- Update warning function to check ui.version for version pinning detection
The rename commit missed the actual runtime export in packages/ui/src/index.ts.
This fixes the bundled UI scenario where clerk.load({ ui }) expects ui.ClerkUI.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/vue/src/plugin.ts`:
- Around line 81-92: Add unit tests to cover CDN UI loading with version pinning
and custom URL by exercising the ClerkUIPromise path: mock or stub
loadClerkUiScript to capture received options and simulate setting
window.__internal_ClerkUICtor, then assert that loadClerkUiScript was called
with clerkUiVersion equal to pluginOptions.ui?.version and clerkUiUrl equal to
pluginOptions.ui?.url and that ClerkUIPromise resolves to
window.__internal_ClerkUICtor; include tests for (1) version pinning only, (2)
custom URL only, and (3) both combined, using the plugin initialization that
creates ClerkUIPromise.
| // Load UI from CDN with version pinning from ui.version | ||
| const ClerkUIPromise = (async () => { | ||
| await loadClerkUiScript({ | ||
| ...options, | ||
| clerkUiVersion: pluginOptions.ui?.version, | ||
| clerkUiUrl: pluginOptions.ui?.url, | ||
| }); | ||
| 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.
🧩 Analysis chain
🏁 Script executed:
git diff --name-only origin/main...HEAD | grep -E '(test|spec)\.(ts|tsx|js|jsx)$|__tests__|\.test\.|\.spec\.'Repository: clerk/javascript
Length of output: 100
🏁 Script executed:
# Find test files related to Vue plugin
fd -e ts -e tsx -e js -e jsx | grep -E 'vue.*test|vue.*spec|__tests__.*vue'Repository: clerk/javascript
Length of output: 956
🏁 Script executed:
# Check the structure of the Vue package
find packages/vue -type f \( -name '*.test.*' -o -name '*.spec.*' -o -path '*/__tests__/*' \) | head -20Repository: clerk/javascript
Length of output: 765
🏁 Script executed:
# Check if there's a test file for plugin.ts
find packages/vue -name 'plugin*.test.*' -o -name 'plugin*.spec.*'Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
# Look for tests that reference loadClerkUiScript or CDN loading
rg -i 'loadClerkUiScript|clerkUiVersion|clerkUiUrl' packages/vue/src --type ts --type tsx -A 3 -B 3Repository: clerk/javascript
Length of output: 87
🏁 Script executed:
# Check the full plugin.ts file to understand context
cat -n packages/vue/src/plugin.ts | head -120Repository: clerk/javascript
Length of output: 4685
🏁 Script executed:
# Check if there are integration tests that might cover plugin loading
rg 'loadClerkUiScript|clerkUiVersion|clerkUiUrl|plugin.ts' packages/vue --type ts -i -lRepository: clerk/javascript
Length of output: 86
🏁 Script executed:
# Check existing tests to see if they cover plugin initialization
cat packages/vue/src/components/__tests__/ClerkHostRenderer.test.ts | head -50Repository: clerk/javascript
Length of output: 1335
🏁 Script executed:
# Check if plugin initialization is tested anywhere in Vue package
rg 'clerkPlugin|install|plugin' packages/vue/src -i -l --type ts | grep testRepository: clerk/javascript
Length of output: 122
🏁 Script executed:
# Check useClerkContext test to see if it covers plugin initialization
cat packages/vue/src/composables/__tests__/useClerkContext.test.tsRepository: clerk/javascript
Length of output: 1320
🏁 Script executed:
# Check integration tests for Vue plugin
cat integration/tests/vue/components.test.ts | head -100Repository: clerk/javascript
Length of output: 4110
🏁 Script executed:
# Look for any test that imports or mocks loadClerkUiScript
rg 'loadClerkUiScript|loadClerkJsScript' packages/vue -i --type ts -l -A 5Repository: clerk/javascript
Length of output: 112
Add unit tests for Vue plugin CDN UI loading with version pinning.
The new clerkUiVersion and clerkUiUrl parameters in the CDN loading path are not covered by existing tests. Add tests to verify the version pinning and custom URL functionality works correctly.
🤖 Prompt for AI Agents
In `@packages/vue/src/plugin.ts` around lines 81 - 92, Add unit tests to cover CDN
UI loading with version pinning and custom URL by exercising the ClerkUIPromise
path: mock or stub loadClerkUiScript to capture received options and simulate
setting window.__internal_ClerkUICtor, then assert that loadClerkUiScript was
called with clerkUiVersion equal to pluginOptions.ui?.version and clerkUiUrl
equal to pluginOptions.ui?.url and that ClerkUIPromise resolves to
window.__internal_ClerkUICtor; include tests for (1) version pinning only, (2)
custom URL only, and (3) both combined, using the plugin initialization that
creates ClerkUIPromise.
Add tests covering the ClerkUIPromise path in plugin initialization: - Version pinning only (ui.version) - Custom URL only (ui.url) - Both version and URL combined - ClerkUIPromise resolves to window.__internal_ClerkUICtor
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.
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.
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.
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.
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
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.