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

Incoming private API breakage #159

Closed
ef4 opened this issue Dec 12, 2017 · 7 comments
Closed

Incoming private API breakage #159

ef4 opened this issue Dec 12, 2017 · 7 comments

Comments

@ef4
Copy link

ef4 commented Dec 12, 2017

Accessing anything on a ComputedProperty instance is private API and is likely to break in the future (possibly the very soon future). This addon is accessing isDescriptor:

https://github.com/simplabs/ember-test-selectors/blob/master/addon/utils/bind-data-test-attributes.js#L30

The tl;dr is that everybody wants to make Ember.get optional and just use real ES5 getters for computed properties. As soon as we do that, component.attributeBindings will stop giving you a ComputedProperty instance with an isDescriptor, and would always return the value directly, even when it is implemented as computed property.

@Turbo87
Copy link
Collaborator

Turbo87 commented Dec 12, 2017

@ef4 any idea how we can figure out if attributeBindings is writable within that code?

@ef4
Copy link
Author

ef4 commented Dec 14, 2017

It seems that this code just warns and gives up when it can't write to attrributeBindings, so perhaps you can just try to set and catch the error if it's a computed property without a setter, so as to convert the error into a pleasant warning.

@Turbo87
Copy link
Collaborator

Turbo87 commented Dec 14, 2017

guess I'll wait until this code actually breaks in canary and then we'll see how to fix/work around it

@ef4
Copy link
Author

ef4 commented Dec 14, 2017

Yeah, that's reasonable. The relevant RFC is emberjs/rfcs#281

@Gaurav0
Copy link

Gaurav0 commented Feb 22, 2018

This issue is causing a test to fail in #172

@Turbo87
Copy link
Collaborator

Turbo87 commented Feb 22, 2018

indeed, I'm just surprised that it works without issues in one of our apps that is running 3.0 🤔

@ef4
Copy link
Author

ef4 commented Feb 22, 2018

It will only break if you happen to have attributesBindings as a computed property. It's not a common thing to do.

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

3 participants