Better record polymorphism

text/0000-find-polymorphism.md

@@ -0,0 +1,155 @@
+- Start Date: (fill me in with today's date, YYYY-MM-DD)
+- RFC PR: (leave this empty)
+- Ember Issue: (leave this empty)
+
+# Summary
+
+Add a `{ polymorphic: true }` flag to `store.findAll`, `store.findRecord`, `store.peekAll`,
+`store/peekRecord` which would include subclasses in the results.
+
+# Motivation
+# Current State
+Model Polymorphism is a popular and widely used feature of Ember Data. It is also supported
+out of the box in JSONApi.
+ Polymorphism is a natural way to model various domains, such as having `Comment` superclass
+and `TextComment`, `VideoComment` subclasses, or a `Shape` superclass that has `Line` and `Circle` as subclasses.
+
+A more advanced use case which is supported today is using Mixins for polymorphism.
+Consider a Social Network app, where several different object can be commented on.
+
+```js
+var Commentable = Ember.Mixin.create({
+  comments: hasMany('comment')`
+});
+```
+
+```js
+var BlogPost = DS.Model.extend(Commentable, {
+  title: DS.attr();
+});
+```
+
+```js
+var Comment = DS.Model.extend({
+  parent: DS.belongsTo('commentable', { polymorphic: true });
+});
+```
+
+The above design allows Ember Data to keep parent/comments OneToMany relationship in sync, which
+would not be possible without being explicit about the polymorphic mixin.
+
+However, today Ember Data only supports polymorphism in relationships. If for example you are building
+an online mocking tool, your models might look like:
+
+```js
+var Shape = DS.Model.extend({
+  color: DS.attr();
+});
+```
+
+```js
+var Circle = Shape.extend({
+  radius: DS.attr();
+});
+```
+
+```js
+var Line = Shape.extend({
+  length: DS.attr();
+});
+```
+
+```js
+var Canvas = DS.Model.extend({
+  shapes: DS.hasMany('shape', { polymorphic: true });
+});
+```
+
+Currently you can do `canvas.get('shapes')` and get Lines and Circles, but you cannot do `store.findAll('shape')`
+and get all the Lines and Circles.
+
+# Detailed design
+
+##FindAll and PeekAll
+`store.findAll('shape', { polymorphic: true })` would return a live array that would have all the shapes and all of the
+shape subclasses.
+
+We would implement a private method, `store._subclassesFor`:
+```
+  /*
+    Returns all the Model types that are subclasses of `type`
+  */
+  store._subclassesFor(type)
+```
+
+`store.findAll('shape', { polymorphic: true })` would listen to live arrays of 'shape' and of all the classes and concat them.
+`store.peekAll('shape', { polymorphic: true })` would behave in the same way.
+
+##FindRecord and PeekRecord
+
+###Fetching case
+In current Ember Data, requesting `store.findRecord('shape', 1, { reload: true })` and getting back `{ data: { id:1, type: 'line' } }` is an error
+condition.
+
+`store.findRecord('shape', id, { reload:true,  polymorphic: true })` and getting back `{ data: { id:1, type: 'line' }}` would not error out, but
+rather check if there exists a different shape with `id:1` in the identity map, and if it does Error out. If it does not, it would
+return the Line DS.Model from the findRecord promise.
+
+###Caching case
+`store.findRecord('shape', id)` currently checks the identity map for existence of the shape record with `id:1` with `type:'shape'`
+`store.findRecord('shape', id, { polymorphic: true })` would also check the identity map of it's subclasses.
+'
+##URLs
+We would make no assumptions about URL structure with regards to polymorphism. In particular `line.save()` would go to `/lines` by default.
+It is very easy for the user to override this and in the LineAdapter.
+
+# Drawbacks
+Added complexity.
+Having to pass `{ polymorphic: true }` to each invokation of find is repetative and noisy, and might be easy to forget. However, if you forget,
+you will immediately see that you are not getting data, and debug. (compare with alternatives, where bugs occur later).
+Composition over Inheritance
+This approach is slightly refactor hostile. If you start with a Model structure that only has shapes and later break them up
+into subclasses you have to track down every place you used `find` and pass `{ polymorphic: true }`. 
+
+# Alternatives
+Kris proposed that we should have an `abstractBaseClass` property on models.
+The above example would be rewritten as:
+
+```js
+var Shape = DS.Model.extend({
+  color: DS.attr();
+});
+Shape.reopenClass({
+  abstract: true
+});
+```
+
+```js
+var Circle = Shape.extend({
+  radius: DS.attr();
+});
+```
+
+```js
+var Line = Shape.extend({
+  length: DS.attr();
+});
+```
+
+This approach has several advantages:
+`store.findAll('shape')` would do the right thing without having to be passed `{ polymorphic: true }`
+It would be much easier for `line.save()` to automagically go to /shapes
+
+However, I believe that the drawbacks are more significant:
+
+  1.This approach is much more restrictive, you can either always include or not include subclasses,
+    but cannot decide at invocation time if you want to opt out
+  2. In larger teams it might be surprising that one day `store.findAll('comments')` is returning other
+random models just because somebody on the team added an abstractBaseClass: true to the `comment` model.
+This is a bigger problem than forgetting to add `{ polymorphic: true }` because errors can happen in a completely
+different part of the app.
+  3. This approach is incosistent with our current relationship polymorphism. It would require us to either deprecate
+`hasMany({ polymorphic: true })` (and we just released 1.0 :( ) or to use an inconsistent mixed approach.
+
+# Unresolved questions
+