Options notation

[DzenisevichK]

Options format:

How can I use objects with copy, include, ignore properties in fromJS/toJS calls?

This is not possible with current options notation or I'm wrong?

Options format is: {   copy|include|ignore: <paths to properties>,   <paths to properties>: {     key|create|update: <callbacks>   } }

To fix this we need to change format to: {   <paths to properties>: copy|include|ignore,   <paths to properties>: {     key|create|update: <callbacks>   } } or as variant of this - replace <paths to properties> with real object structure...

Currently <paths to properties> may be only single property name for key, create, update callback. This example will fail: {   "a.b": {     key|create|update: <callbacks>   } } This was not fixed by #63 issue because related to indexes of arrays.

Options updating:

it's also not clear topic of options merging/updating for nested mapping. We cannot use options for root in nested mapping in create/update because this will introduce dependencies between structures of objects.

As a solution for this I may suggest to use callback or <nested options definition> with create.

Options and arrays:

Currently all arrays are processed with compareArrays function which also sorts arrays. And how is it possible to use indexes of items in <paths to properties>?

[sagacity]

I would prefer the syntax to match the object structure as closely as possible. So if you have an object that looks like a: { b: { c: "d" } } I would like the mapping options to look like this:

options: {
   a: {
      create: ...,
      include: true,
      b: {
         c: {
            ignore: true
         }
      }
   }
}

And it should also work when merging options, so taking the above options, if I still want to add "c" I would like to be able to do something like:

var mapped = ko.mapping.fromJS(data, options);
var unmappedWithCIncluded = ko.mapping.toJS(mapped, { a: { b: { c: { ignore: false } } } });

This will all have to happen in a 3.0 branch because they are breaking changes.

[DzenisevichK]

I would prefer the syntax to match the object structure as closely as possible - agree... but how to map {   a: {     create: true   } } ?

[sagacity]

You mean if the original object also has a property called create? That would certainly be a problem. Maybe we can prefix our options (e.g. __create: true in the options object) but that seems hacky. Do you have an idea?

[DzenisevichK]

This variant will fix this case: {   <paths to properties>: copy|include|ignore,   <paths to properties>: {     key|create|update: <callbacks>   } }

Common rule: property name - path(s) definition, property value - options for related value(s)

It's also shorter and possible to introduce patterns...

[sagacity]

That's true. Is there a special reason you've separated the two components? Instead of doing just:

{
  <path to properties>: {
    copy|include|ignore: true|false
    key|create|update|: <callbacks>
  }
}

[DzenisevichK]

It seems that copy|include|ignore and key|create|update cannot be used together or? If yes then variant: {   id: "copy" } is more shorter...

[sagacity]

It's true that they cannot be used all at once. If you 'copy' you can't 'ignore'. But maybe it would be better to call it something like a 'mapping action', so like:

If you want to map id yourself:

   id: {
      fromJS: function() { ...identical to create callback... }
      toJS: function() { }
   }

If you want to ignore id when unmapping (when calling ko.mapping.toJS):

   id: {
      toJS: "ignore"
   }

And so on. So you can specify for toJS/fromJS what the mapping plugin needs to do for that property. And this could still work with the property notation like "a.b".

[sagacity]

@DzenisevichK Let's do it in two steps:

• First, merge the specific options (the options that you pass into toJS) at every level of the tree. This way, the old behavior for include and ignore will still work. Once this is complete, I can merge it in and release your fixes as 2.1.1 or possibly 2.2.0.
• Then, we can take some time and work on 3.0 where the whole options handling is improved based on the notes in this thread.

[DzenisevichK]

This is not correct to merge options from different levels...

[sagacity]

I know, but this is the current behavior of the old code. I would like to release a 2.1.1 version with your fixes so that at least problems such as #68 will not happen.

Then, we can release a 3.0 with the correct handling, but then we can also improve the options themselves, write new documentation, etc.

[DzenisevichK]

Ok, but at weekends. Also I will try to reduce ko.mapping (at least for own use) to have logically correct/workable functional.

[sagacity]

Ok! Please keep me updated :)

[vantreeseba]

So, the issue with items not being pushed into observable arrays mapped through the mapping.fromJS or fromJSON, looks like it is occuring not because the array itself is acting strange.

