Skip to content

Conversation

@panteliselef
Copy link
Member

@panteliselef panteliselef commented Jan 20, 2025

Description

image

[Keyless]

For Developer who use <ClerkProvider/> as a Server component (not in a "use client" boundary), we can detect when the explicitly set keys match the ones created from keyless. When that is the case, we are now passing a server action to the Clerk UI resulting in the keyless prompt to display a confirmation step which forces the developer to dismiss.

Behind the scenes, on dismiss, we are are deleting the .clerk/ directory as it is no longer necessary. That resets the application and opts it out of keyless.

Unfortunately developers who are using <ClerkProvider/> will not get this confirmation step, but instead the prompt will simply disappear.

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@panteliselef panteliselef self-assigned this Jan 20, 2025
@changeset-bot
Copy link

changeset-bot bot commented Jan 20, 2025

🦋 Changeset detected

Latest commit: 1b36a4a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 22 packages
Name Type
@clerk/clerk-js Minor
@clerk/nextjs Minor
@clerk/types Minor
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/elements Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/localizations Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/clerk-react Patch
@clerk/remix Patch
@clerk/shared Patch
@clerk/tanstack-start Patch
@clerk/testing Patch
@clerk/themes Patch
@clerk/ui Patch
@clerk/vue Patch

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

@vercel
Copy link

vercel bot commented Jan 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clerk-js-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 21, 2025 0:41am

Comment on lines +99 to +103
if (runningWithClaimedKeys) {
output = clientProvider;
} else {
output = <KeylessCookieSync {...newOrReadKeys}>{clientProvider}</KeylessCookieSync>;
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to keep rendering the Keyless prompt which is controlled by the props passed to ClientClerkProvider but we don't want the rest of the app to act like it was on keyless since the developer has now set the keys explicitly.

@panteliselef panteliselef requested a review from a team January 20, 2025 11:56
@panteliselef panteliselef marked this pull request as ready for review January 20, 2025 11:56
@panteliselef panteliselef requested a review from LekoArts January 20, 2025 16:07
@panteliselef panteliselef requested a review from a team January 20, 2025 16:07
Comment on lines 88 to 90
__internal_claimKeylessApplicationUrl: newOrReadKeys.claimUrl,
__internal_copyInstanceKeysUrl: newOrReadKeys.apiKeysUrl,
__internal_keyless_dismissPrompt: runningWithClaimedKeys ? deleteKeylessAction : undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these props are related to keyless, but they use different naming schemes. Can we make them consistent? Proposal:

Suggested change
__internal_claimKeylessApplicationUrl: newOrReadKeys.claimUrl,
__internal_copyInstanceKeysUrl: newOrReadKeys.apiKeysUrl,
__internal_keyless_dismissPrompt: runningWithClaimedKeys ? deleteKeylessAction : undefined,
__internal_keyless_claimApplicationUrl: newOrReadKeys.claimUrl,
__internal_keyless_copyInstanceKeysUrl: newOrReadKeys.apiKeysUrl,
__internal_keyless_dismissPrompt: runningWithClaimedKeys ? deleteKeylessAction : undefined,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, seems like i never pushed this. Thanks for reminding me.

Co-authored-by: Bryce Kalow <bryce@clerk.dev>
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending Bryce's comment (https://github.com/clerk/javascript/pull/4940/files#r1922640604), should be done before merge

@clerk clerk deleted a comment from clerk-cookie Jan 21, 2025
…essage-when-env-keys-and-local

# Conflicts:
#	packages/clerk-js/src/ui/components/KeylessPrompt/index.tsx
#	packages/types/src/clerk.ts
@panteliselef panteliselef enabled auto-merge (squash) January 21, 2025 12:39
@panteliselef panteliselef merged commit 4427c47 into main Jan 21, 2025
29 checks passed
@panteliselef panteliselef deleted the elef/actls-65-nextjs-show-claiming-success-message-when-env-keys-and-local branch January 21, 2025 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants