Mapping parameter is ignored on calls to ko.mapping.fromJS after the first about knockout.mapping HOT 16 CLOSED

This can be fixed by changing

options = ko.utils.unwrapObservable(mappedRootObject)[mappingProperty];

to

options = jQuery.extend(ko.utils.unwrapObservable(mappedRootObject)[mappingProperty], options);

assuming you are OK taking a dependency on jQuery.extend.

Also, I think that changing

result[mappingProperty] = options;

to

jQuery.extend(result[mappingProperty], options);

is probably a good idea, but to be honest I don't really understand that code as well.

from knockout.mapping.

Hi Domenic,

I would prefer not to take a dependency on jQuery, but I guess a function similar to 'extend' can be added to the code.

The reason the options are put into the resulting viewmodel is so that updateFromJS can read the options used for mapping, and reuse them for updating the viewmodel.

Have you already applied your workaround? If so, a pull request is welcome :) (as long as the unit tests still pass, of course)

from knockout.mapping.

Ah, I didn't expect a pull request to actually arrive, well done!

But I still have a question: What is the intended use-case, and why are you trying to share a viewmodel in this way? Why not have two separate viewmodels that you can map in the regular way (and if you want to update them, use updateFromJS)?

from knockout.mapping.

Here was my use case (simplified for clarity); I'm open to suggestions on a better approach:

• Ajax request 1 grabs user stats on page load, for every page on the site. One resulting observable is currentCoins.
  The view model is now bound to #user, which has a binding specifically for currentCoins. #user is in my site template (i.e. is always displayed no matter what page you're on).
• Ajax request 2 grabs product details whenever a product image is clicked. One resulting observable is productCost. The view model should now also have a dependent observable canAfford that is true iff Y <= X. The view model is then bound to #product, which has bindings for both productCost and canAfford.
• When the user clicks over to his purchase history, Ajax request 3 grabs user's coins last week, resulting in observable lastWeekCoins. The view model is now bound to #history, which has bindings for both lastWeekCoins and currentCoins.

The most natural way to do this seems to be to have one view model, which always gets the user stats (including currentCoins), but depending on the page the user is currently on, gets other observables as well. My pages use both user stats and page-specific observables, and page-specific observables can be (and often are) dependent observables that depend on user stats.

from knockout.mapping.

Typically you would have a viewmodel per view (as the name implies :)) so I would propose that you have separate, but smaller, viewmodels that you then bind.

In your case, you'd have a UserViewModel, ProductViewModel and PurchaseHistoryViewModel. Then, if your purchase history needs a reference to the current user, you'd add a property "user" which refers to the UserViewModel.

This way there is a canonical source for the user everywhere in your app: UserViewModel. This also ensures all the different models can never get out of sync.

from knockout.mapping.

This seems like a somewhat reasonable solution, albeit at the cost of more code. Namely, I'd need a custom create callback that sets the view model's user property to the hopefully-already-retrieved UserViewModel. And thus, I suppose, some more synchronization code to make that hopefully into a definitely, or to fill in dummy properties while I wait, or the like.

It is worth pointing out that the benefits in your last paragraph are the same as those I am getting with my current solution, though. The canonical source for the user is always PageViewModel.user, and PageViewModel.selectedProduct's observables can refer to it and have dependent observables referencing it.

Thoughts?

from knockout.mapping.

Yes, you would probably need some synchronization logic to remove the duplication, but in your example, how are you making sure that the data in PageViewModel.user remains equal to the data in UserViewModel?

from knockout.mapping.

In my example, there is only one view model (PageViewModel); the server can return the JSON { user = ... } from Ajax request 1, and { selectedProduct = ... } from Ajax request 2, and using ko.mapping.fromJS with the same object as the third parameter then puts them both in the appropriate place. Honestly I thought this was the whole idea of letting ko.mapping.fromJS accept a third parameter.

from knockout.mapping.

Ah, I see what you are doing now. So you are building up a viewmodel based on data from multiple sources. I have to say this is not a use-case I had in mind originally.

