Create a gist now

Instantly share code, notes, and snippets.

What would you like to do?

ember-data + Promises

The Promise usage within ember-data needs some TLC.

I often find myself needing to tediously manage multiple events, just to know when a simple action, such as save, has completed. In addition, some of the async actions that return promises never reject. (find*)

I think we have a good foundation, but with some extra thought and work, we can make some drastic improvements.

the idea:

DS.Model is a promise-entity that resolves when the record is loaded, or brought into existence. (find/create) All future saves/updates/commits function calls will not affect this "base promise-entity". Rather these functions will return new promises, that will resolve or reject the record based on the result of the functions intent.

Similarly, ManyArrays and AdapterPopulatedRecordArrayss are promise-entitys that resolve on load.

Top level api improvements/changes

  • find*: -> promise-entity

  • createRecord: -> promise-entity

  • commit: -> promise

  • save: -> promise

  • reload: -> promise

  • commit: -> promise

  • deleteRecord: -> promise

  • RecorArray -> promise will resolve when all its known children are resolved, adding/removing later will not be reflected. Goal #1 is initial load.

record = store.find(Record, 1)
record.then(recordDidLoad, recordFailedToLoad);

// ...
record.save().then(didSave, failedToSave)
record.update().then(didUpdate, failedToUpdate)
record.deleteRecord().then(didDelete, failedToDelete)

record.then(...) // error if record is deleted

record.get('transaction').commit().then(didCommit, failedToCommit)

Underlying changes:

  • Adapter should return promises from all its asynchronous functions
    • For some revs, existing adapters using the non-promise API should be supported with deprecation warnings.
  • Store's corresponding async calls into adapter, should be done with promises
  • all promises must be a+ compliant (ideally RSVP)
  • $.ajax can be used at the edges, but must be must be wrapped by RSVP.wrap
    • jQuery promises have some inconsistencies, which makes mixing them with a+ promises a bad idea.

A point of concern:

Although promises are powerful, it is easy to accidentally allow them to eat important exceptions. This is bad. We may want some or all of the following:

  1. Ability to add a resolver for unhandled errors to a promise chain (would require changes to rsvp).
  2. A dev flag for noisy promises to log all non-propagated errors (or perhaps all errors).
  3. A hook in Ember.Application or DS.Store, for unhandled errors.

The Plan

  • [done] get RSVP#resolve merged in
  • [in-progress + issues] get promises resolved with themselves
  • [in-progress] get RSVP2 into ember
  • [in-progress] get RSVP2 into ember-data
  • assure existing promises actually reject and resolve when one expects them to. (find/create)
  • async methods (save/reload/commit etc..) should return promises.
  • store and adapters async communication should be done via promises.

Related Work

https://github.com/tildeio/rsvp.js/pull/63 https://github.com/emberjs/data/pull/724 https://github.com/emberjs/data/pull/858


Compiled by:

@hjdivad @stefanpenner

I like this approach!

I like this as well!

bobbus commented Mar 19, 2013

πŸ‘ Would be great to see this !

Dani723 commented Mar 21, 2013

great approach :)

hajee commented Mar 22, 2013

This would also bring the ember-data workings closer to what works ifor Jquery Ajax calls and thus easier te communicate to a large group of developers that already work with JQuery promises.

@stefanpenner is the StateManager still in use or will that be out the window with this approach?

Owner

@bcadarella the StateManager and events will remain. The promise stuff will sit nicely on top, providing an easy-to-reason-about API.

πŸ‘ i like

syrnick commented Mar 24, 2013

That looks pretty exciting. Is it slated to happen before 1.0 ?

endash commented Mar 26, 2013

+1

Owner

@syrnick well before ember-data 1.0 (as it is not yet even a release candidate)
Foundation work has already begun, I will try and keep this gist to up date.

tchak commented Apr 4, 2013

@stefanpenner I am concerned with create returning promise-entity. In case of error it get rejected, so how we supposed to retry a save ?

I'm really excited for this. This is a necessary change to make ember-data usable in real-world scenarios, with much more elegant error handling than awkward model event listeners :)

hjdivad commented Apr 4, 2013

@tchak I don't think the problem you raise is specific to create. We have the same problem with record.save(). Actually if your complaint is that the promise-entity can leave a resolved or rejected state contra the spec this problem exists even in the success case.

I think the answer is that strictly speaking a promise-entity is not quite a promise. It's just very promise-like and can treated as a promise in all respects except state-mutability.

@stefanpenner thoughts?

Owner

@tchak you bring up a good point. Noted.

Owner

@hjdivad our thoughts are aligned. The root of our problem is these promise-entities and how much we are overloading them. I am positive a solution exists, more thought must be given.

πŸ‘

πŸ‘ I was literally looking into implementing something like this but looked around to see if anyone else is working on a similar solution. Great to see that it's moving in this direction.

@stefanpenner kinda funny, i was just yelling about this a little earlier this morning :)

For us, we're running into a situation where we're trying to chain multiple adapters together. eg: https://gist.github.com/leepfrog/5359583 . Without async being returned from the differing adapter methods, this quickly gets us into a "nightmare level" type of situation (as you can kinda sense from the gist).

Using promises would definitely make it significantly more easy to add in additional adapters.

rmossuk commented Apr 16, 2013

Can't wait for this to be implemented +1

otb commented Apr 17, 2013

Looking forward to this one, too; +1.

This is much needed. Looking forward to it.

mankind commented Apr 20, 2013

πŸ‘

Emerson commented Apr 21, 2013

+1

caifara commented Apr 23, 2013

+1

l33z3r commented Apr 25, 2013

+1

vijayj commented May 16, 2013

This looks great. Any idea when this will get implemented ?

ilovett commented Aug 15, 2013

PLUS ONE

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment