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

Use new getter/setter for computed if available #2961

Merged
merged 1 commit into from
Apr 1, 2015

Conversation

wecc
Copy link
Contributor

@wecc wecc commented Mar 31, 2015

This fixes the ~3400 deprecation warnings Using the same function as getter and setter is deprecated for beta and canary builds.

@rwjblue
Copy link
Member

rwjblue commented Mar 31, 2015

👍

if (arguments.length > 1) {
return computedConfig.set.call(this, key, value);
}
return computedConfig.get.call(this, key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this logic into a shared util helper?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered using https://github.com/rwjblue/ember-new-computed/blob/master/addon/index.js from @rwjblue but didn't know if it was worth it since we only have settable CPs in these two places in ED. If we use a helper like the one linked we should probably make sure that we use it everywhere within ED.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, I'm happy to make it a shared helper too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are already putting supportsComputedGetterSetter into a shared util, but it's only used as part of this function, we should probably move this into the util as well.

@wecc wecc force-pushed the beta-and-canary-deprecations branch from 3da44fc to c5c0861 Compare March 31, 2015 13:33
@wecc wecc force-pushed the beta-and-canary-deprecations branch from c5c0861 to c53815f Compare April 1, 2015 06:13
@wecc
Copy link
Contributor Author

wecc commented Apr 1, 2015

  1. Does the name computedPolyfill make sense?
  2. Do we want all other CPs (getters only) to use this too for consistency?

@igorT
Copy link
Member

igorT commented Apr 1, 2015

I think the getter only cps are gonna stay only functions in 2.0 as well, so I don't think there is a need. computedPolyfill does make sense

wecc added a commit that referenced this pull request Apr 1, 2015
Use new getter/setter for computed if available
@wecc wecc merged commit 2c575aa into emberjs:master Apr 1, 2015
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

Successfully merging this pull request may close these issues.

None yet

4 participants