getoutreach / epf

A framework for keeping your Ember.js apps in sync.
http://epf.io
MIT License
369 stars 33 forks source link

Ep.IdSerializer can break with mongodb #124

Closed pvertenten closed 10 years ago

pvertenten commented 10 years ago

http://screencast.com/t/rETgQdeE

The current serialize implementation checks for isNaN. The problem is that a bson id does not have to have a string. It can be a series of all numbers as well. It took me a long time to run into this issue, but you can eventually get an id that only has numbers and no letters. So the following function does not work in this instance. Its safer to always treat it as a string in this case.

 serialize: function(id) {     if (isNaN(id) || id === null) { return id; }     return +id;   }

ghempton commented 10 years ago

Makes sense. This default serializer is geared towards numeric ids on the backend (e.g. active record). Might be a good idea to change this.

In the short term, you can get around this easily by just creating a custom serializer override:

App.IdSerializer = Epf.IdSerializer.extend

or if you are using EAK you can just put this in app/serializers/id.js.

pvertenten commented 10 years ago

Thanks,

I'm on an earlier version, and have it updated in a private repo. I just noticed the issue was still happening with the latest code.

ghempton commented 10 years ago

Yeah the id serialization semantics are pulled straight from Ember Data. Will close this for now, eventually we might want a more universal default for the root adapter class.