-
Notifications
You must be signed in to change notification settings - Fork 431
feat(clerk-js): remove headless variant #7629
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: 5cff9a0 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR removes the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🤖 Fix all issues with AI agents
In @.changeset/light-eagles-stay.md:
- Around line 1-2: The changeset is empty and must document the breaking changes
introduced: explicitly list affected package names and semver bumps (use major
bumps for breaking changes), describe the removals of clerkJSVariant and the old
import path `@clerk/clerk-js/headless`, and note the new ui prop and its migration
steps; update the changeset body to include a short summary, per-package entries
(e.g., package-name: major), and a brief migration note explaining how to
replace/remove clerkJSVariant, how to update import paths to the new
entrypoints, and how to adopt the new ui prop so consumers can upgrade safely.
In `@packages/expo/src/provider/singleton/createClerkInstance.ts`:
- Around line 1-2: The import path for FapiRequestInit/FapiResponse is invalid;
instead derive request/response types from the public Clerk API: create a
Handler type from Parameters<Clerk["__internal_onBeforeRequest"]>[0], then
extract Req = Parameters<Handler>[0] and use Req as the type for the requestInit
parameter, and similarly extract the response type from
Clerk["__internal_onAfterResponse"] to replace FapiResponse usage; update
function signatures in createClerkInstance.ts to use these derived types
(referencing Clerk, __internal_onBeforeRequest, __internal_onAfterResponse,
Handler, and Req).
🧹 Nitpick comments (1)
packages/react-router/src/client/ReactRouterClerkProvider.tsx (1)
102-109: Type safety concern withas anycasts.The
as anycasts on lines 102 and 108 bypass TypeScript's type checking. While this may be a workaround for complex generic constraints withTUi, it could mask type mismatches at runtime.Consider whether these casts can be replaced with more precise type assertions or if the underlying types can be aligned to avoid the need for
any.
@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: |
packages/upgrade/src/versions/core-3/changes/clerk-ui-url-renamed.md
Outdated
Show resolved
Hide resolved
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.
Looking good! Few minor things:
clerkUIVersion in isomorphicClerk.ts
The old code pulled this.options.ui?.version into clerkUiVersion when calling loadClerkUiScript. Now it just spreads this.options, but there's no clerkUIVersion on IsomorphicClerkOptions, so the version never actually makes it to the script URL builder. Was this dead code before, or do we need to add the field?
clerkUiCtor casing
We renamed clerkUiUrl → clerkUIUrl to match clerkJSUrl, but clerkUiCtor still has the old casing. Should probably be clerkUICtor while we're at it
Env var naming
CLERK_PREFETCH_UI_DISABLED=true mapping to prefetchUI: false is a double negation. Can we just use CLERK_PREFETCH_UI=false instead? Same for the framework-prefixed variants (NEXT_PUBLIC_CLERK_PREFETCH_UI, PUBLIC_CLERK_PREFETCH_UI, etc.)
|
@nikosdouvlis Addressed your feedback. Thanks for going through it! |
c2824b6 to
089add9
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: 2
🤖 Fix all issues with AI agents
In `@integration/templates/next-app-router/src/app/layout.tsx`:
- Line 15: The prefetch toggle currently checks the wrong env var
(`NEXT_PUBLIC_CLERK_PREFETCH_UI_DISABLED`) so documented users won't disable
prefetching; update the prop check in app/layout.tsx for the prefetchUI prop to
read `process.env.NEXT_PUBLIC_CLERK_PREFETCH_UI === 'false' ? false : undefined`
(keeping undefined as the default), referencing the existing prefetchUI prop to
locate and replace the condition.
In `@packages/astro/src/server/build-clerk-hotload-script.ts`:
- Around line 37-42: The Astro hotload script builder is not passing the
extracted PUBLIC_CLERK_UI_VERSION through, so pinned UI versions are ignored;
update getSafeEnv to include clerkUIVersion in its returned object (extracting
PUBLIC_CLERK_UI_VERSION into clerkUIVersion) and then forward that value into
the clerkUIScriptUrl call (the clerkUiScriptSrc creation) by adding
clerkUIVersion to the argument list so clerkUIScriptUrl can select the correct
UI package version.
| const clerkUiScriptSrc = clerkUIScriptUrl({ | ||
| clerkUIUrl: env.clerkUIUrl, | ||
| domain, | ||
| proxyUrl, | ||
| publishableKey, | ||
| }); |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and read the specific file and lines mentioned
fd "build-clerk-hotload-script.ts" packages/astro/src --exec wc -l {} \;Repository: clerk/javascript
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Read the file to see the actual code at lines 37-42
cat -n packages/astro/src/server/build-clerk-hotload-script.ts | sed -n '30,50p'Repository: clerk/javascript
Length of output: 743
🏁 Script executed:
#!/bin/bash
# Search for clerkUIVersion usage in astro package
rg "clerkUIVersion" packages/astro/src -n -B 2 -A 2Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Find clerkUIScriptUrl definition to check its parameters
rg "clerkUIScriptUrl" packages/astro/src -n -A 5Repository: clerk/javascript
Length of output: 1156
🏁 Script executed:
#!/bin/bash
# Check getSafeEnv to see what environment variables are included
rg "getSafeEnv\|clerkUIVersion" packages/astro/src -n -B 2 -A 10Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Read the getSafeEnv function to see what env variables are provided
cat -n packages/astro/src/server/get-safe-env.tsRepository: clerk/javascript
Length of output: 3182
🏁 Script executed:
#!/bin/bash
# Find clerkUIScriptUrl definition in shared package to check parameters
fd "loadClerkJsScript" --type f | head -5Repository: clerk/javascript
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Search broadly for clerkUIVersion in the entire repo
rg "clerkUIVersion" --type ts --type tsx --max-count 20 | head -30Repository: clerk/javascript
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Read the clerkUIScriptUrl definition to check its signature
cat -n packages/shared/src/loadClerkJsScript.tsRepository: clerk/javascript
Length of output: 14498
🏁 Script executed:
#!/bin/bash
# Search for clerkUIVersion more broadly - might be in comments or different spelling
rg "UI.*[Vv]ersion|ui.*version" packages/astro/src -i -nRepository: clerk/javascript
Length of output: 360
Forward clerkUIVersion into the UI script URL builder after extracting it in getSafeEnv.
clerkUIScriptUrl accepts clerkUIVersion and uses it to determine the UI package version (see packages/shared/src/loadClerkJsScript.ts:244-251), but the Astro integration doesn't extract PUBLIC_CLERK_UI_VERSION from the environment in getSafeEnv. This means pinned UI versions in PUBLIC_CLERK_UI_VERSION are currently ignored in Astro hotload mode.
The fix requires two changes:
- Add
clerkUIVersionto thegetSafeEnvreturn object inpackages/astro/src/server/get-safe-env.ts - Forward it to
clerkUIScriptUrlinpackages/astro/src/server/build-clerk-hotload-script.ts:37-42
🛠️ Proposed changes
In packages/astro/src/server/get-safe-env.ts:
clerkJsVersion: getContextEnvVar('PUBLIC_CLERK_JS_VERSION', context),
clerkUIUrl: getContextEnvVar('PUBLIC_CLERK_UI_URL', context),
+ clerkUIVersion: getContextEnvVar('PUBLIC_CLERK_UI_VERSION', context),
prefetchUI: getContextEnvVar('PUBLIC_CLERK_PREFETCH_UI', context) === 'false' ? false : undefined,In packages/astro/src/server/build-clerk-hotload-script.ts (lines 37-42):
const clerkUiScriptSrc = clerkUIScriptUrl({
clerkUIUrl: env.clerkUIUrl,
+ clerkUIVersion: env.clerkUIVersion,
domain,
proxyUrl,
publishableKey,
});🤖 Prompt for AI Agents
In `@packages/astro/src/server/build-clerk-hotload-script.ts` around lines 37 -
42, The Astro hotload script builder is not passing the extracted
PUBLIC_CLERK_UI_VERSION through, so pinned UI versions are ignored; update
getSafeEnv to include clerkUIVersion in its returned object (extracting
PUBLIC_CLERK_UI_VERSION into clerkUIVersion) and then forward that value into
the clerkUIScriptUrl call (the clerkUiScriptSrc creation) by adding
clerkUIVersion to the argument list so clerkUIScriptUrl can select the correct
UI package version.
70808d1 to
66b6ddf
Compare
66b6ddf to
5cff9a0
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: 2
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/rspack.config.js (1)
283-297: AddLimitChunkCountPluginto force the native build into a single chunk.Dynamic imports exist in production code (e.g.,
moduleManager.ts,clerk.ts) which would create separate chunks despitesplitChunks: false. React Native cannot load those additional chunks at runtime. Match the pattern used for other single-bundle variants:Proposed fix
{ output: { publicPath: '', }, + plugins: [ + new rspack.optimize.LimitChunkCountPlugin({ + maxChunks: 1, + }), + ], optimization: { splitChunks: false, }, },
🤖 Fix all issues with AI agents
In `@packages/astro/src/env.d.ts`:
- Around line 8-9: Add the missing PUBLIC_CLERK_UI_VERSION declaration to the
InternalEnv type: update the InternalEnv interface/type (where
PUBLIC_CLERK_UI_URL and PUBLIC_CLERK_PREFETCH_UI are declared) to include a
readonly PUBLIC_CLERK_UI_VERSION?: string; so TypeScript knows this env var
exists and the use in get-safe-env.ts compiles cleanly.
In `@packages/astro/src/server/get-safe-env.ts`:
- Around line 35-37: Add the missing PUBLIC_CLERK_UI_VERSION declaration to the
InternalEnv type in env.d.ts so the usage in get-safe-env.ts (specifically the
clerkUIVersion key and getContextEnvVar('PUBLIC_CLERK_UI_VERSION', context)) has
a matching type; update the InternalEnv interface/type to include
PUBLIC_CLERK_UI_VERSION with the appropriate string (or optional string) type to
resolve the TS2345 build error.
🧹 Nitpick comments (1)
packages/shared/src/loadClerkJsScript.ts (1)
277-280: TODO comment should be addressed.Line 278 contains a TODO comment questioning whether this delegation is needed. This should be resolved before merging or tracked in an issue.
Do you want me to open an issue to track this TODO, or should it be addressed in this PR?
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 511-518: The function getClerkUiEntryChunk currently returns
undefined early when shouldPrefetchClerkUI(this.options.prefetchUI) is false, so
any provided this.options.clerkUICtor is ignored; reorder the checks in
getClerkUiEntryChunk so you first return this.options.clerkUICtor if present,
and only consult shouldPrefetchClerkUI afterwards. Apply the same reorder in
create-clerk-instance (the equivalent initialization function there) so a
supplied clerkUICtor is honored even when prefetchUI is false.
♻️ Duplicate comments (2)
packages/astro/src/server/get-safe-env.ts (1)
35-37: EnsurePUBLIC_CLERK_UI_VERSIONis declared inInternalEnv.This concern was raised in a previous review. Line 36 references
'PUBLIC_CLERK_UI_VERSION'which must be added to theInternalEnvtype inpackages/astro/src/env.d.tsto avoid TypeScript errors (TS2345).packages/astro/src/server/build-clerk-hotload-script.ts (1)
37-43: Past review concern addressed.The
clerkUIVersionis now properly forwarded toclerkUIScriptUrlon line 39, resolving the previously flagged issue about pinned UI versions being ignored.
🧹 Nitpick comments (1)
packages/astro/src/server/build-clerk-hotload-script.ts (1)
9-13: Non-null assertions on potentially undefined values.The non-null assertions (
!) onenv.pk,env.proxyUrl, andenv.domaincould cause runtime issues if these environment variables are not configured. WhilepublishableKeyis typically required,proxyUrlanddomainare optional and commonly undefined.Consider removing the non-null assertions for optional values:
Suggested fix
- // eslint-disable-next-line `@typescript-eslint/no-non-null-assertion` - const publishableKey = env.pk!; - // eslint-disable-next-line `@typescript-eslint/no-non-null-assertion` - const proxyUrl = env.proxyUrl!; - // eslint-disable-next-line `@typescript-eslint/no-non-null-assertion` - const domain = env.domain!; + const publishableKey = env.pk ?? ''; + const proxyUrl = env.proxyUrl; + const domain = env.domain;
| private async getClerkUiEntryChunk(): Promise<ClerkUiConstructor | undefined> { | ||
| if (!shouldPrefetchClerkUI(this.options.prefetchUI)) { | ||
| return undefined; | ||
| } | ||
|
|
||
| if (this.options.clerkUICtor) { | ||
| return this.options.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.
Honor provided clerkUICtor even when prefetchUI is false
Line 512–518 returns undefined before checking this.options.clerkUICtor, so consumers who supply a bundled UI ctor and set prefetchUI={false} lose the ctor and may fail to render UI without an extra script load. This is a functional regression. The same ordering issue exists in packages/astro/src/internal/create-clerk-instance.ts (Line 120–126).
Proposed fix (reorder checks)
- if (!shouldPrefetchClerkUI(this.options.prefetchUI)) {
- return undefined;
- }
-
- if (this.options.clerkUICtor) {
- return this.options.clerkUICtor;
- }
+ if (this.options.clerkUICtor) {
+ return this.options.clerkUICtor;
+ }
+
+ if (!shouldPrefetchClerkUI(this.options.prefetchUI)) {
+ return undefined;
+ }🤖 Prompt for AI Agents
In `@packages/react/src/isomorphicClerk.ts` around lines 511 - 518, The function
getClerkUiEntryChunk currently returns undefined early when
shouldPrefetchClerkUI(this.options.prefetchUI) is false, so any provided
this.options.clerkUICtor is ignored; reorder the checks in getClerkUiEntryChunk
so you first return this.options.clerkUICtor if present, and only consult
shouldPrefetchClerkUI afterwards. Apply the same reorder in
create-clerk-instance (the equivalent initialization function there) so a
supplied clerkUICtor is honored even when prefetchUI is false.
Summary
Fixes: USER-4231
This PR removes the
clerkJSVariant: 'headless'option and replaces it with a newprefetchUIprop that controls whether the@clerk/uiscript is prefetched.Changes
clerkJSVariant: 'headless'option from all SDK packagesprefetchUIprop (boolean | undefined) to control UI bundle prefetchingprefetchUI={false}- Disable prefetching the UI bundle (UI will still load on-demand when needed)prefetchUIomitted orundefined- Prefetch UI normally (default behavior)shouldPrefetchClerkUIhelper functionclerkJSVariant="headless"toprefetchUI={false}clerkUiCtor→clerkUICtorfor casing consistencyClerkJscasing in favor ofClerkJSfor consistencyloadClerkJsScript→loadClerkJSScriptclerkJsScriptUrl→clerkJSScriptUrlbuildClerkJsScriptAttributes→buildClerkJSScriptAttributessetClerkJsLoadingErrorPackageName→setClerkJSLoadingErrorPackageNameLoadClerkJsScriptOptions→LoadClerkJSScriptOptionsEnvironment Variables
You can disable UI prefetching via environment variable:
NEXT_PUBLIC_CLERK_PREFETCH_UI=falsePUBLIC_CLERK_PREFETCH_UI=falseCLERK_PREFETCH_UI=falseUsage
Packages Updated
@clerk/clerk-js(major)@clerk/shared(major)@clerk/react@clerk/nextjs@clerk/astro@clerk/nuxt@clerk/vue@clerk/react-router@clerk/tanstack-react-start@clerk/expo@clerk/chrome-extension@clerk/express@clerk/upgrade(codemod added)Checklist
pnpm buildpassesSummary by CodeRabbit
Breaking Changes
New Features
Chores
Tests / Docs
✏️ Tip: You can customize this high-level summary in your review settings.