alekseykulikov / backbone-offline

[Deprecated] Allows your Backbone.js app to work offline
MIT License
720 stars 56 forks source link

backbone.Offline is messing with id #9

Closed gilbert closed 12 years ago

gilbert commented 12 years ago

So it looks like you copy id into sid and generate a new guid for id. Why is this necessary? I'm asking because I'm not familiar enough with Backbone's source to know.

Changing id loses some useful backbone properties:

  1. Backbone gracefully ignores adding a second model with a duplicate id
    • With Offline, the model will get added anyway since the first id has been changed
  2. Backbone's Collection#get method retrieves a model by its id
    • With Offline, you have to search by sid, which might not exist for newly created models

It's possible I'm taking the wrong approach. Here is my use case:

Can you explain why you're modifying id, and not using a client id similar to Backbone's cid? I want to understand so I can submit a patch if needed.

alekseykulikov commented 12 years ago

Backbone.offline generated new ids for every new object which was added to storage. localStorage is like local DB. When you offline you can not generate id on server and you using local-id for consistence. You still can use Collection#get and it will be always correct no matter was synced this model or not. And feature with duplicate id controlling in the phase of sync. I agree that we should delegate it to Backbone, but now it's still not so.

Why I don't use cid: Cid generated backbone and it's different between user sessions. You don't know which Backbone objects you will be use in current session. Although your idea is interesting and I should thinking about it and this way make library more flexible and more Backbone oriented.

Your way is not typical use case. You want to use backbone.offline as a temporary cache. That's mean that you should use sid for relations between other collections. But when you don't have a sid what id do you want to save to relational model?

gilbert commented 12 years ago

Thanks for the explanation. Since the app will be available offline, backbone.offline does fit my use case since I don't want to rely on the server to create tags.

I think I just need a way to tell backbone.offline "Here is a model from the server". Something like an update() method, or an option for pull(). This method/option would assume that the given model/data is already saved on the server, so it would copy over the id to sid and check for duplicates based on that.

Yes, an option to pull() does sounds like a good idea. It would use the data you give it instead of getting it from the server. Thoughts?

alekseykulikov commented 12 years ago

I think we should use option in init mode like localId: false. It will work good when you don't want to relate your collection with others. Bad offline case: In offline mode you created a model and was generated id: sdhfshdkfj-sdfds-sdfsd-sdf. You connected this model with other models and saved primary ids. When you will sync this model which id will you use? We shouldn't change primary id on sync, it involves bad consequences.

gilbert commented 12 years ago

Does your bad offline case work with backbone.offline now? The option I'm suggesting would only be used when you receive data from the server from somewhere besides pull(). In other words, it will already have a proper id; backbone.offline would treat it as if it did come from pull(), changing the id and sid as it normally would.

gilbert commented 12 years ago

After studying the source code, it turns out there's already a method for this: Offline.Sync#pullItem. This fits what I need.

I'm going to be studying backbone.offline more extensively, since my app needs to be fully useable offline. Just fyi

alekseykulikov commented 12 years ago

Ok. Glad to hear that you decided your problem and look forward for new experience and probably pull requests. Good luck!