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

RFC: Getter CP readOnly by default #12

Closed
wants to merge 15 commits into from

Conversation

stefanpenner
Copy link
Member

No description provided.

@stefanpenner stefanpenner changed the title initial draft RFC: Getter CP readOnly by default Oct 31, 2014

Overridable CP's are unexpected, hard to learn, and wanted in the minority of
cases. When it happens unexpeditly it is nearly always unwanted and the resulting
choas is hard to debug.
Copy link
Member

Choose a reason for hiding this comment

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

s/choas/chaos/

@stefanpenner
Copy link
Member Author

@tomdale r?

@stefanpenner stefanpenner self-assigned this Nov 14, 2014
# Motivation

Overridable CP's are unexpected, hard to learn, and wanted in the minority of
cases. When it happens unexpeditly it is nearly always unwanted and the resulting
Copy link
Member

Choose a reason for hiding this comment

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

s/unexpeditly/unexpectedly/

@mmun
Copy link
Member

mmun commented Mar 27, 2015

What's the status here? Should we rebase and merge?

@stefanpenner
Copy link
Member Author

pending @wycats pings

@mmun mmun assigned wycats and unassigned stefanpenner Mar 27, 2015
@cibernox
Copy link
Contributor

Now that the new CP syntax is enabled in canary this should not take much time. I can take care of this since I already know that part of the codebase.

@rwjblue
Copy link
Member

rwjblue commented Mar 29, 2015

@cibernox - I believe that there may be some open questions (just from reading the comments above), but it should be ready for initial implementation (and that could help answer some of those questions...).

@tomdale
Copy link
Member

tomdale commented Jun 7, 2015

After discussing at the core team, we realized we get this for free when we migrate to the ES5 getter form.

@tomdale tomdale closed this Jun 7, 2015
@cibernox
Copy link
Contributor

cibernox commented Jun 7, 2015

@tomdale I do have a local branch where I implemented this and IIRC it required very little changes. The concerns of the core team are related with this being deprecated with little notice?

@stefanpenner
Copy link
Member Author

example of the future:

export class extends Ember.Object {

  @computed
  otherName() {

  }

  @computed
  get fullName() {

  }
};
  • otherName overrides (current behavior)
  • fullName doesn't, as isn't how es5 descriptors with missing setters work.

The benefit of this, is a totally clean upgrade process.

A future RFC will explore and propose the formal adoption of decorators in ember.

@stefanpenner stefanpenner deleted the read-only-cp branch June 7, 2015 18:35
@cibernox
Copy link
Contributor

cibernox commented Jun 7, 2015

I see it and I like it.

@stefanpenner
Copy link
Member Author

I see it and I like it.

👍

@stefanpenner
Copy link
Member Author

@cibernox someone, (maybe you or me) should write up an RFC for the future integration of decorators.

https://github.com/rwjblue/ember-computed-decorators is a fun example, but since babel fixed some things some serious issues (chaining is broken) now work. We should likely fix that soon, so we can continue to test in userland and the ES2016 spec matures.

kategengler pushed a commit that referenced this pull request Jan 19, 2019
Add option for JSON output to help command
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.

8 participants