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

Usage of targetObject is deprecated #13

Open
malatin3 opened this issue Jul 25, 2018 · 3 comments
Open

Usage of targetObject is deprecated #13

malatin3 opened this issue Jul 25, 2018 · 3 comments

Comments

@malatin3
Copy link
Contributor

Was trying to fix this deprecation:

DEPRECATION: <client-web@component:dynamic-link::ember3232> Usage of targetObject is deprecated. Please use target instead. [deprecation id: ember-runtime.using-targetObject]

However, it seems that target is already being used to bind the html target attribute. So I'm not sure how to approach this one.

Personally, I think that using 'target' for the html property / binding makes more sense than using some alias. But given that Ember is now using target to mean something else.. I'm not sure what else can be done to make them coexist nicely.

Am I overlooking something? Did they overlook this case when they changed the API?

Any thoughts?

Thanks

@malatin3
Copy link
Contributor Author

@asross Did you ever get a chance to think about which direction to take this? I'm willing to work on a PR

@asross
Copy link
Owner

asross commented Aug 24, 2018

Hey, thanks for the issue and your willingness to work on a PR. This is a tricky one, isn't it?

The best thing would be if we could somehow use target to get the component's target object but still allow users to pass target='_blank' when creating the component. That would keep the library totally backwards-compatible. It would be worth looking into whether that's possible.

If it isn't, then I think we should just bite the bullet and introduce another property like linkTarget which we include in attributeBindings as 'linkTarget:target' (but support both target and linkTarget in the params). So basically:

attributeBindings: ['linkTarget:target'],

linkTarget: Ember.computed('params.linkTarget', 'params.target', function() {
  return this.get('params.linkTarget') || this.get('params.target')
}),

performAction: function() {
  var target = this.get('target') || this.get('targetObject') || this.get('_targetObject');
   // ...
}

We can then remove support for the bare target property and bump the major version. Sound like a reasonable plan?

@asross
Copy link
Owner

asross commented Aug 24, 2018

Actually, if we wanted to be really sneaky, we could check if this.get('target') is a string or not, like this:

linkTarget: Ember.computed('params.linkTarget', 'params.target', 'target', function() {
  if (typeof this.get('target') === 'string') {
     return this.get('target');
  } else {
     return this.get('params.linkTarget') || this.get('params.target');
   }
}),

performAction: function() {
  var target = this.get('target');
  if (typeof target === 'string') {
     target = this.get('targetObject') || this.get('_targetObject');
  }
   // ...
}

Then we avoid introducing a backwards incompatibility :)

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

2 participants