ebryn / ember-model

A lightweight model library for Ember.js
MIT License
526 stars 162 forks source link

Inheritance / polymorphism #351

Open ahacking opened 10 years ago

ahacking commented 10 years ago

I am wanting to implement some basic polymorphism / inheritience but have not been able to find any information on how to approach this for Ember.Model.

I did see a mixin approach for Ember.DS documented here: https://coderwall.com/p/hi6hya and thought a similar approach may work for Ember.Model.

I think this is more of a model concern than an adapter concern and Ember.Model.load() looked like the appropriate place to start...

So before I burn any effort on this can you see any obvious show stoppers, guidance, or better ideas for adding some simple polymorphic behaviour to Ember.Model?

ebryn commented 10 years ago

@ckung was looking into this recently. I personally haven't ever had a need for polymorphism in Ember Model itself. Would love to see/hear use cases.

ahacking commented 10 years ago

I have a number of use cases in my project.

For example, I have different kinds of contacts (Person and Organisation). Person gets further refined as does Organisation. As such I want to treat them generically (global address book search/filtering/sorting), and specifically at other times (eg managing an employee, or selecting a business unit). There is a single endpoint for contacts on my server, although I could implement type specific endpoints, its not the desired approach as it creates unnecessary duplication on the server side and does not facilitate global contact search and filtering and other management operations through a single endpoint. Having multiple instances of say 'John' as a generic 'Contact' and 'John' as an 'Employee' and keeping those two models in synch would not be nice and complicate view rendering no-end.

All contacts share some common attributes, but they also have additional attributes and relationships based on their type, eg an organisation has many members which doesn't apply to People. In some cases the type of the related model could be more specific. I also use an Ember validations extension and would like to refine my validations specific to the type as well.

Hence wanting to be able to use mixins to extend the model class with the required facets based on model attributes eg 'type' or possibly other attributes (or rules). I am not really wanting a pure declarative type inheritance approach as I believe mixins provide more flexibility anyway, and from what I have looked at in the Ember.Model code base keeping a single base model type would be a simpler approach because of the way the identity map currently works.

dwickern commented 10 years ago

I solved this by flattening the hierarchy into the base model, like you would in a relational database. Then I used the type discriminator to decide which template to display.

Here's a quick example http://emberjs.jsbin.com/pahutedo/2/edit?html,js,output

(edit: fixed example)

ebryn commented 10 years ago

@ahacking does @dwickern's technique work for your use cases?

ahacking commented 10 years ago

Hmm well the kitchen sink approach that @dwickern suggests is not very pretty and requires a lot of ugly and fragile conditional logic in the validation rules.

Is there no way to dynamically extend a model instance on load via mixins? That's all I'm looking for.

ebryn commented 10 years ago

@ahacking that sounds possible. can you provide a jsbin example of what you would expect to work?

guilhermeaiolfi commented 10 years ago

Usually you would like to get both: View AND Controller instance. In the @dwickern approach, you only get the View. That's not really the responsability of the model as this issue is all about. But I thought that it should be considered when creating a solution for this.

ahacking commented 10 years ago

I got very close to what I wanted using the following model mixin, but no cigar:

App.PolymorphicModel = Em.Mixin.create
  mixinFromData: (data) ->
    App[data.type]  # TODO use injection voodoo once container issues are sorted
  load: (id, data) ->
    mixin = @mixinFromData data
    @reopen(mixin) if mixin
    @_super arguments...

The problem is that reopen() is intended to only extend class prototypes not instances. The use of Em.attr() on a mixin does not work, as whenever metaForProperty() is called for an attribute defined on a mixin it throws an error since it can't find the meta data for the property on the instance constructor.

Ember just doesn't support extending/reopening instances so the other approach to consider is real classes. I can see some problems with that since it doesn't seem possible to change a records class dynamically, and the way Ember-Model works it would need to be able to do that. Also I can see that Ember-Model caching and lookup would require some amount of rework to restrict find etc to subclass scope correctly.

So reopen() on instances would (if it worked) be the lightest touch approach but it requires internal changes in Ember to occur so that property meta-data works and probably a whole bunch of other unforeseen problems with observers and bindings are also solved.

Unless anyone has some better ideas feel free to close.