beta-11 Wrapping relationships breaks quite a lot of app-side code

[ginty]

Calls to retrieve a relation object, like user.get("address") for example are now wrapped in a DS.PromiseObject, however this is not a complete proxy for the real object.

Specifically the following broke in my app:

# The wrapped object is not == the real object, breaks things like
user.get("address") == same_address   # False, compares to the proxy, not sure if JS allows any way around that

# Finds are all broken, I assume compares to the proxy again internally
users.findBy("address", address)

# Logic based on a relationship being empty is now broken, since you always get a proxy back
if user.get("address")
   # Now always runs

# Calls to any methods implemented in the model are not passed through and return undefined, i.e. the proxy does not consider model methods 
user.get("address").format()  # Error, no method format for undefined when the address is present and the class has a format method

It's easy enough to fix once you know what to expect by calling .content everywhere on the promise object, i.e. replace "address" in all of the above with "address.content"

The following would help I think if the proxy cannot be made truly invisible:

• .content needs to be an official API (if it is not already, not sure but it feels like a hack at present)
• DS.Model should get a content method that returns self, helps when not sure if you are looking at a proxy or the real thing

Thanks!

[quaertym]

@ginty I agree that writing content everywhere feels like a hack. What I usually do is set an alias for model.content however there should be a better way.

[sandstrom]

@ginty I think the way forward is to make certain things aware of promises, and 'unwrap' them where appropriate. There is already work completed in that direction, e.g. #2288, #2325 and #750.

However, this won't solve all issues, and the ones you mention are tricky.

