belongsTo relationship causes "used previously in the same computation" error

[wongpeiyi]

First posted in emberjs/data#7196 but was referred here

When an API payload is received with an association not already set on a Ember Data model, the following error is seen:

Error: Assertion Failed: You attempted to update `content` on `<(unknown):ember173>`, 
but it had already been used previously in the same computation.  Attempting to update 
a value after using it in a computation can cause logical errors, infinite 
revalidation bugs, and performance issues, and is not supported.

`content` was first used:

- While rendering:
  
  application
    foo
      this.dataLength

Stack trace for the update:
    at dirtyTagFor (validator.js:514)
    at markObjectAsDirty (index.js:637)
    at notifyPropertyChange (index.js:675)
    at set (index.js:1622)
    at Proxy.set (observable.js:176)
    at InternalModel._updatePromiseProxyFor (-private.js:4379)
    at InternalModel.getBelongsTo (-private.js:4272)

This happens when the model is being consumed in a component template (via a getter) that depends on post.user

Reproduction

https://github.com/wongpeiyi/data-computation

Description

• When creating and saving a Post, no User association is set yet
• The API (mirage) sets a User and responds with a payload
• Ember Data updates the Post model
• Because the model is being consumed by the component getter that depends on filterBy('user.id'), the error is thrown

The error is not thrown when:

• Not filtering (or not consuming the user association)
• Using {{@data.length}} in the template instead of the component getter
• Using a getter on the controller without a component

This seems like a use case that should "just work"

Versions

yarn list v1.19.2
warning Filtering by arguments is deprecated. Please use the pattern option instead.
└─ [email protected]
✨  Done in 0.72s.
yarn list v1.19.2
warning Filtering by arguments is deprecated. Please use the pattern option instead.
└─ [email protected]
✨  Done in 0.65s.
yarn list v1.19.2
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ @ember-data/[email protected]
├─ [email protected]
└─ [email protected]
✨  Done in 0.63s.

[snewcomer]

I think this is a really interesting conversation, especially as we all rely more and more on autotracking inter-opting with things that implement Observables.

My initial feeling was that b/c we are mixing paradigms of an array that implements Observable (push) with autotracking (pull), a solution shown here might be necessary but not preferred.

@pzuraq Is this true that some solutions might not work with autotracking and have to be re-thought in app code or do we have preferred mechanism of fixing the source as detailed towards the bottom of this comment without a full re-write towards autotracking semantics? Perhaps a more general question as well is - are Observable friendly Array-like behaviours necessarily at odds with autotracking or is it a case by case basis like this one?

[snewcomer]

@wongpeiyi I'm working on a reproducible test for this issue. The core of the problem seems to be some side effects (Ember.set) that happen as a result of accessing a belongsTo relationship after your save. I think this test still needs some work to accurately represent your case, so lmk if you have some ideas!

#19009

[wongpeiyi]

@snewcomer thank you!

I don't quite know enough about the internals but I suspect it has more to do with InternalModel + PromiseProxy than ArrayProxy. The array and filterBy('user.id') was only a means to consume the belongsTo('user') relationship.

Rgd InternalModel: the stack trace pointed to https://github.com/emberjs/data/blob/e154539507199a85481f0621e99fb4d2b2efffcb/packages/store/addon/-private/system/model/internal-model.ts#L828
which is called from getBelongsTo, and sets content on promiseProxy and then immediately returns promiseProxy.

I'm just guessing here, but it seems like I'm consuming a belongs-to promiseProxy.content, e.g. post.get('user.content.id'), during render, but it's also set-ing the content, post.user.content, as I'm getting it.

This is conjecture, so let me see if I can create more test cases, e.g. without requiring arrays

[snewcomer]

aa607a6

This commit might be closer to what we are looking for. Lastly, need to trigger glimmer tracking/untrack. I'll see what other steps we might need to take to get this to fail...

[runspired]

I've updated the investigation in the original ticket here: emberjs/data#7196 (comment)

TL;DR user error + mirage funkiness results in this. There's probably some things the framework team should consider for improving these cases though.

One of which is an issue in glimmer-vm in the DEBUG environment wherein having an unstable getter accessed multiple times by a template is problematic only in DEBUG and not in production where PropertyReferences are merged. See glimmerjs/glimmer-vm#1111 for more details. See the linked issue in EmberData for a complete run down.

[snewcomer]

Thanks a lot for the explanation! It was really useful.

Finally got a test reproduction working as well (albeit integration)! Initially reproduced with a list of EmberData like models like the reproduction in this issue. However, pruned the test case to show the simplest path to hitting the error -

https://github.com/emberjs/ember.js/pull/19009/files