Added tracking to the returned result object when assigning data

[kobsy]

Attempt number 2 at resolving an issue where cache updates would be visible to the Ember observation system but not Glimmer's autotracking system. From the description of my first attempt (#398):

I was running into an issue (possibly related to #384) where native getters on my controllers would not recompute their values when the Apollo cache was updated. This became especially apparent when using an API that took advantage of relay-style pagination: a computed property that pulled out all the edges' nodes on a field would not update when either the node was changed or a new edge was added (e.g., as part of a create mutation). What I found particularly odd was that I could make the update work if I added a @computed decorator to the getter. So, for example:

// This did not update when the underlying cache updated
get reviews() { return this.model.movie.reviews.edges.map(edge => edge.node); }

// ... but this did!
@computed('model.movie.reviews.edges.@each.node')
get reviews() { return this.model.movie.reviews.edges.map(edge => edge.node); }

Because I could get it to work using the decorator, it seemed like there was something that wasn't getting set up for autotracking using @tracked when the data was initially assigned, resulting in setProperties() doing nothing until something registered that it was watching the value via the classic observation system.

I first tried simply adding tracked-built-ins, but it uses Proxy under-the-hood, which rules out support for IE11 (since IE does not and never will support Proxy). 😞 Understandably, that was a show-stopper for taking that route.

This second attempt uses a similar (but IE-compatible) approach, creating a class for the sake of facilitating tracking changes to an object's properties in the autotracking system. Since values are assigned to our storage obj using Ember's setProperties, we can take advantage of the fact that Ember's set first looks for a setUnknownProperty method and then use that to JIT define a @tracked property on our obj before assignment. To preserve POJO behavior, the setUnknownProperty method is not enumerable, while all JIT-defined properties are. The end result is that reads and writes to those JIT-defined properties are properly tracked in the autotracking system so that cache updates are reflected as expected in the DOM. 😁

Thanks (again!) for your consideration. 🙂

[josemarluedke]

@kobsy The changes are looking good! I like your approach here.

It seems there is some issue now when I click "Refetch (using route refresh())" button on the movie page (Dummy app). It seems we have an infinite loop refeching. Can you take a look at that?

[kobsy]

@josemarluedke, thanks for the feedback! 🙂 It took some digging as to why the infinite loop was occurring, but it looks like it was due to the way graphql-tools was mocking the integer for star reviews outside of the tests. In the tests, we were supplying specific values for the stars of the reviews, but when just viewing the dummy app, a random integer (positive or negative! 😱) was being returned for each request. This ran afoul of apollo-client checking equality of subsequent results in its ObservableQuery; since it was a random number and thus always different, I think it kept trying to get the latest ad infinitum. 😵 I updated the commit that adds the tests to supply 5 by default for any mocked integer, and that appears to have eliminated the loop when I test it out locally. 👍

[josemarluedke]

@kobsy Thank you for working through this. It is now published under v3.2.0.

[johanrd]

Hi. Great work! I was hoping this would solve a problem I've had. However, my problem only seems close, so I'm posting here as a related question:

When querying a paginated list, it would be good to do data handling inside the model hook so that the model could be used directly like e.g.:

{{#each @model as |organization|}}
  {{organization.name}}
{{/each}}

On first glance, a returning a maped watchQuery like this seems to work:

export default class OrganizationList extends Route {
  async model() {
    const organizationConnection = await this.apollo.watchQuery({query: organizationConnection, 'organizationConnection')
    return organizationConnection?.edges?.map((edge) => edge.node))
  }
}

However, when using a map function like this in the model hook, only the initial load is loaded correctly, whereas the DOM does not get updated upon cache updates.

There are two working alternatives:

1. Create a controller with a getter (as outlined above by @kobsy) could be a solution, however feels a bit unnecessary:

export default class OrganizationListController extends Controller {
  get organizationList() { 
    return this.model.edges.map(edge => edge.node) 
  }
}

2. Skip the map function in the model hook…

export default class OrganizationList extends Route {
  async model() {
    return await this.apollo.watchQuery({query: organizationConnection, 'organizationConnection')
  }
}

…and live with the inherited verbosity the few places where the connection result is used:

{{#each @model.edges as |organization|}}
  {{organization.node.name}}
{{/each}}

Both alternatives work as expected upon cache updates, however they both feel a bit cumbersome / unergonomic.

Do you know of an elegant way to return a nested array like this directly from the model hook, that also is updated when the cache is updated? I feel there may be a simple solution I am missing.

Thanks!