-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[docs] Adds documentation to CoreObject for native classes #17174
Conversation
0fd3bf7
to
21bb4e6
Compare
* Avoid using the `constructor` in favor of `init`. Services are not available | ||
during the `constructor` phase, and the nuances of the two can be confusing. | ||
* Using native classes, and switching back to the old Ember Object model is | ||
fully supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supported/encouraged/discouraged?
export default class XFoo extends Ember.Component({
// properties that I deliberately want to share between instances
tagName: 'ul',
fullName: computed('firstName', 'lastName', function() {
return `${this.firstName} ${this.lastName}`;
}),
// immutable "defaults" for member data, deliberately shared across all instances
firstName: '',
lastName: ''
}) {
// standard ES6 class stuff
logName() {
console.log(this.fullName);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discouraged, since it will be very difficult to codemod half-way states like this. It's much easier for us to go from a clean-slate to full ES classes in the codemod. The purpose of the ability to do this is so addon authors can fully adopt early on, and so we have a path forward for mixins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be addressed in here, but in a linting rule. We shouldn't even suggest this is a possibility in our docs, ideally. We could cover it in guides though.
41b3e1c
to
3390ea4
Compare
@pzuraq - I'd like to get this back on track, think you might have time to revive it soon? |
3390ea4
to
9bef728
Compare
Some notes about `class` usage: | ||
|
||
* `new` syntax is not currently supported with classes that extend from | ||
`EmberObject`. You must continue to use the `create` method when making new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example above refers to CoreObject
, and the above sentence would seem to suggest that I can use new Foo()
as long as I extend from CoreObject
instead of EmberObject
. Is this accurate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, that is not accurate. Will update to refer to CoreObject
instead
9bef728
to
8924c62
Compare
Adds documentation for native classes, prepping for making the officially public API.