So, if I do foo = ko.mapping.fromJS({prop: 1, list: [] };

And then, foo.list.push({id:5}); and finally, ko.mapping.toJSON(foo), it is returns "{ 'prop': '1', 'list', [{}] })". BUT, if you set the mappedCreate callback as function(options){ return ko.mapping.fromJS(options.data); } It returns the whole object when doing foo.list.push({id: 5}).

So it looks like, when toJSON parses the mapped properties, it is ignoring non-mapped properties on items in the mapped array.

I have "temporarily" fixed this on my own code by doing var item = hasCreateCallback() ? createCallback(value) : ko.mapping.fromJS(value); (this is on line 468).

This is not a good solution in my opinion though.

If this issue has already been fixed in #68, good, but the current copy of this from github is not working for me, hence my little fix.

[sagacity]

Does it work for you in 2.1.0? https://github.com/SteveSanderson/knockout.mapping/blob/f996b65ce93ccb934746fdb581dc2060359f9326/build/output/knockout.mapping-latest.debug.js

[steveluscher]

Where you do see this discussion ending up? Right now, we have a mapping in one of our example apps for knockout.meteor that (once making its way through the plugin) results in a mapping like this:

{
  copy: ["_id"],
  key: function (item) { return ko.utils.unwrapObservable(item._id); }
  done: {
    create: function (options) { … }
  }
}

This works with knockout.mapping v2.1.2 or DzenisevichK's knockout.mapping v2.3.0, but it breaks horribly with the master branch's v2.2.1; the create mapping on the done property gets discarded. I suspect this is because of the unrest surrounding the options notation, particularly whether to allow a mix of root-level and property-level directives.

We at knockout.meteor would just like to know which direction the future is in, so that we can remain compatible with master.

[DzenisevichK]

I refused to use ko.mapping. I implemented ko.updateableArray and use it with manually wrapping/unwrapping observables and don't have any problems...

[sagacity]

At the moment the options notation will remain as it is in v2.2, simply because there were too many backwards compatbility issues with the v2.1 release.

The new notation might still surface, but there is no timeframe for that.

@steveluscher If possible, can you create an even more minimal reproduction case on jsfiddle? Maybe there are some alternative mappings we can try.

[sagacity]

@DzenisevichK Btw, I would again like to thank you for all the work you put in and I hope we can find a way to integrate features from your fork for a v3.0 release. But I will have to add more unit tests for all the behaviors so that we can ensure that everything is either backwards compatible or generates a nice error/warning when users do something that is not supported anymore.

[steveluscher]

I think I figured it out @RoyJacobs. In making this fiddle, I learned that you have to move the "root" mappings into a property named "" if you want them to co-exist with descendant property mappings. Otherwise, the descendant property mappings will be ignored.

var mapping = {
  "": {
    copy: ['_id'],
    key: function (item) { return item._id; }
  },
  cities: {
    create: function(opts) { ... }
  }
};

I'd prefer if Knockout Mapping would move mappings that appear to be root-level ones into the "" property, and leave other ones well alone.

When's the Knockout Mapping documentation slated to be updated to reveal this?

[sagacity]

@steveluscher Actually you shouldn't need to use the "" property. It should work as you indicate (everything in root-level being moved to the "" property behind the scenes) so I think this is something that does need to be fixed.

[gliljas]

What about...

var mapping = { copy : [.....], match : [ ["firstName" , { //nested mapping of the firstName property }], [function(o){ return o.name.endsWith("Price"); } , { //nested mapping of properties with a Price name convention }] ] };

A bit verbose, but should solve most cases

[manishkrai]

Hi Friends,

I am facing one ignore mapping issue. Kindly help me to resolve the same.

I have a collection of employees and each employee contains below fields. Key Name Amount

I would like to convert the employees array to observable with Key and Name for each employee as observable but Amount should not be observable. I am trying below mapping, But seems it not working.

var mapping = { 'ignore': ["Amount"] }

self.BatchEmployees = ko.mapping.fromJS(listOfEmployees, mapping);

In result every fields are coming as observable.

[crissdev]

@manishkrai Here's an example of what you need to do to have your code working. The idea is to define the copy option in the right place.

var options = {
    key: function(item) {
        return item.Key;
    },
    create: function(options) {
        return ko.mapping.fromJS(options.data, {
            copy: ['Amount']
        });
    }
};

var data = [
    {Key: '1', Name: 'John', Amount: 100},
    {Key: '2', Name: 'Doe', Amount: 200}
];

ko.mapping.fromJS(data, options);

http://jsfiddle.net/3gygwbu6/