fix(schema): make hydrate() virtuals option work with parent-level dotted virtuals - #16396
Draft
AbdelrahmanHafez wants to merge 5 commits into
Draft
fix(schema): make hydrate() virtuals option work with parent-level dotted virtuals#16396AbdelrahmanHafez wants to merge 5 commits into
AbdelrahmanHafez wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Model.hydrate()’s virtuals: true option so it works with parent-declared dotted virtuals (for example parentSchema.virtual('items.detail')) when hydrating embedded documents that initialize against their child schemas.
Changes:
- Mirror parent-level dotted regular virtuals onto the corresponding embedded schema when the dotted path traverses:
- document arrays
- single nested subdocuments
- maps of subdocuments (
$*)
- Avoid clobbering child schema paths/virtuals by only mirroring when the child schema reports the path as
adhocOrUndefined. - Add tests covering document arrays, single nested subdocs, map subdocs, and the non-regression cases (no
virtualsoption, child conflicts).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
lib/schema.js |
Mirrors parent-level dotted regular virtuals onto embedded schemas so hydrate(..., { virtuals: true }) can preserve/set values on subdocuments. |
test/model.test.js |
Adds coverage for hydration behavior across doc arrays, single nested subdocs, and map subdocs, plus conflict/non-regression checks. |
…ted dotted virtual (gh-15627)
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.
re #15627 follow up to #15638
The main example in the feature request declares the virtual on the parent schema:
The
virtualsoption forhydrate()only handled virtuals declared directly on the child schema. Dotted regular virtuals stay registered on the parent schema only, and since document arrays, single nested subdocuments, and map entries init against their child schemas, the raw virtual value got stripped during hydration.This PR mirrors dotted regular virtuals onto the embedded schema in
Schema.prototype.virtual(), the same way dotted populate virtuals are already mirrored (the gh-8198 workaround a few lines above). It applies to document arrays, single nested subdocuments, and maps of subdocuments (entries.$*.detail), and skips any name that already resolves on the child schema, so real child paths and virtuals declared directly on the child keep working unchanged.Tests cover the three shapes, plus the defaults:
hydrate()without the option still leaves the value unset, and declaring a parent dotted virtual that conflicts with a real child path is still rejected.Edit: the new mirror duplicated the path walk that the gh-8198 populate workaround already does, so I extracted a shared
getEmbeddedSchemaVirtualTarget()helper and pointed both branches at it. While unifying them I found the populate walk throws a TypeError when a dotted populate virtual is declared under a map of primitives (of: Stringhas no child schema), and that it stops at the first missing path segment, so it never mirrored virtuals declared under plain nested objects likenested.children.bar. The shared helper fixes both. There's new coverage for the crash and for hydrating parent virtuals under nested objects, plus tests pinning that populate virtuals under nested objects keep working and that dotted virtuals under a map without$*are still rejected, since map keys are real paths.