But then I suggest to fix it by simply removing the mapping options from the target viewmodel object when calling 'ko.mapping.fromJS', thereby requiring you to always pass it. This is the intended case anyway, and the fact that the existing mapping is used is a side-effect of the implementation that is shared with 'ko.mapping.updateFromJS'.

Do you agree with this fix?

from knockout.mapping.

But then I would not be able to use updateFromJS. With my fix I can do this:

fromJS on { user = ... }, mapping options = userMapping
fromJS on { selectedProduct = ... }, mapping options = selectedProductMapping
every 2 minutes: updateFromJS on { user = ... }, no mapping options necessary

With your fix, I would have to never use updateFromJS, and always use fromJS while keeping track of the mapping options because now I need them every time I get new data. It would be nice if the part of my code that updates the user data doesn't have to know about the process of setting up the view model to receive user data.

from knockout.mapping.

After thinking about this more, my opinion is that the current interface is problematic because it isn't respecting the single responsibility principle. fromJS is responsible for:

1. Initializing the view model for the first time (most easily seen from the overload not accepting a view model);
2. Updating the view model if called subsequently, with the view model accepting overload; and
3. Setting mapping options, either for the current call only (your version) or for all calls now and in the future (my version).

In light of this, I think working toward a more SOLID interface would be the best approach. Here is my idea:

// All subsequent fromJS and updateFromJS calls with respect to viewModel use these.
ko.mapping.options(userMappingOptions, viewModel)
ko.mapping.options(productMappingOptions, viewModel)

// Throws an error if the view model already has observables corresponding to the passed data
ko.mapping.fromJS(userData, viewModel)
ko.mapping.fromJS(productData, viewModel)

// Throws an error if the view model does NOT already contain observables corresponding to the passed data
ko.mapping.updateFromJS(userData, viewModel)

This separates the three responsibilities.

An alternative would be to merge fromJS and updateFromJS, and have the resulting function take divergent code paths depending on whether the data it is passed already exists in the view model or not. This somewhat breaks SRP, but would probably be a more convenient interface for users.

I am happy to get my hands dirty and implement something like this if you think it's a good direction to go in. What do you think?

from knockout.mapping.

Wow, lots of thoughts!

The reason the mapping plugin stores the options with the generated output is because otherwise the lifetime of the options object cannot be determined.

In your 'ko.mapping.options' example this is not a big deal, but if you have a complex mapping setup that also gets invoked a lot (e.g. a grid updating in realtime) then you will introduce memory leaks.

Also, I don't think 'fromJS' is responsible for '2'. Updating the viewmodel should be done exclusively through 'updateFromJS'. The fact that 'fromJS' also updates an existing viewmodel is a side-effect that should probably throw an exception if you try it.

This is also why I don't agree with '3'. Currently 'fromJS' sets the mapping options, so that any subsequent 'updateFromJS' calls can reuse these mapping options for all calls now and in the future.

Could you maybe create a jsfiddle containing a small example of how you would expect the mapping plugin to work and to illustrate where it falls down, because I think we both have a different idea of how it is supposed to work. And I certainly want us to be aligned :)

from knockout.mapping.

Well, I'm not sure exactly what you're asking for that I haven't already provided. I would expect the mapping plugin to have some mechanism for building a view model from multiple sources, some of which may be updated in the future. And I would expect not to have to repeat myself with regard to mapping options. And ideally I would like the setting of mapping options to be decoupled from the retrieval of data from the server.

In the current version, fromJS is responsible for '2', because it takes a parameter that is a view model to mutate. If it were not responsible for '2', it probably shouldn't take this parameter, and should only return the view model constructed from the JS.

from knockout.mapping.

But the thing is, it's also responsible for '3', since 'fromJS' stores mapping options that are then used by all subsequent 'updateFromJS' calls. So you already don't have to repeat yourself with regard to mapping options.

from knockout.mapping.

But, it does not do so if you are assembling the view model from multiple sources, because it overwrites the mapping options! Which brings them back to my original pull request, which would fix the problem.

from knockout.mapping.

Ok, I see your point now. Apologies for taking so long to wrap my head around what you are trying to do!

from knockout.mapping.