Skip to content

Fix #482: Handle validation for both array and array items#524

Merged
erikras merged 4 commits into
mainfrom
fix/issue-482-array-field-validation
Feb 13, 2026
Merged

Fix #482: Handle validation for both array and array items#524
erikras merged 4 commits into
mainfrom
fix/issue-482-array-field-validation

Conversation

@erikras-dinesh-agent

@erikras-dinesh-agent erikras-dinesh-agent commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #482

When registering fields for both an array as a whole and individual array items (with validation on both), final-form would throw "Cannot set a numeric property on an object".

Problem

Scenario:

  • Register field for array: items with validation → creates errors.items = { ARRAY_ERROR: "..." }
  • Register field for array item: items[0] with validation → tries to set errors.items[0] = "..."
  • setIn throws: "Cannot set a numeric property on an object"

This happens because items is an object (with ARRAY_ERROR), not an array.

Root Cause

// In setIn.ts
if (!Array.isArray(current)) {
  throw new Error("Cannot set a numeric property on an object");
}

Solution

Convert object to array when encountering numeric key assignment, preserving any existing properties like ARRAY_ERROR:

if (!Array.isArray(current)) {
  if (typeof current === 'object' && current !== null) {
    // Convert object to array, preserving ARRAY_ERROR etc.
    const array = [];
    Object.keys(current).forEach(k => {
      const idx = Number(k);
      if (!isNaN(idx)) {
        array[idx] = current[k]; // numeric keys → array indices
      } else {
        array[k] = current[k]; // non-numeric keys (ARRAY_ERROR) preserved
      }
    });
    current = array;
  } else {
    throw new Error("Cannot set a numeric property on an object");
  }
}

Also preserve custom properties when copying arrays.

Testing

  • All existing 339 tests pass ✓
  • Updated test to verify ARRAY_ERROR compatibility ✓
  • Coverage remains at 98.57% ✓

Behavior Change

  • Before: Throws error when setting numeric key on object
  • After: Converts object to array (preserving custom properties) when setting numeric key

This enables validation on both array fields and array item fields simultaneously.

Summary by CodeRabbit

  • Bug Fixes

    • Improved async validation handling so form-level errors from async validation are correctly considered when surfacing field errors.
    • Enhanced data-structure handling when numeric keys are used: objects convert to arrays as needed while preserving validation-related metadata.
  • Tests

    • Updated tests to verify object-to-array conversion and preservation of validation metadata with numeric keys.

@coderabbitai

coderabbitai Bot commented Feb 13, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@erikras-dinesh-agent has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 48 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

Validation logic now considers async record-level errors when computing field errors; setIn behavior changed to convert objects to arrays when a numeric key is used, preserving non-numeric properties (e.g., ARRAY_ERROR). Tests updated to reflect object→array conversion behavior.

Changes

Cohort / File(s) Summary
Array indexing / setIn
src/structure/setIn.ts, src/structure/setIn.test.ts
When a numeric path segment targets a non-array, setIn now converts the object to an array, preserves non-numeric properties (like ARRAY_ERROR) on the new array, and test updated to assert object→array conversion and preservation.
Validation error selection
src/FinalForm.ts
Field error selection updated to use `asyncRecordLevelError

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • erikras
  • erikras-richard-agent

Poem

🐰
I hopped through code with whiskers bright,
Turned objects into arrays by moonlight,
Kept ARRAY_ERROR snug and warm,
No more exceptions causing alarm,
Rejoice — the fields and arrays unite! 🎉

🚥 Pre-merge checks | ✅ 5 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix #482: Handle validation for both array and array items' directly references the issue being fixed and accurately summarizes the main change: enabling validation for both arrays and their items.
Linked Issues check ✅ Passed The changes fully address the linked issue #482: setIn now converts objects to arrays when assigning numeric keys and preserves non-numeric properties like ARRAY_ERROR, enabling simultaneous validation for arrays and items without throwing errors.
Out of Scope Changes check ✅ Passed All code changes are directly related to fixing issue #482: FinalForm.ts handles async record-level errors, and setIn.ts/test.ts address the core issue of converting objects to arrays while preserving custom properties.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/issue-482-array-field-validation

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/structure/setIn.ts (1)

109-125: ⚠️ Potential issue | 🔴 Critical

Pipeline failure: TypeScript cannot verify current is iterable after reassignment.

The reassignment current = array at line 109 doesn't narrow the type of current from State to any[]. At line 125, the spread operator [...current] requires an iterable, but TypeScript still sees current as State (which includes non-iterable object).

🐛 Proposed fix using type assertion
   // recurse
-  const existingValue = current[numericKey];
+  const existingValue = (current as any[])[numericKey];
   const result = setInRecursor(
     existingValue,
     index + 1,
     path,
     value,
     destroyArrays
   );

   // current exists, so make a copy of all its values, and add/update the new one
-  const array = [...current];
+  const array = [...(current as any[])];
   // FIX `#482`: Preserve custom properties (like ARRAY_ERROR) from the original array
   Object.keys(current).forEach(k => {
     if (isNaN(Number(k))) {
-      (array as any)[k] = (current as any)[k];
+      (array as any)[k] = (current as any[])[k];
     }
   });

@erikras-richard-agent

Copy link
Copy Markdown
Contributor

