Skip to content

fix(document): clean modified subpaths if unsetting map #15520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vkarpov15
Copy link
Collaborator

Fix #15519

Summary

If setting a map to null/undefined, need to clean modified subpaths. Otherwise we have dangling modified paths which cause issues like #15519.

Examples

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that when a Map path is unset (assigned null or undefined), any modified subpaths are also cleared to prevent dangling change records. It adds the cleanup logic in Document.prototype.$set and a new test case verifying this behavior.

  • Added cleanModifiedSubpaths call for schema maps when unsetting a path in lib/document.js.
  • Introduced a test in test/types.map.test.js for setting then unsetting the same map (GH-15519).

Reviewed Changes

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

File Description
lib/document.js Call cleanModifiedSubpaths for schema.$isSchemaMap when val == null.
test/types.map.test.js New test "handles setting then unsetting the same map" covering the undefined case.
Comments suppressed due to low confidence (2)

test/types.map.test.js:1368

  • Consider adding a similar test case for doc.settings = null to ensure that unsetting the map to null also clears modified subpaths as expected.
    doc.settings = undefined;

lib/document.js:1520

  • [nitpick] Add a brief comment explaining why we need to clean modified subpaths for schema maps when unsetting (e.g., to avoid dangling change records), so future maintainers understand the intent.
  } else if (schema.$isSchemaMap && val == null) {

@@ -1517,6 +1517,8 @@ Document.prototype.$set = function $set(path, val, type, options) {

if (schema.$isSingleNested && (this.isDirectModified(path) || val == null)) {
cleanModifiedSubpaths(this, path);
} else if (schema.$isSchemaMap && val == null) {
Copy link
Preview

Copilot AI Jul 7, 2025

Choose a reason for hiding this comment

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

[nitpick] The two branches for schema.$isSingleNested and schema.$isSchemaMap both call cleanModifiedSubpaths when val == null. Consider merging them into a single condition like if ((schema.$isSingleNested || schema.$isSchemaMap) && val == null) to reduce duplication.

Copilot uses AI. Check for mistakes.

@vkarpov15 vkarpov15 added this to the 8.16.3 milestone Jul 7, 2025
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.

Adding followed by removing a map results in conflict error
1 participant