emberjs / data

WarpDrive is a lightweight data library for web apps — universal, typed, reactive, and ready to scale.
https://api.emberjs.com/ember-data/release
MIT License
3.04k stars 1.33k forks source link

How do you get the snapshot for a model using a public API? #3002

Closed perlun closed 9 years ago

perlun commented 9 years ago

Hi,

(I saw this was already discussed in https://github.com/emberjs/data/issues/2797, but AFAIK it doesn't really cover the use case here, so hence a new issue is IMHO warranted.)

My use case is as follows (Ember 1.11.1/Ember Data 1.0.

The code is basically something like this (CoffeeScript):

    _copyToOfflineStore: (orderDraft, resolve, reject) ->
      # FIXME: Using private API:s is never nice.
      orderDraft = orderDraft._createSnapshot()

      offlineStore = @get('domain.offlineStore')
      adapter = @get('domain.offlineStore.adapter').create()
      adapter.createRecord(@get('domain.offlineStore'), { typeKey: 'orderDraft' }, orderDraft)
        .then () ->
          resolve orderDraft.id
        .catch (error) ->
          reject error

This works, and the data is correctly stored in localforage (IndexedDB in this case). But calling private APIs is of course never advisable, so what would be a more "correct" approach to this? I've tried first doing the serialization manually, wiring up a phony object with a serialize() method; I had that working with Ember 1.8.1/Ember Data 1.0 beta 14 before, but I had other problems that caused me to update to Ember 1.11.1/Ember Data 1.0 beta 16.1 (so that's what I am using at the moment). And now it seems like the phony object (which I would pass in to createRecord) needs more methods: eachAttributes, etc. It feels complex.

If you have other suggestions/pointers to the issue of "copy model instance from one store to another", please do tell. I am not by any means saying that the above is the "best" or even correct approach; I am just trying to get something that works. (First with Ember 1.11.1, but I will unfortunately have to backport the solution to 1.8.1/maybe older Ember Data as well...)

Huge thanks in advance.

fivetanley commented 9 years ago

I think you could use DS.Model#serialize to accomplish your needs here. https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/model/model.js#L424-L426

igorT commented 9 years ago

Moving records between two stores seems highly suspect to me.Instead of having online/offline store, wouldn't it make more sense for you adapter to handle this? (if you really need a copy of a record, doing a record.serialize followed by a serializer.normalize would be the way to go but again I advise against it)

perlun commented 9 years ago

I think you could use DS.Model#serialize to accomplish your needs here. https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/model/model.js#L424-L426

Thanks for the hint @fivetanley, I get an exception like this if I attempt to do it like that:

TypeError: snapshot.eachAttribute is not a function
    at ember$data$lib$system$serializer$$default.extend.serialize (json-serializer.js:474)
    at localforage.js:501
    at tryCatch (ember-1.11.1.debug.js:46937)
    at invokeCallback (ember-1.11.1.debug.js:46949)
    at ember-1.11.1.debug.js:48848
    at ember-1.11.1.debug.js:27239
    at Queue.invoke (ember-1.11.1.debug.js:871)
    at Object.Queue.flush (ember-1.11.1.debug.js:936)
    at Object.DeferredActionQueues.flush (ember-1.11.1.debug.js:741)
    at Object.Backburner.end (ember-1.11.1.debug.js:166)

This could very well be an adapter issue of course; it's the createRecord in this case that assumes that its parameter is a snapshot that it can run serialize on (localforage.js:501). Is this an error in the adapter that should be fixed do you think?

Just to make everything clear: the _createSnapshot approach works perfectly, it's just ugly and unsupported. :smile:

perlun commented 9 years ago

Moving records between two stores seems highly suspect to me.Instead of having online/offline store, wouldn't it make more sense for you adapter to handle this?

Making the adapter handle it may clearly be suitable in some (most?) cases, but here we have a very "explicit" offline scenario, i.e. the app knows in most cases whether the data should be loaded from offline or online. So the approach looks like this:

  1. Check if the data is available in the offline store.
  2. If it is, show an error message.
  3. If it isn't, load the data from the online store and cache that specific record offline (to be able to work with it in the app, without network access)
  4. Once the user is ready with the data, and back online, press the magic "Upload" button to upload the changes back to the server.

I'm not sure I follow in what you mean with letting the adapter handle it? I am using the https://github.com/genkgo/ember-localforage-adapter for doing the offline stuff, works quite OK. Are you suggesting creating a custom adapter; if so, how do you imply it should work?

(if you really need a copy of a record, doing a record.serialize followed by a serializer.normalize would be the way to go but again I advise against it)

Thanks. I tried that, but now I get an error about type.eachAttribute is not a function. It seems like adapter.createRecord doesn't really like it when it gets a serialized & normalized object; it really expects something else...

(I tried with both the localforage adapter and a plain RESTAdapter, and I had the same behavior.)