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

Add support for STI #2407

Closed
tim-evans opened this issue Oct 22, 2014 · 12 comments · Fixed by #6272
Closed

Add support for STI #2407

tim-evans opened this issue Oct 22, 2014 · 12 comments · Fixed by #6272

Comments

@tim-evans
Copy link
Contributor

Having support to mark relations as having a Single Table Inheritance model would be nice.

We have STI in our app, and see performance hits because we have to fetch the base class (Person) and the implementing class (Client) for ED to understand what's going on.

Any thoughts / ideas on this?

@miguelcobain
Copy link

Totally 👍

I've written a proposal for this in the Ember discussion forum, back in Nov '13: http://discuss.emberjs.com/t/proposal-class-centered-polymorphism/3457/1

I really think this is the way to go.
We're basically giving some meaning to subclassing a model. I find this really natural.

@tim-evans
Copy link
Contributor Author

This probably belongs in this function:

https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/store.js#L1193-L1212

But will have to wait until MODEL_FACTORY_INJECTIONS is removed from ember.

@tim-evans
Copy link
Contributor Author

Any updates on this? I recently ran into a bug that messed up data due to the way I'm trying to hack around ED not supporting this

@leifcr
Copy link

leifcr commented Sep 15, 2015

I'm currently trying to get this working with Emberfire (FirebaseExtended/emberfire#245), but it would be better if it was part of ED.

@tim-evans
Copy link
Contributor Author

qunitjs/qunit#632

@fivetanley
Copy link
Member

@tim-evans What is the path forward here?

@tim-evans
Copy link
Contributor Author

I think the path forward is to either provide the ability to mark models as STI or implicitly use STI modeling if models are inherited. One way to do this is mixins, another inheritance. I think Mixins may be the most flexible, since some backend stores may support multiple inheritance. Anyways, I'd like to be able to fetch a model of a generic type and be returned the specific type of that object instead of ED going back to the server to fetch it. (It should obviously do that if it's not in the store).

Complications include:

  • Mismatched types. Fetching a model of type Event and getting a return type of Workshop may be weird to handle. The server would be correct in this case; it's just specifying the sub-type. Validations could be done on the fly for this, checking the STI tree for the model to see if they are in the same tree. If so 👍.
  • Users expecting old behavior. Not sure about this one, as I feel most users assume this is the default, but this will definitely break what has happened previously.

@leifcr
Copy link

leifcr commented Jan 16, 2016

Would this be the same as proposed in this rfc emberjs/rfcs#67?

@tim-evans
Copy link
Contributor Author

Yes and no. Same desired output, different approach. STI is a subset of polymorphism IMO. The STI support is more "magical" than the polymorphic approach. I'm fine with approach laid out in the RFC, but it doesn't address systems that use STI in a way that maps well ORM-wise. A top-level mixin on the root model may very well be "good enough", where it forces relationships with the given type to be polymorphic.

So, I think we can build first-level STI support on top of the proposals in the RFC. I'm afraid of some push-back of additional complexity from core with an STI mixin, but I think in the end, it's worth it.

@navels
Copy link

navels commented Oct 13, 2022

This was closed as completed . . . can someone summarize what support there actually is for STI?

@runspired
Copy link
Contributor

runspired commented Oct 14, 2022

@navels use the identifiers API for properly merging identities (see rfc, tests, or docs for the various methods https://api.emberjs.com/ember-data/4.7/functions/@ember-data%2Fstore/setIdentifierGenerationMethod). Support for this landed ~3.10 iirc, maybe earlier.

Use the explicit polymorphic relationship APIs for polymorphic relationships: see https://rfcs.emberjs.com/id/0793-polymporphic-relations-without-inheritance support for this landed in 4.7

@navels
Copy link

navels commented Oct 14, 2022

thanks!

. . . time passes . . .

Oh geez, that identifiers rfc is gold. I had the polymorphic relationships pretty well understood, had found that rfc easily, but somehow the other one had escaped me and instead I found various half-implemented and abandoned implementations of different things that may have addressed STI.

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.

6 participants