fix(populate): make `refPath` work as a function by AbdelrahmanHafez · Pull Request #16035 · Automattic/mongoose

AbdelrahmanHafez · 2026-02-20T03:47:54Z

While working on refPath as a function, I found two behavior gaps:

In document assignment/casting ($set / create), function refPath was not handled correctly for the "assign a document to an ObjectId ref" flow, leading to confusing cast errors (for example, path.split is not a function wrapped in CastError).
In populate, function refPath was called with options.path (for example notifications.target) instead of an indexed path per subdoc (for example notifications.0.target).

That mismatch meant array subdocs could not reliably resolve per-element model paths, and $set vs populate could disagree.

What this PR changes:

Unifies function refPath call semantics across assignment/casting and populate.
Keeps the established design from Run refPath and ref functions on subdocument being populated rather than top level doc #8469 (comment):
- this is the subdocument/document currently being processed.
- returned refPath is interpreted as root-relative.
For array subdocs, passes root-relative indexed path to function refPath (for example notifications.0.target, channels.1.notifications.0.target).
Uses the same indexed path format in both $set and populate flows.
Throws MongooseError (instead of leaking TypeError) when function refPath returns a non-string.
Preserves discriminator behavior by normalizing numeric segments only for internal discriminator schema traversal (not for user-facing path passed to refPath).

I chose to always use indexed paths because assignment/casting needs a concrete element path to resolve the correct model for that exact subdocument.
If populate uses an unindexed path while $set uses indexed resolution, behavior diverges.
Using indexed root-relative paths everywhere gives one consistent contract and ensures function refPath computes the same result in both flows.
This passes all the current tests since the existing tests relied on regex replacements like path.replace(/\.item$/, '.kind');

Given how broken refPath currently is, I think it's safe to introduce this change in a minor version

I tested this feature extensively with different scenarios:

top-level docs
subdoc arrays
deeply nested arrays
lean()
doc.populate()
shared schema reused across multiple arrays
single nested subdocs
parity of path argument between $set and populate
explicit non-string function refPath error handling in both assignment and populate.

Additionally, I updated populate docs to clarify function refPath contract, root-relative indexed path behavior, and the array-subdocument pattern.

In particular, for array subdocs, static string refPath like items.targetModel is insufficient when per-element resolution is needed. The recommended pattern is function refPath, for example:

refPath: function(doc, path) {
  return path.replace(/\.target$/, '.targetModel');
}

…le parent arrays

…ulation

…amic references in arrays re #16028

Copilot

Pull request overview

This PR fixes and clarifies support for function-style refPath across both document assignment/casting ($set / create) and populate(), ensuring consistent (doc, path) semantics (including indexed paths for array subdocs) and improving error handling when refPath returns invalid values.

Changes:

Aligns refPath function invocation semantics between assignment/casting and populate (correct this, doc, and root-relative indexed path).
Improves error reporting by throwing MongooseError when a function refPath returns a non-string, rather than leaking TypeError.
Adds extensive regression tests and updates populate documentation to codify the function refPath contract.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/model.populate.test.js	Adds comprehensive populate coverage for function `refPath`, including indexed paths in arrays and lean/doc.populate parity.
test/document.test.js	Adds assignment/casting (`$set` / `create`) coverage for function `refPath`, including non-string return error handling.
lib/helpers/populate/modelNamesFromRefPath.js	Ensures non-string `refPath` values throw a `MongooseError` early with a clear message.
lib/helpers/populate/getModelsMapForPopulate.js	Updates populate model resolution for function `refPath` to use indexed paths per subdoc and avoids discriminator traversal issues with numeric segments.
lib/document.js	Fixes assignment/casting flow to call function `refPath` correctly (with root-relative indexed path) and validates return type.
docs/populate.md	Documents the function `refPath` contract (root-relative `path`, indexed array paths, consistent behavior across assignment and populate).

lib/helpers/populate/getModelsMapForPopulate.js

… with map subdocuments re #16028

hasezoey

LGTM, some minor nitpicks.

hasezoey · 2026-02-20T10:13:39Z

lib/helpers/populate/getModelsMapForPopulate.js

 const StrictPopulate = require('../../error/strictPopulate');
+const numericPathSegmentPattern = '\\.\\d+(?=\\.|$)';


Maybe there should be a empty line in between (as the first part are just imports)

hasezoey · 2026-02-20T10:23:55Z

lib/helpers/populate/getModelsMapForPopulate.js

