Skip to content

chore(nextjs): Display keyless prompt until .clerk/ is removed - #4940

Merged
panteliselef merged 15 commits into
mainfrom
elef/actls-65-nextjs-show-claiming-success-message-when-env-keys-and-local
Jan 21, 2025
Merged

panteliselef merged 15 commits into
mainfrom
elef/actls-65-nextjs-show-claiming-success-message-when-env-keys-and-local

Conversation

@panteliselef

@panteliselef panteliselef commented Jan 20, 2025 •

Copy link
Copy Markdown
Contributor

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

changeset-bot Bot commented Jan 20, 2025 •

Copy link
Copy Markdown

🦋 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

vercel Bot commented Jan 20, 2025 •

Copy link
Copy Markdown

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
Copy Markdown
Contributor 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
Comment thread packages/nextjs/src/server/keyless-node.ts Outdated
@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 thread packages/nextjs/src/server/keyless-node.ts Outdated
Comment on lines +88 to +90
__internal_claimKeylessApplicationUrl: newOrReadKeys.claimUrl,
__internal_copyInstanceKeysUrl: newOrReadKeys.apiKeysUrl,
__internal_keyless_dismissPrompt: runningWithClaimedKeys ? deleteKeylessAction : undefined,

Copy link
Copy Markdown
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
Copy Markdown
Contributor 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.

Comment thread packages/nextjs/src/app-router/server/ClerkProvider.tsx
Co-authored-by: Bryce Kalow <bryce@clerk.dev>

@LekoArts LekoArts left a comment

Copy link
Copy Markdown
Contributor

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
@panteliselef
panteliselef deleted the elef/actls-65-nextjs-show-claiming-success-message-when-env-keys-and-local branch January 21, 2025 12:46

This branch was successfully deployed

1 active deployment
Preview — 1b36a4ad Deployed Jan 21, 2025 by vercel[bot]
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.

4 participants