Skip to content

Conversation

@nikospapcom
Copy link
Contributor

@nikospapcom nikospapcom commented Jan 7, 2025

Description

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:

After

Screenshot 2025-01-07 at 5 35 15 PM

@nikospapcom nikospapcom self-assigned this Jan 7, 2025
@changeset-bot
Copy link

changeset-bot bot commented Jan 7, 2025

🦋 Changeset detected

Latest commit: 4c5fe2f

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

This PR includes changesets to release 23 packages
Name Type
@clerk/localizations Patch
@clerk/clerk-js Patch
@clerk/types Patch
@clerk/clerk-react Patch
@clerk/ui Patch
@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/nextjs Patch
@clerk/nuxt Patch
@clerk/react-router Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
@clerk/shared Patch
@clerk/tanstack-start Patch
@clerk/testing Patch
@clerk/themes 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 7, 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 8, 2025 10:16am

Comment on lines 10 to 23
export const web3CallbackErrorHandler: Web3CallbackErrorHandler = (err, setError) => {
if (
isClerkAPIResponseError(err) &&
err.errors[0].meta?.paramName === 'identifier' &&
err.errors[0].code === 'form_param_nil'
) {
const error = new ClerkRuntimeError('Please install a Web3 wallet.', {
code: 'web3_missing_identifier',
});

return handleError(error, [], setError);
}
return handleError(err, [], setError);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We introduced web3CallbackErrorHandler and avoid to throw an error in getWeb3Identifier and generateWeb3Signature because we don't want to introduce a break change

const { provider } = params;
const ethereum = await getEthereumProvider(provider);

// TODO: Improve error handling for the case when the provider is not found
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a TODO for core-3

Copy link
Member

Choose a reason for hiding this comment

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

Can we include something in the todo comment that indicates it's a breaking change for core 3? 🤔

const { identifier, nonce, provider } = params;
const ethereum = await getEthereumProvider(provider);

// TODO: Improve error handling for the case when the provider is not found
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a TODO for core-3

@nikospapcom nikospapcom marked this pull request as ready for review January 7, 2025 16:08
err.errors[0].meta?.paramName === 'identifier' &&
err.errors[0].code === 'form_param_nil'
) {
const error = new ClerkRuntimeError('Please install a Web3 wallet.', {
Copy link
Member

Choose a reason for hiding this comment

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

Should we mention the browser?

Suggested change
const error = new ClerkRuntimeError('Please install a Web3 wallet.', {
const error = new ClerkRuntimeError('Please install a Web3 wallet in your browser.', {

err.errors[0].meta?.paramName === 'identifier' &&
err.errors[0].code === 'form_param_nil'
) {
const error = new ClerkRuntimeError('Please install a Web3 wallet.', {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can make the error a bit more user friendly

Suggested change
const error = new ClerkRuntimeError('Please install a Web3 wallet.', {
const error = new ClerkRuntimeError('A Web3 Wallet extension cannot be found. Please install one to continue.', {

passkey_pa_not_supported: 'Registration requires a platform authenticator but the device does not support it.',
passkey_registration_cancelled: 'Passkey registration was cancelled or timed out.',
passkey_retrieval_cancelled: 'Passkey verification was cancelled or timed out.',
web3_missing_identifier: 'Please install a Web3 wallet.',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
web3_missing_identifier: 'Please install a Web3 wallet.',
web3_missing_identifier: 'A Web3 Wallet extension cannot be found. Please install one to continue.',

@nikospapcom nikospapcom force-pushed the nikospap/user-1143-preview-an-error-if-no-web3-wallet-is-present-and-installed branch from 6e6903e to 697cdcb Compare January 8, 2025 10:12
@nikospapcom nikospapcom force-pushed the nikospap/user-1143-preview-an-error-if-no-web3-wallet-is-present-and-installed branch from 697cdcb to 4c5fe2f Compare January 8, 2025 10:13
@nikospapcom nikospapcom merged commit 80e1117 into main Jan 8, 2025
29 checks passed
@nikospapcom nikospapcom deleted the nikospap/user-1143-preview-an-error-if-no-web3-wallet-is-present-and-installed branch January 8, 2025 10:30
jakobevangelista pushed a commit that referenced this pull request Jan 9, 2025
wobsoriano pushed a commit that referenced this pull request Feb 8, 2025
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