fix(ui): Add missing noop methods to unsafeMetadata field in SignUpStart#8500
fix(ui): Add missing noop methods to unsafeMetadata field in SignUpStart#8500wobsoriano merged 4 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 1f95f07 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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 |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe SignUpStartInternal code now constructs unsafeMetadata as a FormControlState-shaped object (id, value, and no-op clearFeedback, setValue, onChange, setError) before appending it to fieldsToSubmit and calling buildRequest. A regression test was added that injects unsafeMetadata, mocks signUp.create to reject with a ClerkAPIResponseError, renders SignUpStart with a ticket URL, and asserts signUp.create is called without the component throwing. A .changeset patch entry was added for Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui/src/components/SignUp/__tests__/SignUpStart.test.tsx`:
- Line 492: The test uses a weak assertion
(expect(fixtures.signUp.create).toHaveBeenCalled()) which can pass before the
CardStateProvider's async error-handling (and the clearFeedback TypeError) runs;
update the test to assert a post-error side effect instead—either waitFor a
rendered error state that only appears after clearFeedback runs, or spy on
CardStateProvider's noop clearFeedback and assert that spy was called (and did
not throw); alternatively wrap the render in a helper that surfaces unhandled
promise rejections so failures in the async effect fail the test. Use
identifiers fixtures.signUp.create, CardStateProvider, and clearFeedback to
locate the code to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5e794e81-be7a-4db1-832e-ebbee8ca8ac1
📒 Files selected for processing (2)
.changeset/wild-insects-design.mdpackages/ui/src/components/SignUp/__tests__/SignUpStart.test.tsx
✅ Files skipped from review due to trivial changes (1)
- .changeset/wild-insects-design.md
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Addressed CR's comment 👍🏼 Added a commit that verifies removing the fix causes the test to fail |
3 participants
Description
Fixes
TypeError: e.clearFeedback is not a functionwhen<SignUp unsafeMetadata={...} />is used and a Clerk API error is thrown. TheunsafeMetadatasynthetic field pushed tofieldsToSubmitwas missingclearFeedbackandsetErrorstubs, unlike every other synthetic field in the same function.Fixes #8496 #3996
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change