Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ export const KeyRbacDialog = ({
watch,
} = methods;

const watchedRoleIds = watch("roleIds");
const watchedDirectPermissionIds = watch("directPermissionIds");
const watchedRoleIds = watch("roleIds", []);
const watchedDirectPermissionIds = watch("directPermissionIds", []);
Comment on lines +106 to +107
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.

⚠️ Potential issue | 🟡 Minor

🧩 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() returns null (because the field is set), and the defaultValue parameter is not applied. RHF generally expects you to use null or "" as the “cleared” value rather than undefined. [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 on defaultValue, and validation errors don’t change what watch() 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 the watch/useWatch defaultValue, or useForm({ defaultValues })). Also, setting a field to undefined effectively 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 -100

Repository: 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 -50

Repository: unkeyed/unkey

Length of output: 84


🏁 Script executed:

rg "usePersistedForm" --type tsx --type ts -B 3 -A 10 | head -80

Repository: unkeyed/unkey

Length of output: 84


🏁 Script executed:

fd "edit-rbac" --type f

Repository: 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 -150

Repository: unkeyed/unkey

Length of output: 5871


🏁 Script executed:

fd "update-key-rbac.schema" --type f

Repository: unkeyed/unkey

Length of output: 226


🏁 Script executed:

fd "use-persisted-form" --type f

Repository: 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.

Suggested change
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.


// Calculate all effective permissions for GrantedAccess component
const allEffectivePermissionIds = useMemo(() => {
Expand Down
Loading