+          for (const key of Object.keys(source)) {
+            const value = source[key];


Is this faster than just using Object.entries()?

hasezoey · 2026-02-20T10:29:04Z

test/document.test.js

+        it('should work with string refPath when assigning a document (baseline)', async function() {
+          // Arrange
+          const { User, ActivityLog } = createTestContext({ refPath: 'targetModel' });
+          const user = await User.create({ name: 'Hafez' });
+
+          // Act
+          const activityLog = await ActivityLog.create({ targetModel: 'User', targetUser: user });
+          const activityLogFromDb = await ActivityLog.findById(activityLog._id).populate('targetUser');
+
+          // Assert
+          assert.equal(activityLogFromDb.targetUser.name, 'Hafez');
+        });


If this is the baseline, shouldnt it be ordered before the other tests?

…ray and use faster string comparison

vkarpov15

I made a couple of minor improvements, mostly perf related, but LGTM overall thanks.

Reading through this PR made me just want to get rid of refPath() function support though - we added that in #6683 but I find the refPath() function API to be inferior to ref() function, which we added even earlier (at least #6554) . WDYT?

vkarpov15 · 2026-02-22T19:57:28Z

lib/document.js

      }
-      const modelName = val.get(refPath);
+
+      if (typeof refPath === 'function' && !refPath[modelSymbol]) {


What's the reason for this refPath[modelSymbol] check here?

vkarpov15 · 2026-02-22T20:02:28Z

lib/helpers/populate/getModelsMapForPopulate.js

+  let normalizedRefPath = null;
+  const schemaPath = schema?.path;
+
+  if (schemaPath != null && populatePath.endsWith('.' + schemaPath)) {


For performance reasons, we should avoid using startsWith() and endsWith(), in favor of charAt() and slice()

vkarpov15 · 2026-02-22T20:09:36Z

lib/helpers/populate/getModelsMapForPopulate.js

+ *
+ * @param {Function} refPath
+ * @param {Document|Object} doc
+ * @param {SchemaType} schema


Going forward, we should make sure we do not name SchemaType variables schema, that is confusing since a Schema is a distinct class. I know this is far from the only place where we use schema to refer to a SchemaType, but I would like to change that going forward.

AbdelrahmanHafez added 6 commits February 20, 2026 02:52

test(document): assert refPath works properly as a function re #16028

c69655f

test(document): finalize refPath function spec

ab16162

test: enhance population tests for indexed paths with lean and multip…

ed9c9d4

…le parent arrays

test: add validation for non-string refPath in document and model pop…

065d17d

…ulation

fix(document): fix refPath for populate, and $set re #16028

e07a9d3

docs(populate): enhance refPath functionality with examples for dyn…

a5b13bd

…amic references in arrays re #16028

AbdelrahmanHafez requested a review from Copilot February 20, 2026 04:10

Copilot started reviewing on behalf of AbdelrahmanHafez February 20, 2026 04:11 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

lib/helpers/populate/getModelsMapForPopulate.js Outdated Show resolved Hide resolved

AbdelrahmanHafez added 2 commits February 20, 2026 05:46

test(populate): add test for function refPath in populating $* path…

d006b7c

… with map subdocuments re #16028

fix(populate): supprt map paths with refPath as function

0c9e28b

AbdelrahmanHafez requested review from hasezoey and vkarpov15 February 20, 2026 04:54

AbdelrahmanHafez marked this pull request as ready for review February 20, 2026 04:54

hasezoey reviewed Feb 20, 2026

View reviewed changes

perf: consolidate _getSubdocsWithIndexedPaths to avoid unnecessary ar…

3e61ae9

…ray and use faster string comparison

vkarpov15 approved these changes Feb 22, 2026

View reviewed changes

fix tests

5676472

vkarpov15 merged commit 36b0729 into master Feb 22, 2026
52 checks passed

vkarpov15 added this to the 9.2.2 milestone Feb 22, 2026

AbdelrahmanHafez deleted the codex/gh-16028-codex branch February 23, 2026 05:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(populate): make `refPath` work as a function#16035

fix(populate): make `refPath` work as a function#16035
vkarpov15 merged 10 commits intomasterfrom
codex/gh-16028-codex

AbdelrahmanHafez commented Feb 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

hasezoey left a comment

Uh oh!

hasezoey Feb 20, 2026

Uh oh!

hasezoey Feb 20, 2026

Uh oh!

hasezoey Feb 20, 2026

Uh oh!

vkarpov15 left a comment

Uh oh!

vkarpov15 Feb 22, 2026

Uh oh!

vkarpov15 Feb 22, 2026

Uh oh!

vkarpov15 Feb 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		const StrictPopulate = require('../../error/strictPopulate');
		const numericPathSegmentPattern = '\\.\\d+(?=\\.\|$)';

		for (const key of Object.keys(source)) {
		const value = source[key];

Uh oh!

Conversation

AbdelrahmanHafez commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

hasezoey left a comment

Choose a reason for hiding this comment

Uh oh!

hasezoey Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

hasezoey Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

hasezoey Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

vkarpov15 left a comment

Choose a reason for hiding this comment

Uh oh!

vkarpov15 Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

vkarpov15 Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

vkarpov15 Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AbdelrahmanHafez commented Feb 20, 2026 •

edited

Loading