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

Remove computed-property.override deprecation #282

Open
tniezurawski opened this issue Aug 23, 2019 · 5 comments
Open

Remove computed-property.override deprecation #282

tniezurawski opened this issue Aug 23, 2019 · 5 comments

Comments

@tniezurawski
Copy link

I see this deprecation since Ember 3.9.0:

DEPRECATION: The <my-app@component:ember-modal-dialog/-tether-dialog::ember4138>#targetAttachmentClass computed property was just overriden. This removes the computed property and replaces it with a plain value, and has been deprecated. If you want this behavior, consider defining a setter which does it manually. [deprecation id: computed-property.override] See https://emberjs.com/deprecations/v3.x#toc_computed-property-override for more details.

I tried to identify where targetAttachmentClass was overridden but I can't find it 🤷‍♂
Any clue?

@jameshahn2
Copy link

This has to do with a function somewhere in the code. If someone can walk me through it I'd be glad to write the PR. If you search if (this._ in this repo, it's what most closely resembles the message in the deprecation. I'm wondering if it is a reason my image popups stopped working. Let's get this fixed y'all!

@lukemelia
Copy link
Contributor

Hi @jameshahn2, the reason for this error is that targetAttachmentClass is a computed property. If a value is passed in for the property, Ember's historical behavior is that the passed value would replace the computed function. This is now deprecated, hence the warning. So we need a new way to provide a default value for targetAttachmentClass. I think that a way to do it would be to make the init method of the component(s) set targetAttachmentClass if and only if this.targetAttachmentClass is undefined.

@lukemelia
Copy link
Contributor

I'm happy to review a PR to fix this but don't have time to work on one myself currently.

@jameshahn2
Copy link

jameshahn2 commented Sep 17, 2019

Awesome @lukemelia. Just started the conversation on Discuss. Hope we can get to the bottom of it :)

@evanb2
Copy link

evanb2 commented Dec 5, 2019

@jameshahn2 Any progress on this?

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

4 participants