[CLEANUP beta] Remove ObjectController

[cibernox]

ArrayController will be removed in a different PR.

Some tests for itemController that used ObjectController could have been refactored to use a basic controller, but since it's also a deprecated functionality I just deleted them too.

[stefanpenner]

Some tests for itemController that used ObjectController could have been refactored to use a basic controller, but since it's also a deprecated functionality I just deleted them too.

itemController is also dead, so i think those tests can likely just be removed? Although what you endedup doing also seems fine.

[cibernox]

@stefanpenner They will be removed eventually (and actually #11484 removes more), but since it's not directly related to this (probably it's more related to the elimination of deprecated {{each}} usages) I removed the minimum amount of stuff to get the tests green.

[stefanpenner]

cc @rwjblue

[stefanpenner]

is there a legacy addon for this to live in ?

[mixonic]

There is no addon for maintaining legacy controller APIs. You must create one if we plan to extract this functionality and the corresponding tests (or enable the functionality via a private flag).

[cibernox]

I will create an addon this weekend, so put this on hold for now.

Just to clarify, the functionality that we intend to extract is only the class itself (ObjectController and ArrayController), but not things like it's usage within itemControllers or the automatic generation depending on what's returned form the model hook, right?

[stefanpenner]

the functionality that we intend to extract is only the class itself (ObjectController and ArrayController)

yes

but not things like it's usage within itemControllers

i dont think so, but someone else should confirm cc @wycats / @rwjblue

the automatic generation depending on what's returned form the model hook, right?

yup not this.

[cibernox]

Extracted addon for ObjectController and ArrayController: https://github.com/cibernox/ember-proxy-controllers

[mixonic]

@cibernox awesome. I would like the repo to live under the Ember project- will make a place for it this afternoon when I am at a laptop.

[stefanpenner]

@cibernox @mixonic https://github.com/emberjs/ember-proxy-controllers

[cibernox]

I'll push to that repo it this evening

[stefanpenner]

@cibernox emberjs/ember-legacy-controllers#1

I have left some comments for you

[ebryn]

we need to be very careful about this extraction, the plugin is going to be used by many people and this is going to be part of their ember 2.x upgrade experience.

the functionality that we intend to extract is only the class itself (ObjectController and ArrayController), > but not things like it's usage within itemControllers or the automatic generation depending on what's returned form the model hook, right?

all existing behavior must be maintained with the plugin. all of the existing tests should still pass with it installed and it should work & feel exactly like it did before (minus the deprecation warnings)

[stefanpenner]

@ebryn @rwjblue @cibernox it is likely also a good idea if we can run the extracted tests as part of embers own test suite, or we wont easily tell we broke something.

I realize this is all lots of work, but i suspect it will be less work in the long run.

[ebryn]

@stefanpenner i recall talking to @wycats about how we can surface failures on canary from community projects using ember-try. i think even @rwjblue told me some work has started on that?

[cibernox]

all existing behavior must be maintained with the plugin. all of the existing tests should still pass with it installed and it should work & feel exactly like it did before (minus the deprecation warnings)

I kind of disagree with this.

I agree that the extracted classes should work exactly as they did before, but some behaviour like the automatic generation of Object/ArrayControllers based on the returned values of the model hook has been deprecated and considered harmful for a long time. I think that is acceptable and even a good thing to force users to explicitly extend generate and extend from those controllers. If we keep that functionality and also remove deprecations we are helping to perpetuate this ad infinitum.

[stefanpenner]

Automatic generation yes cannot continue to work, I suspect @ebryn did not take that into account. Users will need to be informed how to migrate though, as auto generated won't continue to work.

[stefanpenner]

@cibernox i think you have addressed all the concerns in the other PR. I believe we can move forward.

As mentioned in the other PR i'll be available for you tomorrow/thursday to unblock and move to merg.

[cibernox]

I'm revisiting this. What are the blockers on the PR and the array counterpart?