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

Question #369

Closed
basz opened this issue Oct 23, 2019 · 3 comments · Fixed by #370
Closed

Question #369

basz opened this issue Oct 23, 2019 · 3 comments · Fixed by #370

Comments

@basz
Copy link
Contributor

basz commented Oct 23, 2019

Not sure if this is a bug or if I'm simply misunderstanding usage but I noticed the following;

const list = changeset.get('arrayOfItems');
list.push(aNewItem);
changeset.set('arrayOfItems', list);

This will not mark the property arrayOfItems as changed. It does not appear in the changeset.changes. Eventhough I use changeset.set.

When I use the following does mark property arrayOfItems as changed.

changeset.set('arrayOfItems', JSON.parse(JSON.stringify(list)));

I think I get why this is happens; the value being a reference, which did not change, but it is a little confusing...

Version

2.1.2

Expected Behavior

Using changeset.set on arrays or lists should also mark the property as changed when it's different.

@snewcomer
Copy link
Collaborator

See linked PR. Thank you for the issue! Lmk if you have any more questions!

@basz
Copy link
Contributor Author

basz commented Oct 28, 2019

I see. So one must create a shallow copy in user land and changes are detected?... Perhaps set('arrayOfItems', [... arrayOfItems]) is the most elegant way to do that...

But I wonder, is there a reason the library can't relieve us of that mental weight for us?

@snewcomer
Copy link
Collaborator

In this case you are on operating on the same array throughout the lifecycle of that object, by the time we compare both array to determine if there are changes, the array has the same items in the array.

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 a pull request may close this issue.

2 participants