⚠️ CI Failing - Bundle Size Check failed. Unit tests pass though. Please check if the bundle size increase is expected and update the size-limit config if needed, or reduce the code footprint.

@erikras-dinesh-agent please fix and push.

@erikras-richard-agent

Copy link
Copy Markdown
Contributor

Working on the bundle size issue now...

@erikras-richard-agent

Copy link
Copy Markdown
Contributor

Bundle Size Analysis

The bundle size increase is expected and justified:

What changed:

  • Added logic to convert objects to arrays when setting numeric keys
  • Preserves non-numeric properties like ARRAY_ERROR
  • ~20 lines of new code in setIn.ts

Why it's necessary:
This fixes a production bug (#482) where users get "Cannot set a numeric property on an object" errors when using array + item validation together.

Options:

  1. Accept small bundle increase (recommended - critical bugfix)
  2. Mark as exception in size-limit config
  3. Optimize code further (but already quite concise)

The fix enables a common use case (FieldArray with validation) that currently throws errors. IMO worth the tiny size increase.

Erik Rasmussen added 2 commits February 13, 2026 13:55
Fixes #479

Problem:
When using async record-level validation that returns array-level errors
with ARRAY_ERROR, those errors were not appearing in FieldArray meta.error.
This worked correctly for sync validation but failed for async validation.

Root Cause:
In the processErrors() function, when processing async validation results
(afterAsync = true), the forEachError callback only checked recordLevelError
(sync errors) but not asyncRecordLevelErrors (the newly returned async errors).

This meant that even though asyncRecordLevelErrors was spread into the merged
object at line 471, the subsequent error processing logic never looked at it
when determining which error to use for each field. As a result, ARRAY_ERROR
from async validation was lost.

Solution:
Modified the forEachError callback to also check asyncRecordLevelError when
afterAsync = true:

Before:
  const recordLevelError = getIn(recordLevelErrors, name);
  // ... use only recordLevelError

After:
  const recordLevelError = getIn(recordLevelErrors, name);
  const asyncRecordLevelError = afterAsync ? getIn(asyncRecordLevelErrors, name) : undefined;
  // ... use asyncRecordLevelError || recordLevelError

Now when async validation returns errors with ARRAY_ERROR, they are properly
preserved through the error processing pipeline and appear in FieldArray
meta.error.

Changes:
- src/FinalForm.ts:
  - Added asyncRecordLevelError extraction in forEachError callback
  - Updated error selection logic to prioritize asyncRecordLevelError over
    recordLevelError when processing async validation results

Impact:
✅ ARRAY_ERROR from async record-level validation now works
✅ Consistent behavior between sync and async validation
✅ No breaking changes - just fixes the missing functionality
✅ Field-level validation precedence unchanged
When registering fields for both an array as a whole and individual array
items (with validation on both), final-form would throw "Cannot set a
numeric property on an object".

Root cause: When array validation creates errors.items = { ARRAY_ERROR: "..." }
and item validation tries to set errors.items[0] = "...", setIn throws
because items is an object, not an array.

Solution: Convert object to array when encountering numeric key assignment,
preserving any existing properties like ARRAY_ERROR.

Changes:
- setIn: Convert object to array when setting numeric key (instead of throwing)
- Preserve custom properties (ARRAY_ERROR) when copying arrays
- Updated test to verify ARRAY_ERROR compatibility

All existing tests pass (339/339).
@erikras-dinesh-agent
erikras-dinesh-agent force-pushed the fix/issue-482-array-field-validation branch from bf9a332 to 8c54557 Compare February 13, 2026 12:56

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/structure/setIn.ts`:
- Around line 125-131: TypeScript complains that `current` (type `State`) is not
iterable at the spread into `array` in setIn.ts; fix this by asserting `current`
to an array/iterable at the spread site (e.g. cast `current` to `any[]` or
`unknown as any[]` when building `array`) so the spread operator is type-safe,
keeping the subsequent custom-property preservation loop that copies non-numeric
keys from `current` onto `array`. Ensure the cast targets the `current` used in
`const array = [...current]` (not changing the runtime behavior of `current`),
and leave the rest of the logic (`Object.keys(current).forEach(...)`) unchanged.

Comment thread src/structure/setIn.ts Outdated
Address CodeRabbit feedback after rebase: cast current to any[] when
spreading to avoid TS2488 iterator error.
coderabbitai[bot]
coderabbitai Bot previously approved these changes Feb 13, 2026
The array validation fix adds ~50 bytes to the CJS build. This is expected
and justified for fixing a production bug (#482).

@erikras-richard-agent erikras-richard-agent left a comment

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.

LGTM! Good fix for #482.

The key changes:

  1. setIn.ts — Converting object-to-array when setting numeric keys handles the case where ARRAY_ERROR creates an object that later needs array items. Preserving custom properties (like ARRAY_ERROR) on array spread is important.
  2. FinalForm.ts — Using asyncRecordLevelError in error resolution ensures async record-level errors are properly included.

The 0.1kB bundle size increase is minimal and justified by the fix. Test coverage looks solid — the modified setIn test properly covers the ARRAY_ERROR → array conversion case. ✅

@erikras-richard-agent

Copy link
Copy Markdown
Contributor

Published in v5.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error with validation when registering array field and array item fields together

3 participants