danielspaniel / ember-data-change-tracker

extending ember data to track and rollback changes including objects and associations
MIT License
106 stars 47 forks source link

BUG - info object can be undefined #70

Closed BnitoBzh closed 5 years ago

BnitoBzh commented 5 years ago

https://github.com/danielspaniel/ember-data-change-tracker/blob/c73b707e0d653c86423c0ccc9a8d9b98947cd27b/addon/tracker.js#L345

In the isKnown function, the info variable can be empty, due to the only option. If the only option does not contain the provided key, this.metaInfo(model, key) returns undefined and naturally the function fails.

danielspaniel commented 5 years ago

is there easy fix for this? If so make it happen. I am so far away from ember world ( spinning around the clojure universe now ) that I am just out of reach

BnitoBzh commented 5 years ago

please ask to @richardfrosztega to do the fix, I unfortunately have no time to do the necessary

pjcarly commented 5 years ago

I seem to be having problems with this function as well, but unrelated to the only option, as I do not use it.

It seems that in my case, the isKnown function is called for a hasMany relationship. But inside the trackerKeys on the constructor, the hasMany relationship is not found.

This function returns undefined in my case. Because the trackerKeys do not contain my hasMany relationship https://github.com/danielspaniel/ember-data-change-tracker/blob/a84506898c5e7c269bd8f774d94f961cc5a168cf/addon/tracker.js#L121-L127

I'd be willing to take a look, however, I don't understand the addon that well.

pjcarly commented 5 years ago

I managed to work around the issue for now with:

https://github.com/pjcarly/ember-data-change-tracker/commit/ede961a52d007c727dfbb4c4fb35fbb3493a3f98

¯\_(ツ)_/¯

richardfrosztega commented 5 years ago

Hi @pjcarly, @BnitoBzh,

I've followed up on the original change with the pull request above. It's difficult for me to prove this is what you need without some unit/acceptance tests from yourselves. But I've reasoned it though and think this might help.

Please feel free to see if this is what you need and talk to @danielspaniel about integration.

danielspaniel commented 5 years ago

v0.9.3 has the fix from #72