search

grofit / knockout.merge

Adds basic merging functionality into knockout js mapping namespace
MIT License
16 stars 3 forks source link

Observable with extender is not being written to #17

Closed ballwood closed 9 years ago

ballwood commented 9 years ago

Hi,

Come across a problem with knockout.merge. If an observable has an extender it won't write any value to it. I've included a jsFiddle below to show the behaviour, I would have a go at fixing it myself but I have no clue where to start regarding firing them, I thought they were automatic...

http://jsfiddle.net/zrr79ag1/4/

grofit commented 9 years ago

From a quick look I assume its because you are passing back a computed, and generally computeds are seen as read only values so they are ignored, assuming this is the case I am not sure how to work around it as you need to be able to tell if a computed is writeable and I believe knockout only exposes isComputed... also your 2nd property should match the vm property name (incorrect) not value, If I get time later I will take more of an in depth look.

ballwood commented 9 years ago

Change the giant if that wraps everything at the top to: 

if (ko.isComputed(knockoutElement) && !ko.isWriteableObservable(knockoutElement)) return;
grofit commented 9 years ago

oh good to see they have added that helper method in, I couldn't find it on a precursory look.

If you can, add the change in a pull request and just add a couple of tests for the scenario.

ballwood commented 9 years ago

Took me a while to find it, name is confusing as its a computed but obviously inherits from observable. I'll do a pull this evening when I'm back from work.

grofit commented 9 years ago

its ok, I will have 10 mins in a bit, will add the change and some tests in there, can do a new npm release then too.

grofit commented 9 years ago

Done, its in version 1.5.3

ballwood commented 9 years ago

awesome cheers :)