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

Multiple recomputes of a getList derived property #449

Open
1 of 5 tasks
ivospinheiro opened this issue Jan 8, 2019 · 4 comments
Open
1 of 5 tasks

Multiple recomputes of a getList derived property #449

ivospinheiro opened this issue Jan 8, 2019 · 4 comments
Assignees
Labels

Comments

@ivospinheiro
Copy link

ivospinheiro commented Jan 8, 2019

Derived properties from can-connect getList result are being recomputed for every single result list item when a second call is made.

How often can you reproduce it?

  • Always
  • Sometimes
  • Rarely
  • Unable
  • I didn’t try

Description:
Check the following example with tests describing what was the expected behavior:
https://codepen.io/ivospinheiro-the-reactor/pen/MZXjOw?editors=1010

It seems that in this case the recompute is being triggered due to change on the length on todo items assignedTo property which in this case has not changed since the result is the same.

Environment:

Software Version
can-connect version 5.21.4
Browser Chrome 71
Operating system Ubuntu
@cherifGsoul cherifGsoul self-assigned this Jan 9, 2019
justinbmeyer added a commit that referenced this issue Jan 10, 2019
@justinbmeyer
Copy link
Contributor

@ivospinheiro The issue here is that the Person objects aren't able to be diffed because they don't have an identity. Does name work logically as an identity?

If I change Person to:

const Person = DefineMap.extend("Person",{
		name: {type: "string", identity: true}
});

The tests pass for me.

This sub-section explains how identity can be used to make CanJS's diff work properly:

image

You'll find that content here: https://canjs.com/doc/can-rest-model.html#Definedatatypes

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Jan 10, 2019

Some notes on why it happens without identity:

  1. can-diff/smart-merge is called with {"name":"mow lawn","id":5,"assignedTo":[{"name":"john"}]}.
  2. This calls mergeList with the Person.List and [{"name":"john"}].
  3. Because there is no identity, mergeList uses === to compare the existing Person instance and the object{"name":"john"}. These are not === so a patch updating the list is created and applied.

One solution might be to create a "smart deep diff". A smart deep diff would, instead of default to === when there is no identity, do a deep diff.

@ivospinheiro
Copy link
Author

Thanks Justin by the clarification, but probably in certain cases we are not able to define the identity property.
I have faced this issue originally using CanJS 4 and in my case the list was a list of links using HATEOAS.
For instance the response of the server could be:

fixture("GET /api/todos", [
  { name: "mow lawn", id: 5, links: [{href: "self", url: "/api/todos/5"}, {href: "history", url: "/api/todos/5/history"}]},
  { name: "learn canjs", id: 6, links: [{href: "self", url: "/api/todos/6"}, {href: "history", url: "/api/todos/6/history"}]},
]);

I'm not sure if we could have an identity in this case, eventually url.

@justinbmeyer
Copy link
Contributor

Yeah, we can create something to help, but I’m not sure what. By the way, you can overwrite updatedInstance to update differently (such as use assignDeep)

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

4 participants