Skip to content

Instantly share code, notes, and snippets.

@stefanpenner
Last active December 15, 2015 01:19
Show Gist options
  • Star 40 You must be signed in to star a gist
  • Fork 2 You must be signed in to fork a gist
  • Save stefanpenner/9ccb0503e451a9792ed0 to your computer and use it in GitHub Desktop.
Save stefanpenner/9ccb0503e451a9792ed0 to your computer and use it in GitHub Desktop.

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

tildeio/rsvp.js#63 emberjs/data#724 emberjs/data#858


Compiled by:

@hjdivad @stefanpenner

@Dani723
Copy link

Dani723 commented Mar 21, 2013

great approach :)

@hajee
Copy link

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.

@bcardarella
Copy link

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

@stefanpenner
Copy link
Author

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

@pangratz
Copy link

👍 i like

@KasperTidemann
Copy link

+1

@syrnick
Copy link

syrnick commented Mar 24, 2013

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

@endash
Copy link

endash commented Mar 26, 2013

+1

@stefanpenner
Copy link
Author

@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
Copy link

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 ?

@thomasboyt
Copy link

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
Copy link

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?

@stefanpenner
Copy link
Author

@tchak you bring up a good point. Noted.

@stefanpenner
Copy link
Author

@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.

@darthdeus
Copy link

👍

@evilmarty
Copy link

👍 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.

@leepfrog
Copy link

@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
Copy link

rmossuk commented Apr 16, 2013

Can't wait for this to be implemented +1

@otb
Copy link

otb commented Apr 17, 2013

Looking forward to this one, too; +1.

@seanrucker
Copy link

This is much needed. Looking forward to it.

@mankind
Copy link

mankind commented Apr 20, 2013

👍

@Emerson
Copy link

Emerson commented Apr 21, 2013

+1

@KirillSuhodolov
Copy link

+1

@caifara
Copy link

caifara commented Apr 23, 2013

+1

@l33z3r
Copy link

l33z3r commented Apr 25, 2013

+1

@Goltergaul
Copy link

+1

@vijayj
Copy link

vijayj commented May 16, 2013

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

@TrailHacker
Copy link

+1

@ilovett
Copy link

ilovett commented Aug 15, 2013

PLUS ONE

@rodchito
Copy link

+1

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