'observe' mapping options doesn't work with sub-object #137

Open
Tavriets opened this Issue Feb 10, 2013 · 8 comments

Comments

Projects
None yet
8 participants
Contributor

Tavriets commented Feb 10, 2013

Hi devs,

It looks like the 'observe' mapping doesn't work quite well for objects which contain sub-object.
Here is a sample:

// Working scenario
var data = { "toObserve": "foo", "toIgnore": "bar" };
var mapping = { "observe": ["toObserve"] };
// it is all good on next line
var vm = ko.mapping.fromJS(data, mapping); 

// Failing scenario
var data = { "toObserve": "foo", "toIgnore": { /*...*/ } };
var mapping = { "observe": ["toObserve"] };
// next line fails fails on attempt to treat "toIgnore" object as observable
var vm = ko.mapping.fromJS(data, mapping);

I think that in visitPropertiesOrArrayEntries callback (latest revision, line 467) the "value" must be unwrapped from observable as it is actually done few lines below for writable arrays. By doing that I get expected view model even for data objects with deep sub-object hierarchy. Could you please have a look on that? Thank you.

Cheers,
Tavriets

Collaborator

RoyJacobs commented Feb 11, 2013

@Tavriets, would it be possible for you to create a pull request with your fix, and add some unit tests that show the behavior? (i.e. the test should fail before your fix, and succeed after your fix)

Contributor

Tavriets commented Feb 12, 2013

No problem, will do.

Tavriets added a commit to Tavriets/knockout.mapping that referenced this issue Feb 16, 2013

Tavriets added a commit to Tavriets/knockout.mapping that referenced this issue Feb 16, 2013

RoyJacobs added a commit that referenced this issue Feb 18, 2013

I was having the exact same problem with nested objects and this fix solved the issue.

I can see this is in the source but hasnt been included into the build yet.

This fix solved my issue as well. (Currently using v2.4.1) Can you please merge this into master? Thanks!

It seems this fix was possibly lost somewhere. As of latest debug version of source code 2.4.1, on line 467 unwrapObservable is not used on value. value() is used instead.

Changing

mappedRootObject[indexer] = value();

to

mappedRootObject[indexer] = ko.utils.unwrapObservable(value);

seems to work

pruchai commented Nov 16, 2013

@mikehaas763 thank you for the fix! i just spent an hour trying to troubleshoot my code!

Hi, there
Any chance of pushing this fix in a new release any time soon? I run into this issue today and I was lucky to stumble to a stackoverflow thread that mentioned this bug.
It will help people save time and frustration

Thanks

@bellostom the author says: "Due to lack of time this project is currently not actively maintained."
Check the many forks that this project has, maybe someone has already fixit!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment