Skip to content
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

doc.isModified() does not work when a nested key is added to a document and the first level key itself does not exist #689

Open
s7dhansh opened this issue Oct 17, 2018 · 7 comments
Assignees
Labels

Comments

@s7dhansh
Copy link

doc.isModified('status.active') works when status exists as an object, and fails when status is undefined.
Is it intended? Seems to be a bug to me.

@s7dhansh s7dhansh changed the title doc.isModified() does not work when a nested key is added to a document, when the first level key itself does not exist? doc.isModified() does not work when a nested key is added to a document and the first level key itself does not exist Oct 17, 2018
@lukejagodzinski lukejagodzinski self-assigned this Oct 17, 2018
@lukejagodzinski
Copy link
Member

@s7dhansh probably it's a bug, I will check it

@lukejagodzinski
Copy link
Member

Fixed in jagi:astronomy@2.6.1

@s7dhansh
Copy link
Author

@lukejagodzinski thanks. but now it has caused another bug.
doc.isModified('apps.3.new') also turns out to be true now when modifying doc.apps.1. It's causing a lot of trouble. Reverted fro now.

@lukejagodzinski
Copy link
Member

Hehe ehh that's what happens when you release bugfix quickly without testing it :P. Right, sorry I will fix that

@lukejagodzinski
Copy link
Member

@s7dhansh sorry but in this case changing this behavior would require me to rewrite a lot of diffing algorithm and it can't be easily implemented. I will revert to the previous version.

@s7dhansh
Copy link
Author

s7dhansh commented Oct 20, 2018

I checked your last couple of commits. Won't this work?

Updates code (modeled after something that I am using now).

const modified = _includes(modified, pattern);
if (!pattern.includes('.') || modified) return modified;
const segments = pattern.split('.');
let parent = '';
return segments.some((segment) => {
	parent += (parent ? '.' : '') + segment;
	return (doc.getModifier().$set || {})[parent] || (doc.getModifier().$unset || {})[parent];
});

I understand that there will not be just $set, but only $set and $unset should suffice as anything else would mean that the current field is definitely getting modified directly.
I am not sure about the performance though, depends on how expensive is getModifier. But assuming you would be already calling it at least once for modification, you can cache the result and re-use it here.

@lukejagodzinski
Copy link
Member

@s7dhansh from what I remember using getModifier should work but it's kinda ugly hack. The best way to solve this problem would be to modify diffing algorithm to include nested fields even when parent was unset or set. I will test it and let you know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants