kurko / ember-sync

MIT License
282 stars 28 forks source link

Found elements don't have `emberSync` method defined (works in master) #9

Closed tute closed 10 years ago

tute commented 10 years ago
this.emberSync.find('farm', 1).then(function(farm) {
  return farm.emberSync === undefined;
});

That code is true. This means that if I find a model through emberSync I can't save it through emberSync (and persist the changes in both online and offline form.

kurko commented 10 years ago

@tute this is weird. Let me check on that.

@jakecraige do you have more context on this? I remember you implemented it.

tute commented 10 years ago

I'm now going through the source, I see a test that asserts find will inject this method on found object. So it has to be something about the integration between ember-sync and my ember-cli (itself an issue described in #8).

kurko commented 10 years ago

@tute it's unlikely it has anything to do with #8. If you're doing this.emberSync.find, then Ember Sync is already working in your app.

We have this test that covers this bug, so I need to investigate it more. On a side note, it seems to me like this ok(record.emberSync) is very fragile anyway. If record.emberSync returns a boolean, the tests will mistakenly pass.

jakecraige commented 10 years ago

@kurko This is supposed to embed it so I'm not sure why it wouldn't. I wonder if there is some way you can end up querying without that method being set yet. Having this empty method would silence something like that showing up

tute commented 10 years ago

When I do createRecord it does have emberSync.save() defined. If you need me to perform certain steps to reproduce I'd love to!

tute commented 10 years ago

See this debugger session: screen shot 2014-07-07 at 11 35 49 am

tute commented 10 years ago

I'll try from master, it seems this commit is before the released version I'm using: https://github.com/kurko/ember-sync/commit/92a7b1ee5fee456d3eee894fe83c4c7a2b328ac3

tute commented 10 years ago

Indeed that's the issue, in master it works. :)

Issue remains until a new version is released I guess.

Thank you both for your work!

kurko commented 10 years ago

My bad, I guess. I'll tag a release tonight.

tute commented 10 years ago

Cool, wait for it's release (https://www.npmjs.org/package/ember-sync).

kurko commented 10 years ago

Done.