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

Polymorphic Pattern via (seemingly arbitrary) Mixin w/ JSONAPI #3919

Closed
hhff opened this issue Nov 12, 2015 · 6 comments
Closed

Polymorphic Pattern via (seemingly arbitrary) Mixin w/ JSONAPI #3919

hhff opened this issue Nov 12, 2015 · 6 comments

Comments

@hhff
Copy link

hhff commented Nov 12, 2015

Hi All!

Apologies if this has been resolved, but seeing as I'm on Ember Data 1.13.9 I thought I'd report this.

I'm working on some basic polymorphic stuff with Active Model Serializers 0.10.0.rc2, and its JSONAPI Adapter.

I'm used to the "Mixin" pattern for Polymorphism in Ember, however I've noticed that this works without any hitches:

// mixins/statisticable.js

import Ember from 'ember';
import DS from 'ember-data';

export default Ember.Mixin.create();
// models/recap.js

import DS from 'ember-data';

// ---> NOTE: Nothing is mixed in here!

export default DS.Model.extend({
  statistics: DS.hasMany('statistics')
});
// models/statistic.js

import DS from 'ember-data';

export default DS.Model({
  statisticable: DS.belongsTo('statisticable', { polymorphic: true })
});

In the case above, recap.js does not have Statisticable mixed in. It simply requires that mixin:statisticable exists on the container, and doesn't check that it's mixed into the class Polymorphic class.

Without the mixin existing, ED complains that there's no model called statisticable.

So it seems like the requirement for there to be a mixin:statisticable is kinda arbitary? Or am I missing something?

@rwjblue
Copy link
Member

rwjblue commented Nov 12, 2015

You have to have either a model or a mixin when using polymorphic types. See here for the current implementation.

@hhff
Copy link
Author

hhff commented Nov 13, 2015

Yup - I understand (and quite like) the "official" pattern. Just seems like a stumbling block that the statisticable model (inverse to the polymorphic declaration) doesn't actually need the Mixin applied to it - it simply needs to exist.

In the above case, does the "dummy mixin" actually do anything? Again - I may be missing something.

@igorT
Copy link
Member

igorT commented Nov 13, 2015

In this case the statistics relationship might not be wired up correctly, because you didn't declare it on the parent class but should otherwise work

@hhff
Copy link
Author

hhff commented Nov 14, 2015

Thanks dudes! Recap#statistics is declared in the models/recap.js so that relation should be fine.

Again - I'm more highlighting that it's a little confusing there's no check to ensure the Mixin is actually applied to the "polymorphic" Model (Recap in this case).

Feels arbitrary that a Mixin even has to exist here. Just wanted to let you know it may be confusing for beginners!

@wecc
Copy link
Contributor

wecc commented Oct 21, 2016

I'm going to close this as it's working as intended by not applying the mixin – it just has to exist.

In #4375 we're tracking the improvement of docs around polymorphic relationships.

@wecc wecc closed this as completed Oct 21, 2016
@hhff
Copy link
Author

hhff commented Oct 22, 2016

thanks @wecc !

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

No branches or pull requests

4 participants