-
Notifications
You must be signed in to change notification settings - Fork 431
feat(ui): enforce minimum clerk-js version compatibility #7667
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?
feat(ui): enforce minimum clerk-js version compatibility #7667
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
f6578d0 to
682ea78
Compare
682ea78 to
a033dda
Compare
Add runtime version check in ClerkUi constructor to detect incompatible @clerk/clerk-js versions. In development instances, logs a warning; in production, throws ClerkRuntimeError with actionable upgrade guidance.
a033dda to
d1d402b
Compare
| @@ -1,3 +1,5 @@ | |||
| export const MIN_CLERK_JS_VERSION = '5.112.0'; | |||
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.
Is 5.112.0 the correct version here or should this be pinned to 6.0.0 since thats the major for core 3? The tests seem to use 6.0.0 as the expected minimum which made me wonder if this was intentional or if the constant just needs updating.
| logger.warnOnce(incompatibilityMessage); | ||
| } else { | ||
| throw new ClerkRuntimeError(incompatibilityMessage, { code: 'clerk_ui_version_mismatch' }); | ||
| } |
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.
Should this be the other way around? Typically we want to fail fast in development (so devs catch issues early) but be more lenient in production (to avoid breaking user-facing apps). The current behavior warns in dev (easy to miss) but throws in prod (could cause outages).
| `@clerk/ui@${ClerkUi.version} requires @clerk/clerk-js@>=${MIN_CLERK_JS_VERSION}, ` + | ||
| `but found an incompatible version${clerkVersion ? ` (${clerkVersion})` : ''}. ` + | ||
| `Please upgrade @clerk/clerk-js (or your framework SDK) to a compatible version.`; | ||
| } |
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 at clerk-js, it always instantiates ClerkUi with new ModuleManager(), and clerkVersion comes from the Clerk instance which should always have its version set. Is there a scenario where ClerkUi would be instantiated without a moduleManager or without knowing the clerkVersion? If this is just defensive coding thats fine, but if theres an actual use case Im curious what it is :)
|
This is fine |
Add runtime version check in ClerkUi constructor to detect incompatible @clerk/clerk-js versions. In development instances, logs a warning; in production, throws ClerkRuntimeError with actionable upgrade guidance.
Also moves shared version utilities to @clerk/shared/utils.
Description
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change