-
Notifications
You must be signed in to change notification settings - Fork 357
fix(clerk-js): Pass metadata as variables for error localization #6167
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?
fix(clerk-js): Pass metadata as variables for error localization #6167
Conversation
🦋 Changeset detectedLatest commit: 61a42ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 Git ↗︎
|
📝 Walkthrough## Walkthrough
A new changeset file was added to document a patch update for the `@clerk/clerk-js` package, describing changes related to passing metadata as variables for error localization. In the source code, the `translateError` function within `useLocalizations` was updated to extract additional metadata fields (`identifiers` and `emailAddresses`) from the error's `meta` object. These fields are used to construct a variables object, which is then passed to the translation function to support dynamic token replacement in localized error messages.
## Suggested labels
`clerk-js`
## Suggested reviewers
- aeliox
- tmilewski 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
🧹 Nitpick comments (1)
packages/clerk-js/src/ui/localization/makeLocalizable.tsx (1)
92-92
: Consider adding type safety for destructured metadata fields.The destructuring assumes
identifiers
andemailAddresses
exist in the meta object, but there's no validation of their structure or type.-const { paramName = '', identifiers, emailAddresses } = meta || {}; +const { paramName = '', identifiers = [], emailAddresses = [] } = meta || {};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/twenty-games-repair.md
(1 hunks)packages/clerk-js/src/ui/localization/makeLocalizable.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/clerk-js/src/ui/localization/makeLocalizable.tsx (1)
101-102
: LGTM! Metadata variables are correctly passed to localization function.The implementation correctly passes the metadata variables to both the parameterized and non-parameterized error key lookups, enabling dynamic token replacement in localized error messages as intended.
.changeset/twenty-games-repair.md (1)
1-6
: LGTM! Changeset follows standard format and accurately describes the changes.The changeset correctly identifies this as a patch-level change for
@clerk/clerk-js
and provides a clear, concise description of the enhancement.
|
||
if (!code) { | ||
return ''; | ||
} | ||
|
||
const metaArgsAsVariables = { identifiers: identifiers?.[0], emailAddresses: emailAddresses?.[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.
🛠️ Refactor suggestion
Handle edge cases for array access and improve variable naming.
The current implementation has several potential issues:
- No validation that
identifiers
andemailAddresses
are arrays - Silent failure if arrays are empty (returns
undefined
) - Variable names don't match the original field names, which could cause confusion
-const metaArgsAsVariables = { identifiers: identifiers?.[0], emailAddresses: emailAddresses?.[0] };
+const metaArgsAsVariables = {
+ identifiers: Array.isArray(identifiers) && identifiers.length > 0 ? identifiers[0] : undefined,
+ emailAddresses: Array.isArray(emailAddresses) && emailAddresses.length > 0 ? emailAddresses[0] : undefined,
+};
Alternatively, if you want to maintain consistency with the original field names from the PR objectives:
-const metaArgsAsVariables = { identifiers: identifiers?.[0], emailAddresses: emailAddresses?.[0] };
+const metaArgsAsVariables = {
+ identifiers: Array.isArray(identifiers) && identifiers.length > 0 ? identifiers[0] : undefined,
+ email_addresses: Array.isArray(emailAddresses) && emailAddresses.length > 0 ? emailAddresses[0] : undefined,
+};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const metaArgsAsVariables = { identifiers: identifiers?.[0], emailAddresses: emailAddresses?.[0] }; | |
const metaArgsAsVariables = { | |
identifiers: Array.isArray(identifiers) && identifiers.length > 0 | |
? identifiers[0] | |
: undefined, | |
emailAddresses: Array.isArray(emailAddresses) && emailAddresses.length > 0 | |
? emailAddresses[0] | |
: undefined, | |
}; |
🤖 Prompt for AI Agents
In packages/clerk-js/src/ui/localization/makeLocalizable.tsx at line 98, the
code accesses the first element of identifiers and emailAddresses without
checking if they are arrays or if they contain elements, which can cause silent
failures or errors. To fix this, add validation to confirm identifiers and
emailAddresses are arrays and have at least one element before accessing the
first item. Also, rename the variables to match the original field names for
clarity and consistency.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
@@ -89,15 +89,17 @@ export const useLocalizations = () => { | |||
} | |||
|
|||
const { code, message, longMessage, meta } = (error || {}) as ClerkAPIError; | |||
const { paramName = '' } = meta || {}; | |||
const { paramName = '', identifiers, emailAddresses } = meta || {}; |
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.
Per the example, not_allowed_access: '{{identifiers}} is not allowed to access this page!!!!!'
- would this be confusing there are multiple identifiers
(or emailsAddresses
) returned.
Description
When using localization on
unstable_errors
we pass no variables, although in some cases FAPI returns values in themeta
property. This PR allows to use the following 2 identifiers that are included in the error object coming from FAPI in the localized error message:identifiers
: This is an array of identifiersemail_addresses
: This is an array of email addressesExample error from FAPI:
Example localization value provided by the user:
Resulting in the following error:
Checklist
pnpm test
runs as expected.pnpm build
runs as expected.Type of change
Summary by CodeRabbit