Skip to content

fix(array): compare dates by value in indexOf() so pull() removes them (backport #16535 to 8.x) - #16590

Merged
vkarpov15 merged 1 commit into
Automattic:8.xfrom
mrityunjay-pandey:backport-8x/array-indexof-dates
Oct 16, 2026
Merged

vkarpov15 merged 1 commit into
Automattic:8.xfrom
mrityunjay-pandey:backport-8x/array-indexof-dates

Conversation

@mrityunjay-pandey

Copy link
Copy Markdown

Summary

Backport of #16535 to 8.x.

MongooseArray#pull() finds matches with MongooseArray#indexOf(), which compares with ==. Two Date objects are only == when they are the same instance, so on 8.x pulling a date by value leaves it in the local array even though the $pullAll removes it on save. If the array is also pushed to before saving, the atomics fall back to $set and the pulled date is written back.

The fix on master makes indexOf() compare dates by timestamp, the same way addToSet() already does. It cherry-picks cleanly onto 8.x (git cherry-pick -x 3b76fce84), no changes needed.

Examples

The tests added in #16535 are included. On current 8.x (8.24.5) the matches dates by value test fails; with this change it passes.

Ran locally on Linux, Node 24: test/types.array.test.js (67 passing), the full npm test suite (4045 passing, 0 failing, 50 pending; plain 8.x gives 4043 passing), and ESLint on the changed files.

MongooseArray#pull() finds matches with MongooseArray#indexOf(), which
compares with `==`. Two Date objects are only `==` when they are the
same instance, so pulling a date left it in the local array while the
$pullAll still removed it on save. If the array was also pushed to
before saving, the atomics fall back to $set and the pulled date was
written back.

addToSet() already compares dates by timestamp; do the same in
indexOf(), which also makes includes() work for dates.

(cherry picked from commit 3b76fce)
@vkarpov15 vkarpov15 added this to the 8.24.6 milestone Oct 16, 2026
@vkarpov15
vkarpov15 merged commit 48b8af0 into Automattic:8.x Oct 16, 2026
48 checks passed
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.

3 participants