Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for nested props #148

Open
mike-engel opened this issue Mar 12, 2015 · 4 comments
Open

Allow for nested props #148

mike-engel opened this issue Mar 12, 2015 · 4 comments

Comments

@mike-engel
Copy link

I think it would be a good idea to allow for nested props when extending ampersand-state or ampersand-model. I'm not quite sure what the best way would be to do this, but right now to nest props in a model it's a pain. I don't expect this scenario to be all that uncommon. If there's interest, there'd need to be a discussion about the best syntax, etc.

Here's an example response from our server:

[
  {
    "id": 1873,
    "createdOn": "2015-03-12T19:38:28",
    "type": "graph",
    "graph": {
      "type": "bar",
      "timeframe": 7,
      "data": [
        {
          "x": "2013-03-27T01:37:27",
          "y": 2844
        },
        {
          "x": "2013-03-28T09:39:25",
          "y": 3879
        }
      ]
    }
  },
  {
    etc...
  }
]

Right now, I have to do something like this for nested props:

var AmpersandModel = require( 'ampersand-model' ),
    GraphModel = require( './graph' );

module.exports = AmpersandModel.extend({
  props: {
    id: 'number',
    createdOn: 'string,
    type: 'string',
    graph: {
      type: 'model',
      default: GraphModel
    }
  }
});

It would be super nice to have the entire definition in one file. Something like:

var AmpersandModel = require( 'ampersand-model' ),
    GraphModel = require( './graph' );

module.exports = AmpersandModel.extend({
  props: {
    id: 'number',
    createdOn: 'string,
    type: 'string',
    graph: 'object',
    graph.type: 'string',
    graph.timeframe: 'string',
    graph.data: 'collection'
  }
});

or

var AmpersandModel = require( 'ampersand-model' ),
    GraphModel = require( './graph' );

module.exports = AmpersandModel.extend({
  props: {
    id: 'number',
    createdOn: 'string,
    type: 'string',
    graph: {
      type: 'object',
      children: {
        type: 'string',
        timeframe: 'string',
        data: 'collection'
      }
    }
  }
});

Maybe I just haven't experimented enough with this where the first one would work. Thoughts?

@pgilad
Copy link
Member

pgilad commented Mar 13, 2015

Just use:

var Graph = require('./models/graph');
//...
children: {
  graph: Graph
}

and it will be automatically accessible and will bubble events and all ;-)

see https://github.com/AmpersandJS/ampersand-state#children-ampersandstateextend-children--profile-profile--

@mike-engel
Copy link
Author

Thanks @pgilad. While I understand that's a way to do that, it's not effecient. If I have a JSON response three levels deep, I've created three files, two of them unnecessary if there was a way to nest props.

@lukekarrys
Copy link
Contributor

Hey @mike-engel, while I agree that there could be an easier way, I'm not sure what or how that would look internally (or how feasible it would be to implement).

I've run into this problem in the past, and my solution was to not break up the nested child models into separate files, and instead create the model definitions as local variables. I know this isn't a solution, but it at least helped me not create too many extra files.

@pgilad
Copy link
Member

pgilad commented Mar 23, 2015

@lukekarrys I think a possible solution would be to dynamically create the children & collections in order to support nested properties. I would create them all with default values (allowing any sub-property type). and thus user will have access to the nested properties (including events).

If a user wants to customize and/or add more logic to the nested props, he should create a child model or collection by himself

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

No branches or pull requests

4 participants