fix: provide default empty array for watched form fields#5196
fix: provide default empty array for watched form fields#5196
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
perkinsjr
requested review from
Flo4604,
MichaelUnkey,
chronark,
imeyer,
mcstepp and
ogzhanolguncu
as code owners
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA single file update to the KeyRbacDialog component adds default empty array values to watched form fields. This prevents undefined access errors when computing effective permissions and in dependent hooks. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can get early access to new features in CodeRabbit.Enable the |
![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 the current code and only fix it if needed.
Inline comments:
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/index.tsx:
- Around line 106-107: The watched values returned by watch("roleIds", []) and
watch("directPermissionIds", []) can be non-array (null or other types) when
persisted state is corrupted, so normalize at read-time: read the raw value via
watch(...) into local variables (e.g., rawRoleIds, rawDirectPermissionIds) and
coerce to arrays using Array.isArray checks (e.g., const watchedRoleIds =
Array.isArray(rawRoleIds) ? rawRoleIds : []; similarly for
watchedDirectPermissionIds) and optionally map/validate element types if needed;
update any downstream code that uses .includes() or array spread to use these
normalized watchedRoleIds/watchedDirectPermissionIds so operations won’t throw
on malformed persisted state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 88872f19-2bc7-4bdd-8a87-006c4d2b30ef
📒 Files selected for processing (1)
web/apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/index.tsx
| const watchedRoleIds = watch("roleIds", []); | ||
| const watchedDirectPermissionIds = watch("directPermissionIds", []); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React Hook Form v7 watch method defaultValue parameter behavior when field value is null or invalid type
💡 Result:
In React Hook Form v7, watch(name, defaultValue) (and useWatch({ name, defaultValue })) uses defaultValue only as a fallback when the watched field is not yet set / returns undefined (commonly on the first render before the field is registered, or when a value doesn’t exist in form state). It’s not a general “nullish coalescing” fallback. [1] [2] [3]
So, for your cases:
- If the field value is
null:watch()returnsnull(because the field is set), and thedefaultValueparameter is not applied. RHF generally expects you to usenullor""as the “cleared” value rather thanundefined. [4] - If the field value is an “invalid type”:
watch()returns whatever is currently stored in RHF’s form state—it does not coerce types based ondefaultValue, and validation errors don’t change whatwatch()returns. (Type-safety is primarily a TypeScript concern at compile time, not runtime.) [1] [3] - If the field value is
undefined: RHF treats that like “not set” and you’ll observe fallback behavior to defaults (either thewatch/useWatchdefaultValue, oruseForm({ defaultValues })). Also, setting a field toundefinedeffectively can snap it back to its default. [1] [5]
Practical rule: if you need a fallback when a value is null, do it yourself: const v = watch('x'); const resolved = v ?? myFallback; (or handle null explicitly), because RHF’s defaultValue is primarily for the undefined/unregistered case. [1] [3]
Sources
[1] https://reacthookform.caitouyun.com/api/useform/watch
[2] https://react-form-hook.com/docs/usewatch
[3] https://app.unpkg.com/react-hook-form@7.70.0/files/dist/watch.d.ts
[4] https://react-form-hook.com/docs/usecontroller/controller
[5] https://github.com/orgs/react-hook-form/discussions/5858
🏁 Script executed:
head -150 "web/apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/index.tsx" | tail -100Repository: unkeyed/unkey
Length of output: 2907
🏁 Script executed:
wc -l "web/apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/index.tsx"Repository: unkeyed/unkey
Length of output: 214
🏁 Script executed:
rg "updateKeyRbacSchema" --type tsx --type ts -A 15 | head -50Repository: unkeyed/unkey
Length of output: 84
🏁 Script executed:
rg "usePersistedForm" --type tsx --type ts -B 3 -A 10 | head -80Repository: unkeyed/unkey
Length of output: 84
🏁 Script executed:
fd "edit-rbac" --type fRepository: unkeyed/unkey
Length of output: 213
🏁 Script executed:
cat -n "web/apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/hooks/use-edit-rbac.ts"Repository: unkeyed/unkey
Length of output: 3627
🏁 Script executed:
cat -n "web/apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/index.tsx" | head -150Repository: unkeyed/unkey
Length of output: 5871
🏁 Script executed:
fd "update-key-rbac.schema" --type fRepository: unkeyed/unkey
Length of output: 226
🏁 Script executed:
fd "use-persisted-form" --type fRepository: unkeyed/unkey
Length of output: 153
🏁 Script executed:
cat -n "web/apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/update-key-rbac.schema.ts"Repository: unkeyed/unkey
Length of output: 597
🏁 Script executed:
cat -n "web/apps/dashboard/hooks/use-persisted-form.tsx"Repository: unkeyed/unkey
Length of output: 3627
Normalize array fields at read-time to handle corrupted persisted state
The watch(..., []) defaultValue only applies when a field is undefined. If persisted state contains malformed values (null, non-array types), watch() will return the corrupted value, and downstream operations (Line 113: .includes(), Line 116: spread) will fail or misbehave. Since usePersistedForm directly restores JSON without schema validation, add runtime normalization when accessing these watched values.
Normalize array fields
- const watchedRoleIds = watch("roleIds", []);
- const watchedDirectPermissionIds = watch("directPermissionIds", []);
+ const normalizeStringArray = (value: unknown): string[] =>
+ Array.isArray(value) ? value.filter((item): item is string => typeof item === "string") : [];
+
+ const watchedRoleIds = normalizeStringArray(watch("roleIds"));
+ const watchedDirectPermissionIds = normalizeStringArray(watch("directPermissionIds"));This aligns with the coding guideline to parse inputs at boundaries into typed structures, preventing illegal states from propagating through the component logic.
📝 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 watchedRoleIds = watch("roleIds", []); | |
| const watchedDirectPermissionIds = watch("directPermissionIds", []); | |
| const normalizeStringArray = (value: unknown): string[] => | |
| Array.isArray(value) ? value.filter((item): item is string => typeof item === "string") : []; | |
| const watchedRoleIds = normalizeStringArray(watch("roleIds")); | |
| const watchedDirectPermissionIds = normalizeStringArray(watch("directPermissionIds")); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@web/apps/dashboard/app/`(app)/[workspaceSlug]/apis/[apiId]/keys/[keyAuthId]/_components/components/table/components/actions/components/edit-rbac/index.tsx
around lines 106 - 107, The watched values returned by watch("roleIds", []) and
watch("directPermissionIds", []) can be non-array (null or other types) when
persisted state is corrupted, so normalize at read-time: read the raw value via
watch(...) into local variables (e.g., rawRoleIds, rawDirectPermissionIds) and
coerce to arrays using Array.isArray checks (e.g., const watchedRoleIds =
Array.isArray(rawRoleIds) ? rawRoleIds : []; similarly for
watchedDirectPermissionIds) and optionally map/validate element types if needed;
update any downstream code that uses .includes() or array spread to use these
normalized watchedRoleIds/watchedDirectPermissionIds so operations won’t throw
on malformed persisted state.
2 participants
What does this PR do?
This PR fixes an issue where
watchedDirectPermissionIdswas non-iterable becauseusePersistedFormloaded corrupted state, causing a spread operator failure.To address this, an empty array
[]was added as the default value forwatch("roleIds")andwatch("directPermissionIds")to ensure these fields are always arrays. This prevents potential issues when these form fields are uninitialized or have no selected values.Fixes UNKEY-DASHBOARD-3Y
Type of change
How should this be tested?
To test this, ensure that form fields
roleIdsanddirectPermissionIdsare always treated as arrays, even when uninitialized or having no selected values. Specifically, verify that operations expecting iterable values onwatchedDirectPermissionIds(and potentiallywatchedRoleIds) no longer fail due to non-iterable types. This can be reproduced by scenarios whereusePersistedFormmight load corrupted or unexpected state for these fields.Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated