Inconsistent parsing for children and collections

[jlit]

Define a 'gear' model and gear-collection:

var Model = require('ampersand-model');

module.exports = Model.extend({
    props: {
        id: 'string',
        name: 'string'
    },
    parse: function (data) {
        data.name = data.name + '_modified';
        console.log('models/gear/parse: ', data);
        return data;
    }
});

// gear-collection
var Collection = require('ampersand-rest-collection');
var Gear = require('./gear');

module.exports = Collection.extend({
    model: Gear,
});

Now define a widget model that has both a singleton child gear and a collection of gears:

var Model = require('ampersand-model');
var GearModel = require('./gear');
var GearCollection = require('./gear-collection');

module.exports = Model.extend({
    props: {
        id: 'string',
        name: 'string'
    },
    children: {
        defaultGear: GearModel       
    },
   collections: {
        gears: GearCollection
    }
});

The names of all the gears in the gears collection with end in '_modified' but the name of the defaultGear will not.

It looks as if children are instantiated using the constructor with attributes which then defaults to options parse=false but the models hydrated via the collection have parse=true. Not sure how to override that inconsistent behavior.

[kamilogorek]

You're right. Children are initialized on L442 and there's no parse set.
I verified and this change would not break any of our current tests.
@AmpersandJS/core-team what's your take on this one?

[wraithgar]

Can you show the instantiation code for this? maybe a requirebin or something? I'm having trouble tracking through the states that would cause what you're seeing.

Children are a little different than collections in that they're instantiated empty and populated if there is data, whereas a collection only instantiates a model when it has the data for it. So this may not be as simple of a fix as we think (or I'm overthinking it).

[jlit]

Take the demo app. Change as follows:

Add some animal data to the fakeapi:

var _ = require('underscore');

var people = [
    {
        id: 1,
        firstName: 'Henrik',
        lastName: 'Joreteg',
        coolnessFactor: 11,
        pets: [
            {
                _reallystupidattributename: 'Spot',
                type: 'dog'
            },
            {
                _reallystupidattributename: 'Fluffy',
                type: 'dog'
            }
        ],
        mascot: {
            _reallystupidattributename: 'Nils Olav',
            type: 'penguin'
        }
    },
    {
        id: 2,
        firstName: 'Bob',
        lastName: 'Saget',
        coolnessFactor: 2,
        pets: [
            {
                _reallystupidattributename: 'Killer',
                type: 'cat'
            }
        ],
        mascot: {
            _reallystupidattributename: 'Frodo',
            type: 'lion'
        }
    },
etc..

Create an animal model and an animals collection.

var AmpersandModel = require('ampersand-model');

// models/animal
module.exports = AmpersandModel.extend({
    props: {
        name: ['string', true, 'Unknown'],
        type: ['string', false, '']
    },
    parse: function (data) {
        data.name = data._reallystupidattributename;
        return data;
    }
});

var Collection = require('ampersand-rest-collection');
var Animal = require('./animal');

// models/animals
module.exports = Collection.extend({
    model: Animal,
});

Add mascot and pets and derived petNames property to person model:

var AmpersandModel = require('ampersand-model');
var AnimalCollection = require('./animals');
var AnimalModel = require('./animal');

// models/person
module.exports = AmpersandModel.extend({
    props: {
        id: 'any',
        firstName: ['string', true, ''],
        lastName: ['string', true, ''],
        coolnessFactor: ['number', true, 5]
    },
    children: {
        mascot: AnimalModel
    },
    collections: {
        pets: AnimalCollection
    },
    session: {
        selected: ['boolean', true, false]
    },
    derived: {
        fullName: {
            deps: ['firstName', 'lastName'],
            fn: function () {
                return this.firstName + ' ' + this.lastName;
            }
        },
        avatar: {
            deps: ['firstName', 'lastName'],
            fn: function () {
                return 'http://robohash.org/' + encodeURIComponent(this.fullName) + '?size=80x80';
            }
        },
        editUrl: {
            deps: ['id'],
            fn: function () {
                return '/person/' + this.id + '/edit';
            }
        },
        viewUrl: {
            deps: ['id'],
            fn: function () {
                return '/person/' + this.id;
            }
        },
        petNames: {
            deps: ['pets'],
            fn: function () {
                return this.pets.pluck('name').join(', ');
            }
        }
    }
});

I changed the person view and template to render the data to the screen but you can just inspect app.people. Look at pets[].name and mascot.name:

[HenrikJoreteg]

Hmm... so this is a bit tricky because as @wraithgar pointed out, the children get instantiated before they have data, so there's nothing to parse at that time.

So by the time it gets the data. It's just doing a set() which doesn't parse again...

[HenrikJoreteg]

potentially we could pass data to _initChildren, but i'm not sure what other consequences that would have.

[kamilogorek]

One way to change this behavior would be to persist parse option on initialization and change it only if it has been passed again during set/reset. This way state would know how to apply the data on it's own.

[pgilad]

Your parse is not doing what it's supposed to be doing:

parse: function (data) {
        data.name = data._reallystupidattributename;
        return data;
    }

Is basically manipulating the data. parse is meant to unenvelope the data, not manipulate it. That means if server returns something like:

{
  error: null,
  code: null,
  response : {}
}

You can use parse to get the response property.

If you want to remap name, just define it as a derived property.

[jlit]

Finally! Someone to tell me what my code is supposed to be doing. Where have you been all my life?

Sigh.

From the Ampersand docs:

parse is called when the state is initialized, allowing the attributes to be modified, remapped, renamed, etc., before they are actually applied to the state. In ampersand-state, parse is only called when the state is initialized, and only if { parse: true } is passed to the constructor's options:

[pgilad]

Don't mind the docs (hopefully you can improve them for the new comers with a PR).

Read backbone docs, they are the original concepts behind ampersand:
http://backbonejs.org/#Collection-parse
And
http://backbonejs.org/#Model-parse

[jlit]

Wishful thinking doesn't change the docs or the intention of parse. Your assertion that parse "is meant to unenvelope the data, not manipulate it" has no basis in fact.

Override this if you need to work with a preexisting API, or better namespace your responses.

[pgilad]

I see so this was Poo's Law at it's best, as I thought I was actually helpful. Either way I've never needed to parse non-server responses, and perhaps that is the way I design my code.

Considering making nested children/collections parse on init - I can live with that, only if it's opt-in.