Skip to content

fix(document): avoid setting _skipMarkModified when setting nested path with merge option #15484

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

Merged
merged 1 commit into from
Jun 22, 2025

Conversation

vkarpov15
Copy link
Collaborator

Fix #11913

Summary

Looks like this _skipMarkModified option isn't necessary - it was intended to prevent marking subpaths as modified before when we added the merge option. It was introduced here: 98150ac

Examples

@vkarpov15 vkarpov15 added this to the 8.16.1 milestone Jun 20, 2025
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 fixes issue #11913 by removing the unnecessary _skipMarkModified flag when setting nested paths with the merge option. The changes include a new test case to verify that only the intended subpaths are marked modified and an update in the document set logic to remove the _skipMarkModified option.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/document.test.js Adds a test case to validate proper modified path behavior.
lib/document.js Removes the _skipMarkModified flag in nested path setting.
Comments suppressed due to low confidence (2)

test/document.test.js:14450

  • [nitpick] Consider adding a short inline comment in the test to clarify why the modifiedPaths are expected to include both the parent ('name') and the nested ('name.first') keys, which will help future maintainers understand the expected behavior for the merge option.
    human.set(h, null, null, { merge: true });

lib/document.js:1117

  • The removal of the _skipMarkModified flag is intentional per the issue, but ensure that this change doesn't lead to unintended marking of unchanged subpaths. It would be beneficial to double-check that the merge option behavior remains consistent in all nested update scenarios.
        this.$set(pathName, valForKey, constructing, options);

@vkarpov15 vkarpov15 merged commit 62d1349 into master Jun 22, 2025
71 checks passed
@vkarpov15 vkarpov15 deleted the vkarpov15/gh-11913 branch June 22, 2025 17:42
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.

Unmodified fields being marked as modified.
2 participants