1. Overloading === is complicated in js[1], also you'd need a way to wait for both to resolve before comparing. Something like Ember.RSVP.hash({a: user.get("address"), b: same_address }).then(function(hash) { return hash.a === hash.b }) should work though.
2. findBy and similar could be enhanced to unwrap promises, but since it would need to handle unresolved promises it would then need to return a promise – and that would be confusing [and breaking], so I think address.then(function(address) { users.findBy("address", address); // ... }); is better.
3. if user.get("address")
   Your analysis is correct, this is hard since relations may need to be fetched before you'll know if they exist. But something like user.get("address").then(function(address) { if (address) { // test here } } should work.
4. Similar to the if above, you must wait for the promise to resolve.

Bottom line is that promises are hard, but so are callbacks. I personally find promises more useful (though I understand if they feel confusing).

A common theme in rich clients is that they work on a subset of data, with latency between the authoritative store (backend DB) and the client. In these cases, doing this async is necessary (in server-side code you can often resolve relationships etc. synchronously). Given these constraints, promises are really useful.

@ginty Do you have any thoughts or suggestions on how the docs can be improved? It's easy to submit PRs to the docs, with ideas on improvements. For example: https://github.com/emberjs/website/blob/master/source/guides/models/finding-records.md

[1] http://stackoverflow.com/questions/10539938/override-the-equivalence-comparison-in-javascript

[tpitale]

I was reading the CHANGELOG, and I couldn't find where this change was mentioned or what it meant. The most I could find was "Refactor relationship handling code". Is that it? Either way, it might be useful to mention.

[ahacking]

I've also been bitten by this behaviour, even knowing that the promise has settled and comparing some belongsTo relationship property with another model instance always fails, you must use relationship.content instead.

The typical use case is in an observer where you want to update something conditionally. Its too easy to forget the '.content' as its not needed in other places.

I have so much controller wrapping of model concerns and promise related constructs like this to deal with now that quite honestly I really am questioning where the payoff in Ember Data is above the adapter, serializer and identity map.

[ginty]

The promise behaviour makes sense in the context of asynchronous relationships and in which case using relationship.then as described by @sandstrom above is reasonable.
However it is downright annoying and certainly unexpected when dealing with a synchronous relationship which is defined as such and the models are side-loaded by the API.

Similar to @ahacking I am preferring now not to use Ember Data for synchronous relationships when I can get away with it and have the API return embedded json blobs from which I can then just extract what I need using synchronous code. I guess this only works for cases where the relationship data is read only.

[ginty]

@sandstrom Thanks for the reply above, certainly helpful. For me part of the problem was that my original code never treated these relationships like promises because I guess they never returned promises until now. I can certainly see the rationale for this.

On the doc question the problem is that the model documentation does not mention that the relationships return promises at all and certainly examples like you show on how to deal with equality and similar logic on the relationship are required.

I can help submit some doc patches here, but one problem for a user of ember data (rather than a core contributor) is that it is difficult to know what the end goal is. For example are synchronous relationships really meant to return promises or is that just a quirk of the current beta?

[bmac]

@stefanpenner / @igorT I've think I recall hearing you mentioning refactoring the PromiseObject is that a real thing or a figment of my imagination? And more specifically will that impact the current relationship api or can @ginty start opening pr to fix up the doc examples now?

[stefanpenner]

a real thing or a figment of my imagination

it is a real thing, after this current push on HTMLBars and streams, we plan on removing promise proxy all together in-favor of making the underlying RSVP system promise aware.

[eccegordo]

Absolutely agree with the comments in this thread.

I think a little bit of idiomatic documentation or a short guide on how to concretely handle promise behavior would go a long way. Just provide some examples and light commentary on the following:

• what "promise aware" behavior looks like in practice
• how to tell is something is sync or async ( e.g. foo.then() === undefined )
• common coding styles and patterns that are designed to make promisified objects and models easier to work with
• known gotchas and examples
• how to debug and guard against common pit falls (screencast?)
• where the $!@^)* is Zalgo? Can't we stare into the abyss occasionally without having to watch our backs all the time? Someone needs to keep tabs on Zalgo's whereabouts for all our sakes! ... 😄

I feel like I am running into the nuances of promises all the time and it often leads to a real and palpable feeling of frustration.

I think this is mostly a documentation problem and just better understanding of how to code well functioning promise aware code. Hope it gets better.

[bmac]

Great idea @eccegordo. Do you have any interest / bandwidth to take a stab at putting together some of that documentation?

[eccegordo]

@bmac I would probably be down for helping some, if only to increase my own understanding of ember data internals. I don't always mean to be a complainer! 😄

The challenge is there is still a lot I need to understand about ED

A few main interests I personally have:

• how to write less verbose promise code. Especially when setting up complex relationships and multiple models the nested thennables are a pain.
• one sided relationships, do we always have to define inverses? What about other relationship types? Many to One, hasOne, Has many through, has and belongs to, etc. While I recognized ED is not active record, I would love to see an ED equivalent of the rails Active Record Associations guide http://guides.rubyonrails.org/association_basics.html
• Multiple models in a route or how to have complex relationships handled together in a single operation, route hook, or save operation or function. How to effectively use the RSVP hash and array types. How to deal with a mix of sync and async attrs.
• What exactly is "broken" with ember data today? Is there an easy to read list somewhere? Perhaps more on the "theory" of ember data. Why is it the way it is and what are the conscious tradeoffs being made? I think that is pure gold, because I suspect many people like myself come into ED with preconceived notions.
• code patterns, code patterns, code patterns. Real and interesting examples of what you can do with ember data. The main guides seem quite limited.

I have started to look closer at the test cases to better understand ED. If I can make it through some of that and gain more insight I would be down to at least trying to add to some of the documentation effort. I fear I would still need some input and guidance on some of the finer points of ED.

Is there a plan of record for future documentation?

Let me see if I can organize my thoughts into a more coherent outline and get back to you.

Would love to help if I can make the time.

[slindberg]

A few months ago, 100% of relationships in my app were synchronous, so I've been slowly trying to move towards async relationships as things get refactored, and I'm running into this same set of problems.

I think I can say that ideally, application code should not need to care whether an object is a DS.PromiseObject vs. when it is an actual DS.Model instance, but in practice I've found that this is not really possible. I was surprised to find just how many places my app was comparing models by identity, either directly with === or through array helpers like findBy or filterBy (not to mention instanceof checks, which has its own problems with model factory injections). Comparison by id is not really a viable alternative in these cases since you often don't know whether a record is new or not.

Accessing relationships on a record makes computed properties difficult, but there are usually workarounds. What makes promise wrappers especially difficult in my experience is passing models between different (conceptual) components in code. Every component (be it a controller, service, other model, or actual component) has its own API through properties, handlebars attributes, or method arguments that could expect a model instance. Simplistic example:

{{! `model` was resolved in the model hook, so is a real DS.Model instance}}
Thing: {{thing-component model=model}}

{{! `model.parent` is an async belongsTo relationship, so is a DS.PromiseObject wrapper}}
Parent Thing: {{thing-component model=model.parent}}

When some methods return promise wrappers and others model instances, it forces all APIs that expect models to deal with promise wrappers. This is simply not a reasonable expectation for developers given the amount of boilerplate (DS.PromiseArray/DS.PromiseObject) and hackiness (model.content) involved.

The road to ED 1.0 post makes it sound like the solution is DataBoundPromises, but IMHO the only solution is better primitives in Ember core for dealing with asynchrony. There's been a buzz around 'async components', which I haven't found good definition for, but would presumably solve the above example by automatically unwrapping any attributes that are promises. I think this is a step in the right direction, but falls short by itself.

I think to get the rest of the way, what we need is the ability to do something like:

export default Ember.Component.extend({
  async isThingOtherThing() {
    var thing = await this.get('thing');
    var otherThing = await this.get('model.otherThing');

    return otherThing === thing;
  }.property('thing', 'model.otherThing')
});

But I honestly have no idea how feasible something like this is, given the current synchronous nature of CPs. This could be similar to what @stefanpenner mentioned above, since I doubt he actually meant he wants to make the promise system promise-aware 😉

we plan on removing promise proxy all together in-favor of making the underlying RSVP system promise aware.

[fivetanley]

@stefanpenner what is the status of this? is glimmer promise-aware?

[stefanpenner]

@stefanpenner what is the status of this? is glimmer promise-aware?

Not yet, but we would like to do so.

[fivetanley]

Closing in favor of emberjs/ember.js#12825