fix(form-core): use equals() for value-type equality (Temporal, Decimal.js)

[mixelburg]

Fixes #2195

The evaluate function in form-core used key-based deep equality for all objects. This broke for value types like Temporal (TC39 Stage 4, now in browsers/Deno/Node) and Decimal.js, which have no enumerable properties — two instances with the same value would always be considered unequal.

This PR adds a duck-typed check: if both objects have the same constructor and the left-hand object has an equals() method, we delegate to it. This covers Temporal.PlainDate, Temporal.Instant, Temporal.Duration, Decimal.js, and similar value types.

The change is placed before the Map/Set checks and after the Date check, so more specific instanceof checks take precedence.

evaluate(Temporal.PlainDate.from('2026-01-01'), Temporal.PlainDate.from('2026-01-01'))
// was: false
// now: true

All 439 existing tests pass; 1 new test added.

Summary by CodeRabbit

• Bug Fixes

  • Improved equality comparison to properly handle custom objects with custom equality methods.

• Tests

  • Added test coverage for custom object equality comparison.

[coderabbitai]

📝 Walkthrough

Walkthrough

Updated the evaluate deep-equality helper in form-core to support custom object equality by detecting and delegating to an equals() method on objects with matching constructors, before falling back to built-in type comparisons. Added unit test validation for the new behavior.

Changes

Custom Object Equality Support

Layer / File(s)	Summary
Evaluate equals() method support and tests packages/form-core/src/utils.ts, packages/form-core/tests/utils.spec.ts	evaluate now detects equals methods on objects sharing the same constructor and delegates equality checking to them, enabling support for Temporal, Decimal.js, and similar value types. Unit test verifies the behavior for both equal and unequal instances.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🐰 A rabbit's hop through code so neat,
Custom equals make values complete!
Temporal dates now dance just right,
No more dirty checks in sight! ✨
Decimals too shall find their way—
Deep equality saves the day! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding support for equals() method for value-type equality in form-core.
Description check	✅ Passed	The PR description covers the problem, solution, and impact, but the checklist items are not marked as completed.
Linked Issues check	✅ Passed	The code changes fully address issue #2195 by implementing the duck-typed equals() check for value types like Temporal and Decimal.js.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the deep-equality logic in the evaluate function and adding corresponding test coverage.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

packages/form-core/tests/utils.spec.ts (1)

696-706: ⚡ Quick win

Consider expanding test coverage for edge cases.

The current test verifies the happy path (matching constructors, working equals() method). Consider adding test cases for:

• Objects with different constructors (should fall back to key-based equality)
• Interaction with other equals-implementing types to ensure the constructor check works correctly

This would increase confidence that the constructor guard prevents unintended cross-type comparisons.

📋 Suggested additional test cases

it('should not use equals() when constructors differ', () => {
  class TypeA {
    constructor(private val: number) {}
    equals(other: TypeA) {
      return this.val === other.val
    }
  }
  
  class TypeB {
    constructor(private val: number) {}
    equals(other: TypeB) {
      return this.val === other.val
    }
  }

  // Different constructors, should fall back to key-based equality
  // Since both have no enumerable properties, they appear equal by keys
  expect(evaluate(new TypeA(1), new TypeB(1))).toEqual(true)
  
  // But if we add a public property, the constructor check prevents 
  // equals() from being called inappropriately
  class TypeC {
    constructor(public val: number) {}
    equals(other: TypeC) {
      return this.val === other.val
    }
  }
  
  class TypeD {
    constructor(public val: number) {}
    equals(other: TypeD) {
      return this.val === other.val
    }
  }
  
  expect(evaluate(new TypeC(1), new TypeD(1))).toEqual(true) // same enumerable keys
  expect(evaluate(new TypeC(1), new TypeD(2))).toEqual(false) // different values
})

🤖 Prompt for 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.

In `@packages/form-core/tests/utils.spec.ts` around lines 696 - 706, Add tests
exercising cross-constructor behavior of evaluate: create two classes with
identical equals() implementations but different constructors (e.g., TypeA and
TypeB) and assert evaluate(new TypeA(1), new TypeB(1)) falls back to key-based
equality; then add classes with public/enumerable property (TypeC/TypeD) and
assert evaluate(new TypeC(1), new TypeD(1)) is true for same values and
evaluate(new TypeC(1), new TypeD(2)) is false, so the constructor guard
preventing cross-type equals() invocation and the key-based fallback are
covered.

packages/form-core/src/utils.ts (1)

442-447: 💤 Low value

Consider defensive check for objB.equals to prevent edge-case runtime errors.

While the constructor check ensures both objects are of the same type, a defensive programmer might delete or override the equals method on objB. Adding a check for typeof (objB as any).equals === 'function' would prevent potential runtime errors in pathological cases.

🛡️ Proposed defensive check

 if (
   typeof (objA as any).equals === 'function' &&
+  typeof (objB as any).equals === 'function' &&
   objA.constructor === objB.constructor
 ) {
   return (objA as any).equals(objB)
 }

🤖 Prompt for 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.

In `@packages/form-core/src/utils.ts` around lines 442 - 447, The block that calls
(objA as any).equals(objB) should first defensively verify that (objB as
any).equals is also a function to avoid runtime errors if objB has had equals
removed or overwritten; update the condition in the equality branch (where objA
and objB constructors are compared) to require typeof (objA as any).equals ===
'function' && typeof (objB as any).equals === 'function' before returning (objA
as any).equals(objB), referencing the existing objA, objB and equals symbols.

🤖 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.

Nitpick comments:
In `@packages/form-core/src/utils.ts`:
- Around line 442-447: The block that calls (objA as any).equals(objB) should
first defensively verify that (objB as any).equals is also a function to avoid
runtime errors if objB has had equals removed or overwritten; update the
condition in the equality branch (where objA and objB constructors are compared)
to require typeof (objA as any).equals === 'function' && typeof (objB as
any).equals === 'function' before returning (objA as any).equals(objB),
referencing the existing objA, objB and equals symbols.

In `@packages/form-core/tests/utils.spec.ts`:
- Around line 696-706: Add tests exercising cross-constructor behavior of
evaluate: create two classes with identical equals() implementations but
different constructors (e.g., TypeA and TypeB) and assert evaluate(new TypeA(1),
new TypeB(1)) falls back to key-based equality; then add classes with
public/enumerable property (TypeC/TypeD) and assert evaluate(new TypeC(1), new
TypeD(1)) is true for same values and evaluate(new TypeC(1), new TypeD(2)) is
false, so the constructor guard preventing cross-type equals() invocation and
the key-based fallback are covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3e6440c0-d12f-4eac-b77c-687989d21d7a

📥 Commits

Reviewing files that changed from the base of the PR and between 6a73479 and 0bc8606.

📒 Files selected for processing (2)

• packages/form-core/src/utils.ts
• packages/form-core/tests/utils.spec.ts