Skip to content

fix(map): support ObjectId keys in MongooseMap#has() - #16634

Merged
vkarpov15 merged 1 commit into
Automattic:masterfrom
NgoQuocViet2001:fix-map-has-objectid-key
Nov 1, 2026
Merged

vkarpov15 merged 1 commit into
Automattic:masterfrom
NgoQuocViet2001:fix-map-has-objectid-key

Conversation

@NgoQuocViet2001

Copy link
Copy Markdown
Contributor

Summary

MongooseMap#get(), set() and delete() convert ObjectId keys to strings (added in #9938 for #9936), and the set() docs say maps support strings and ObjectIds as keys. has() is not overridden, so the native Map#has looks up the ObjectId instance itself and returns false for a key that get() finds:

const userId = new mongoose.Types.ObjectId();
doc.votes.set(userId, 1);
doc.votes.get(userId); // 1
doc.votes.has(userId); // false, expected true

This PR adds a has() override that converts ObjectId keys the same way get() does, then calls super.has(). String keys are unchanged.

Examples

Added a test in test/types.map.test.js.

  • Ran: npx mocha --exit test/types.map.test.js (65 passing). The new test fails on master with false !== true on has(userId).
  • Ran: the other test files that use maps (document, model, populate, query, schema, versioning): 2641 passing, 0 failing.
  • Checked: npx eslint lib/types/map.js test/types.map.test.js is clean.

Copilot AI 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.

🟡 Changes recommended

TypeScript declarations still restrict Map.has() to string keys, preventing typed callers from using the new runtime ObjectId-key support and requiring an updated signature with a regression test.

1 open issue
What changed in this PR

This PR makes MongooseMap#has() recognize ObjectId keys, matching the key conversion already used by get(), set(), and delete().

Changes:

  • Converts ObjectId keys to strings before checking map membership.
  • Adds regression coverage for existing, missing, newly inserted, and deleted keys, while checking string-key behavior.
File Description
test/​types.map.test.js Tests ObjectId membership checks and string-key compatibility.
lib/​types/​map.js Adds and documents the has() override.

Review effort: Balanced

Comment thread lib/types/map.js Resolved
@vkarpov15 vkarpov15 added this to the 9.11.5 milestone Nov 1, 2026

@vkarpov15 vkarpov15 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks 👍

@vkarpov15
vkarpov15 merged commit 807bf8d into Automattic:master Nov 1, 2026
32 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