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

subModelTypes as class references instead of string class names #508

Open
ghost opened this issue Dec 10, 2014 · 2 comments · May be fixed by #509
Open

subModelTypes as class references instead of string class names #508

ghost opened this issue Dec 10, 2014 · 2 comments · May be fixed by #509

Comments

@ghost
Copy link

ghost commented Dec 10, 2014

Why subModelTypes classes can be only strings?
Is it possible/correct to have subModelTypes class references?

For example this code

Mammal = Animal.extend({
    subModelTypes: {
        'primate': 'Primate',
        'carnivore': 'Carnivore'
    }
});

Primate = Mammal.extend();
Carnivore = Mammal.extend();

MammalCollection = AnimalCollection.extend({
    model: Mammal
});

to work the same as this

Mammal = Animal.extend({
    subModelTypes: {}
});

Primate = Mammal.extend();
Mammal.prototype.subModelTypes.primate = Primate;

Carnivore = Mammal.extend();
Mammal.prototype.subModelTypes.carnivore = Carnivore;

MammalCollection = AnimalCollection.extend({
    model: Mammal
});

In our application, we create a base class, send it in event, anyone can listen the event and register its subClass

// file: events.js
(function(){
  window.events = {};
  _.extend(window.events, Backbone.Events);
})();

// file: primate.js
(function(){
  events.on("register:mammals", function(Mammal) {
    var Primate = Mammal.extend({defaults: {title: 'Primate'}});
    Mammal.prototype.subModelTypes.primate = Primate;
  });
})();

// file: carnivore.js
(function(){
  events.on("register:mammals", function(Mammal) {
    var Carnivore = Mammal.extend({defaults: {title: 'Carnivore'}});
    Mammal.prototype.subModelTypes.carnivore = Carnivore;
  });
})();

// file: mammals.js
(function(){
  var Mammal = Backbone.RelationalModel.extend({
    subModelTypes: {}
  });

  events.trigger("register:mammals", Mammal);

  var MammalCollection = Backbone.Collection.extend({
    model: Mammal
  });

  var mammals = new MammalCollection;
  mammals.reset([{type: 'primate'}, {type: 'carnivore'}]);

  console.log(mammals.toJSON());
  console.log(
    mammals.at(0).get('type'),
    mammals.at(0) instanceof Mammal.prototype.subModelTypes[mammals.at(0).get('type')], 
    mammals.at(0).get('title')
  );
  console.log(
    mammals.at(1).get('type'),
    mammals.at(1) instanceof Mammal.prototype.subModelTypes[mammals.at(1).get('type')],
    mammals.at(1).get('title')
  );
})();

If I change the getObjectByName method from

getObjectByName: function( name ) {
    var parts = name.split( '.' ),
        type = null;

    ...
}

to

getObjectByName: function( name ) {
    if (name.prototype instanceof Backbone.RelationalModel) {
        return name;
    }

    var parts = name.split( '.' ),
        type = null;

    ...
}

then the example above seems to work. But I am not sure if it's the right way to do this. Also I don't know how this will affect the store.


Because it is not possible to have subModelTypes as class references, we did this:

  • Instead of registering {'type': ClassReference} in the subModelTypes property, we created a custom property MammalCollection.types = {}; and register there the subTypes.
  • Then we made the MammalCollection.model a function that is called on .reset() and search class by type in MammalCollection.types and return the model instance

Then we faced the problem that MammalCollection.model is replaced by Backbone-relational. More details in #472

@ghost ghost linked a pull request Dec 10, 2014 that will close this issue
@ghost
Copy link
Author

ghost commented Dec 10, 2014

I created a pull request with some tests in case you will accept this feature. If this is not the way it should be solved, fell free to close that pull request

@hippich
Copy link

hippich commented Jan 30, 2015

This looks like a right change.

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

Successfully merging a pull request may close this issue.

2 participants