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

Deprecate usage of `targetObject`. #14168

Closed
rwjblue opened this Issue Aug 31, 2016 · 10 comments

Comments

Projects
None yet
5 participants
@rwjblue
Copy link
Member

rwjblue commented Aug 31, 2016

We currently support having a targetObject property on components, but as of 2.9 we no longer use this property on components internally.

We should deprecate having a targetObject here.

@rwjblue rwjblue added the Deprecation label Aug 31, 2016

@Cryrivers

This comment has been minimized.

Copy link

Cryrivers commented Aug 31, 2016

Thanks for the clarification, although I wasn't able to explain clearly on Slack. :)

Basically, we used targetObject in a component to get the controller that it belongs to. (Say you need to call a function in the controller, from your components. I wouldn't suggest to do so since Ember 2.3 1.13 introduced closure actions.)

However, as of 2.9, targetObject no longer exists. I tried with this._targetObject but it has a different behavior compared with the original targetObject. In 2.9, _targetObject in a component points to its parent component, if any, instead of the controller it belongs to.

So I believe this deprecation is needed and hopefully can help you if you are running into targetObject problem in 2.9

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Sep 4, 2016

This issue represents deprecating having a targetObject specified (either via template invocation or in your component definition). The targetObject property on Ember.Component was intended as a "write only API" (as in you are supposed to define your own if you'd like, but not try to read from the default one). We certainly never intended for this.get('targetObject') to be used.

Basically we used targetObject in a component to get the controller that it belongs to.

It sounds to me like you were using this.get('targetObject'), is that right?

I wouldn't suggest to do so since Ember 2.3 introduced closure actions.

FWIW, Ember 1.13 introduced closure actions.

@Cryrivers - Can you give me an example JSBin or ember-twiddle to show the behavior you are discussing so we can better understand if we need to deprecate reading from targetObject as well as specifying it?

@Cryrivers

This comment has been minimized.

Copy link

Cryrivers commented Sep 5, 2016

@rwjblue https://ember-twiddle.com/e2f017a62fb16c2163d267d49e2d0ed9?openFiles=components.component-b.js%2C

You can open the console and switch between Ember 2.7.0 and canary.

In 2.7.0, targetObject points to ApplicationController whereas it points to its parent component in canary.

P.S: We used targetObject before closure action was introduced. (started our project with Ember 1.12) So we definitely need to remove all targetObject usages.

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Sep 5, 2016

@Cryrivers - Thanks for working through this with me. You have definitely identified a bug (action bubbling from within a component's block does not work correctly on canary), but I'm still not 100% sure if we should add a targetObject property back that issues a deprecation.

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Sep 5, 2016

Created #14216 to track the block scoping issue in canary/2.9.

@jcope2013

This comment has been minimized.

Copy link
Contributor

jcope2013 commented Sep 9, 2016

also using targetObject as well in a similar use case, is the conclusion that targetObject will be no longer available in >= 2.9?

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Sep 9, 2016

targetObject is not available to read from in 2.9 (AFAIK that style of use was not something that we intended to support generally speaking), but if your components have a targetObject it will be respected.

We could add it back for reading with a deprecation, but I would rather avoid that if possible. Hopefully others chime in that hit the issue so we can gauge impact...

@Dhaulagiri

This comment has been minimized.

Copy link
Contributor

Dhaulagiri commented Sep 9, 2016

I found this broke for us in 2.8 but using _targetObject instead seemed to give synonymous behavior

@fooey

This comment has been minimized.

Copy link

fooey commented Sep 12, 2016

We ran into this too, but it was fairly easy to fix once we figured out what was wrong.

@rwjblue

This comment has been minimized.

Copy link
Member Author

rwjblue commented Sep 13, 2016

We discussed this during the last core team meeting, and decided that based on community usage of reading the targetObject property we will bring it back with a deprecation (listed as until: 2.12.0) for both setting and retrieving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.