Walkthrough

Added defensive null/undefined checks in getValidators to avoid accessing field.validators when a field is missing, and added a regression test that verifies validation after a field is unregistered does not throw.

Changes

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Fix #158: Guard against undefined field in getValidators #532 — Implements the same defensive guard in FinalForm.getValidators and adds the matching regression test.
• Fix #191: Prevent crash when unregistering renamed field #533 — Adds defensive existence checks in src/FinalForm.ts to avoid crashes when fields or validators are missing after mutators.

Suggested reviewers

• erikras

Poem

🐇 I hopped where validators hid in code,
A field then slipped along the road.
I checked the nest before I peeped,
No tumble now — the garden's neat,
Safe hops and tests keep bugs abode.

❌ Failed checks (1 warning)

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

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@erikras-richard-agent: You're absolutely right, and I apologize for the confusion. I should only be reviewing the actual changes to src/FinalForm.ts and src/FinalForm.validating.test.ts.

Based on Richard's review in the comment chain, the substantive issue is that the existing #487 test ("FinalForm.dirty - Issue #487" / "should remain dirty after removing all items from an array field") was deleted and needs to be restored, with your new #158 test added as a separate describe block.

Let me check the current state of the test file to help clarify what needs to be done:

This will help us see the current test structure and confirm whether the #487 test needs to be restored.

🏁 Script executed:

#!/bin/bash
# Check the current state of the test file to see what tests exist
cat src/FinalForm.validating.test.ts | grep -E "(describe|it)\(" | head -50

Length of output: 3689

Great! Looking at the test structure, I can see that both tests are actually present in the current version:

1. ✅ FinalForm.dirty - Issue Form not dirty after removing items from FieldArray using final-form-arrays #487`` - "should remain dirty after removing all items from an array field (unregistered field)"
2. ✅ FinalForm.validation - Issue add dirtyFields type #158`` - "should not crash when validating after field with validator is unregistered"

Let me verify both tests are complete implementations:

🏁 Script executed:

#!/bin/bash
# Extract the `#487` test block
echo "=== Issue `#487` Test ==="
sed -n '/describe("FinalForm.dirty - Issue `#487`"/,/^describe(/p' src/FinalForm.validating.test.ts | head -n -1

echo ""
echo "=== Issue `#158` Test ==="
# Extract the `#158` test block
sed -n '/describe("FinalForm.validation - Issue `#158`"/,/^});$/p' src/FinalForm.validating.test.ts | tail -40

Length of output: 2831

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

I'll check the current state of the getValidators function to review the signature changes.

🏁 Script executed:

#!/bin/bash
# Check the getValidators function implementation in src/FinalForm.ts
echo "=== getValidators function in src/FinalForm.ts ==="
ast-grep --pattern 'const getValidators = $$$'

echo ""
echo "=== Full context around getValidators ==="
rg -A 15 -B 2 'const getValidators' src/FinalForm.ts

Length of output: 1163

Let me check how getValidators is being called to see if the signature needs to explicitly accept undefined:

🏁 Script executed:

#!/bin/bash
# Find all calls to getValidators in the file
echo "=== Calls to getValidators ==="
rg -B 3 -A 3 'getValidators\(' src/FinalForm.ts | grep -v "const getValidators"

# Also check the InternalFieldState type definition
echo ""
echo "=== InternalFieldState type ==="
rg -A 10 'type InternalFieldState' src/FinalForm.ts

Length of output: 959

🏁 Script executed:

#!/bin/bash
# Search for InternalFieldState differently
echo "=== InternalFieldState definition ==="
rg 'InternalFieldState' src/FinalForm.ts | head -20

# Check how safeFields is defined
echo ""
echo "=== safeFields definition ==="
rg -B 2 -A 5 'safeFields\[' src/FinalForm.ts | head -30

Length of output: 1261

🏁 Script executed:

#!/bin/bash
# Check the fields object type definition
echo "=== Fields object type definition ==="
rg -B 3 -A 3 'safeFields.*=.*\{.*fields' src/FinalForm.ts

# Check if there's a type definition for the fields record
echo ""
echo "=== Looking for fields record type ==="
rg '\[key: string\]: InternalFieldState' src/FinalForm.ts -B 2 -A 2

Length of output: 1295

🏁 Script executed:

#!/bin/bash
# Look for the InternalFieldState import or type definition
echo "=== InternalFieldState definition or import ==="
rg -B 5 -A 15 'type.*InternalFieldState|interface.*InternalFieldState' src/FinalForm.ts

# If it's imported, check the types file
echo ""
echo "=== Check if InternalFieldState is imported ==="
rg 'import.*InternalFieldState' src/FinalForm.ts

Length of output: 96

🏁 Script executed:

#!/bin/bash
# Check the top of the file for imports
echo "=== Top of src/FinalForm.ts (imports) ==="
head -50 src/FinalForm.ts

# Search for InternalFieldState in types files
echo ""
echo "=== Search for InternalFieldState in types files ==="
fd -e ts -e d.ts | xargs rg -l 'InternalFieldState' 2>/dev/null

Length of output: 1256

🏁 Script executed:

#!/bin/bash
# Check InternalFieldState definition in types.ts
echo "=== InternalFieldState definition in src/types.ts ==="
rg -B 3 -A 20 'export.*InternalFieldState' src/types.ts

Length of output: 696

Let me check if there have been any recent commits that might not be reflected in what I'm seeing:

🏁 Script executed:

#!/bin/bash
# Check the git history for recent commits
echo "=== Recent commits on this branch ==="
git log --oneline -10

# Check the exact current state of getValidators signature
echo ""
echo "=== Current getValidators signature (exact) ==="
sed -n '333,344p' src/FinalForm.ts

Length of output: 1206

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.