search

derbyjs / racer

Realtime model synchronization engine for Node.js
1.18k stars 117 forks source link

Avoid striping ids in local collections while bundling #185

Closed vmakhaev closed 10 years ago

vmakhaev commented 10 years ago

We strip doc ids in collections during creation of bundle. Ids in remote docs are restored on client, but ids in local docs never restored. It makes data inconsistent between server and client.

minicuper commented 10 years ago

Ok, Nate but there is still a bug with LocalDocs ids. Racer stripes all ids but recreate only RemoteDocs ids. What is the best way to fix it?

Look: RemoteDocs - https://github.com/codeparty/racer/blob/master/lib/Model/RemoteDoc.js#L55-L61 LocalDocs - https://github.com/codeparty/racer/blob/master/lib/Model/LocalDoc.js#L14-L16

Maybe we shoud make something like this to unify the two types of Docs?

LocalDoc.prototype._updateCollectionData = function() {
  var snapshot = this.shareDoc.snapshot;
  if (typeof snapshot === 'object' && !Array.isArray(snapshot)) {
    snapshot.id = this.id;
  }
  this.collectionData[this.id] = snapshot;
};
nateps commented 10 years ago

Hey, sorry for the revert with no explanation. I've been working pretty much non-stop for the past 3 weeks up until now finishing the port of our primary application to Derby 0.6, and this pull request was simply broken. Our app is now launched on Derby 0.6, and all super critical issues are fixed.

What you wanted to do here makes sense, but the condition was put in the wrong place (unbundle vs. bundle) and it just totally broke everything when running this code. Please be a lot more careful before pulling in any new code to master before extensively testing it with real apps as well as the unit tests.

I'll fix the bug with improperly removing ids on LocalDocs now.

nateps commented 10 years ago

Fixed the bug in b5d7a3856745db756037f7e5011c7985536165e4 / 983c778b73ae1723166523e56f24e2cde57def60

minicuper commented 10 years ago

I was on a kind of vacation during last two weeks but nice to see the fix. Next time will test PRs more carefully.

And thanks, Nate. Derby is getting stronger and stronger.