fix: compare own enumerable properties on Date, RegExp, and boxed primitives#111
Open
spokodev wants to merge 1 commit into
Open
fix: compare own enumerable properties on Date, RegExp, and boxed primitives#111spokodev wants to merge 1 commit into
spokodev wants to merge 1 commit into
Conversation
…mitives Date, RegExp, and boxed-primitive (String/Number/Boolean) instances were compared only by their internal value (valueOf/toString), so two such objects with differing own enumerable data properties were reported as deeply equal. This contradicts the documented rule that all own and inherited enumerable properties are considered (only Error is exempt) and diverges from node's util.isDeepStrictEqual. AND the type-specific value check with the existing objectEqual own-key comparison so extra own properties are taken into account while valueOf, -0/NaN, and the Error special-case semantics are preserved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
deep-eqlignores own enumerable properties onDate,RegExp, and boxed-primitive (String/Number/Boolean) instances. Two such objects are reported deeply equal even when they carry differing own enumerable data properties.Same for
RegExp(const r = /x/; r.foo = ...) and boxednew Number(1)/new String(...)/new Boolean(...).This contradicts the documented contract in the README "Rules" section:
Only
Erroris documented as an exception. The control case is already correct (eql({foo:1}, {foo:2}) === false), which shows that only the special-cased types bypass the rule.Root cause
In
extensiveDeepEqualByType(), theString/Number/Boolean/Datebranch returns only thevalueOf()comparison, and theRegExpbranch returns only thetoString()comparison. Bothreturnbefore theobjectEqual()default branch, so own enumerable properties are never compared.Fix
AND the type-specific value check with the existing
objectEqual()own-enumerable-key comparison for those branches.objectEqual()already returnstruewhen both sides have zero own enumerable keys, so plain instances are unaffected.valueOf/-0/NaNsemantics and the documentedErrorspecial-case are untouched.Verification
Red -> green: 5 new tests (one per affected type) assert that instances with differing own enumerable properties are not equal. They fail before the fix and pass after.
Each new case was cross-checked against
node:util.isDeepStrictEqualas an oracle, including the cases that must stay equal (same prop value, no extra props,new Number(NaN),new Date(NaN)) and theErrorspecial-case (extra own props still ignored, as documented).Full suite green: 179 passing, 0 failing. Lint clean.