canjs / can-connect

Model layer utilities for every JavaScript framework! Assemble real-time, high performance, restful data connections.
https://canjs.com/doc/can-connect.html
MIT License
29 stars 16 forks source link

Multiple recomputes of a getList derived property #449

Open ivospinheiro opened 5 years ago

ivospinheiro commented 5 years ago

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?

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
justinbmeyer commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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)