-
Notifications
You must be signed in to change notification settings - Fork 426
feat(ui,react): Add shared React variant to reduce bundle size #7601
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: c84b0ef 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.
|
📝 WalkthroughWalkthroughAdds a shared React build variant for 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
@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: |
Introduces a "shared" variant of @clerk/ui that externalizes React dependencies, allowing the host application's React to be reused instead of bundling a separate copy. Changes: - Add @clerk/ui/register module to register React on globalThis - Add ui.shared.browser.js build variant with externalized React - Add React version compatibility checking in @clerk/react - Add clerkUiVariant option to load the appropriate variant - Make dev server React externalization conditional via --env shared Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Extract version checking logic into testable utility (versionCheck.ts) - Add comprehensive unit tests for version parsing and bounds checking - Move version compatibility check to module level for better performance - Add dev warning when pnpm-workspace.yaml fallback is used - Add warning for React version mismatch in register modules - Remove redundant clerkUiVariant assignment in isomorphicClerk.ts Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The new exports field in clerk-js/package.json blocked deep imports that expo and chrome-extension packages depended on. This adds: - ./internal/fapi export for FapiRequestInit/FapiResponse types (expo) - ./no-rhc export for the no-RHC variant (chrome-extension) Also updates expo to use the new clean import path. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename register/index.js to register/index.cjs to fix module syntax mismatch (package uses "type": "module" but file was CommonJS) - Add register/index.d.cts for CJS type declarations to fix "Masquerading as ESM" attw error - Update exports to point to correct file extensions - Run pnpm dedupe to clean up lockfile Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The new exports field introduces ESM entry points (.mjs) with CJS type declarations (.d.ts), which triggers attw's "Masquerading as CJS" warning. This is expected behavior for this package's build setup, so we ignore the false-cjs rule. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This reverts commit 74be850.
…rhc variant" This reverts commit 364059b.
f50f25d to
d7c670c
Compare
Add required curly braces after if conditions in versionCheck.ts and fix import sorting in index.ts. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
bratsos
left a 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.
I haven't run it locally yet, but I did a deep dive in the code and it looks good! That's a great optimization 🚀
I'll run it locally on Monday if it's not merged yet as a sanity check.
|
Love it! Are we tracking the routing logic for this variant in the sdk-infra-workers repo? |
|
@jacekradko nope, I'll take a look! |
|
@jacekradko On second thought, I'm not sure we need additional logic in the proxy...we don't need/want the ability to pin specific domains to this variant necessarily, it's something that's internal to the SDK |
|
!snapshot |
|
Hey @brkalow - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.2.9-snapshot.v20260117034135 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/fastify@2.6.9-snapshot.v20260117034135 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260117034135 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260117034135 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260117034135 --save-exact |
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/ui/rspack.config.js`:
- Around line 20-36: The externals handler sharedReactExternalsHandler currently
externalizes 'react' and 'react-dom' but misses subpaths like 'react-dom/client'
and 'react-dom/server', so update the function (the sharedReactExternalsHandler
that takes ({ request }, callback)) to also detect requests equal to
'react-dom/client' and 'react-dom/server' and call callback(null,
['__clerkSharedModules', 'react-dom/client'], 'root') and callback(null,
['__clerkSharedModules', 'react-dom/server'], 'root') respectively (keeping the
existing branches for 'react' and 'react-dom' and leaving callback() as the
default).
Externalize react-dom/client in the shared variant and register it on globalThis.__clerkSharedModules so host apps can share it with @clerk/ui. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.
Really nice work here! I tried the current (outdated) snapshot out with the dashboard a bit and it seems to work well.
ui.browser.js in the dash shrank from 36.1kb to 19.4kb which is a nice win. The rabbit might have a point about react-dom/client though, which might possibly move the needle a bit more?
Update: Oh, I had totally missed this was already addressed and Coderabbit just missed picking up on it. Will do a new snapshot and try it out.
|
!snapshot |
|
Hey @Ephem - the snapshot version command generated the following package versions:
Tip: Use the snippet copy button below to quickly install the required packages. npm i @clerk/agent-toolkit@0.2.9-snapshot.v20260121133427 --save-exact
npm i @clerk/astro@3.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/backend@3.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/chrome-extension@3.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/clerk-js@6.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/dev-cli@1.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/expo@3.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/expo-passkeys@1.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/express@2.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/fastify@2.6.9-snapshot.v20260121133427 --save-exact
npm i @clerk/localizations@4.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/msw@0.0.1-snapshot.v20260121133427 --save-exact
npm i @clerk/nextjs@7.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/nuxt@2.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/react@6.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/react-router@3.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/shared@4.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/tanstack-react-start@1.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/testing@2.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/ui@1.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/upgrade@2.0.0-snapshot.v20260121133427 --save-exact
npm i @clerk/vue@2.0.0-snapshot.v20260121133427 --save-exact |
|
Hmm, trying out this very latest snapshot, I'm not seeing any bundle size gains in a local prod build of the dashboard. Logging // This comes from logging it in the build:
const bounds: VersionBounds[] = [
[ 18, 0, -1, 0 ],
[ 19, 0, 0, 3 ],
[ 19, 1, 1, 4 ],
[ 19, 2, 2, 3 ],
[ 19, 3, 3, 0 ]
];
it('returns true for compatible versions', () => {
expect(isVersionCompatible('19.3.0-canary-f93b9fd4-20251217', bounds)).toBe(true);
});Not sure what's going on, I might very well be doing something wrong. 🤔 |
| type VersionBounds = [major: number, minMinor: number, maxMinor: number, minPatch: number]; | ||
|
|
||
| // Read supported React range from pnpm-workspace.yaml catalogs and parse into bounds | ||
| function getClerkUiSupportedReactBounds(): VersionBounds[] { |
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.
I don't love the logic here, doing some iteration...
…pendencies - Remove hardcoded fallback version range in tsup.config.ts - Start from package.json peerDependencies.react to determine version range - If it's a catalog reference (catalog:XXX), resolve it from pnpm-workspace.yaml - If it's already a version range, parse it directly - Extract parseRangeToBounds to build-utils/ for testability - Add comprehensive tests for version range parsing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
ui.shared.browser.jsbuild variant that externalizes React dependencies, allowing the host application's React to be reused instead of bundling a separate copy@clerk/ui/registermodule to register React onglobalThis.__clerkSharedModulesfor sharing with@clerk/uiclerkUiVariantoption to explicitly control which variant to use@clerk/reactusers, the shared variant is automatically detected and enabled for compatible React versionsfixes USER-4440
Test plan
@clerk/reactpackage@clerk/react🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.