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

[docs] Adds documentation to CoreObject for native classes #17174

Merged
merged 1 commit into from
Jan 4, 2019

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Nov 1, 2018

Adds documentation for native classes, prepping for making the officially public API.

@pzuraq pzuraq force-pushed the add-core-object-docs branch from 0fd3bf7 to 21bb4e6 Compare November 1, 2018 21:35
* 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.
Copy link
Contributor

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);
  }
}

Copy link
Contributor Author

@pzuraq pzuraq Nov 1, 2018

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.

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 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.

@rwjblue
Copy link
Member

rwjblue commented Dec 7, 2018

@pzuraq - I'd like to get this back on track, think you might have time to revive it soon?

@pzuraq pzuraq force-pushed the add-core-object-docs branch from 3390ea4 to 9bef728 Compare December 10, 2018 18:52
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
Copy link
Contributor

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?

Copy link
Contributor Author

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

@pzuraq pzuraq force-pushed the add-core-object-docs branch from 9bef728 to 8924c62 Compare December 10, 2018 21:42
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.

4 participants