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

[CS2] Update docs for classes, breaking changes #4438

Merged
merged 4 commits into from
Feb 9, 2017

Conversation

Copy link
Collaborator

@lydell lydell left a comment

Choose a reason for hiding this comment

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

Great!

@connec
Copy link
Collaborator

connec commented Feb 8, 2017

@GeoffreyBooth for the classes section, I'd be inclined to leave in the following in the paragraph about :: (in particular, the extends operator still works 😄 ):

If structuring your prototypes classically isn’t your cup of tea, CoffeeScript provides a couple of lower-level conveniences. The extends operator helps with proper prototype setup, and can be used to create an inheritance chain between any pair of constructor functions; and :: gives you quick access to an object’s prototype.

I'd also prefer it if the last paragraph was kept (the mention of static properties in particular is probably value):

Finally, class definitions are blocks of executable code, which make for interesting metaprogramming possibilities. Because in the context of a class definition, this is the class object itself (the constructor function), you can assign static properties by using @property: value, and call functions defined in parent classes: @attr 'title', type: 'text'

It could probably use a simple example, though it didn't have one in the 1.x docs. (I'd be happy to come up with one in a separate PR.)


Also, total sidebar, but we could be smarter with => yield this by compiling to the old function () { ... }.call if it contains a yield, and throwing an error if it contains a yield with an arguments or super.


I've also just realised that the example for Class methods can’t be used with new (uncommon) could be a bit neater with a static method:

class Namespace
  @Klass = ->
new Namespace.Klass  # throws a TypeError at runtime

Also, I wonder if we should put a note somewhere obvious that people should submit issues with their use-cases if they've found awkward changes to work around. I'm sure that there are a good few cases (e.g. => yield this, super in non-methods) that we could smooth over with some motivation.

@GeoffreyBooth
Copy link
Collaborator Author

@connec I want to get away from talking so much about prototypes, since that's a bit confusing re the old implemention vs the new. In my mind the extends operator doesn't make sense outside the context of classes; if there's another use for it, perhaps another section is needed, to discuss prototypes and our sugar for working with them. Maybe that's where the deleted paragraphs should go.

I also don't want to encourage people to use executable class bodies; I feel like that should be almost an undocumented feature, so that there's no obligation for us to keep support for it if it becomes unmanageable.

@connec
Copy link
Collaborator

connec commented Feb 8, 2017

Perhaps we should split the sections into "Classes", and another (or subsection) on "Prototypal inheritance" including :: and extends.

Regarding ECBs, quite a few CS projects I've seen leverage them to some degree or another (either for class-private variables, static properties, or metaprogramming such as mixins). I don't agree that we shouldn't feel an obligation to keep support for them for as long as possible, as they're one of CS' distinguishing features. Certainly, for the alpha, they will be present and will function as before, so they should be documented as before imo.

If something comes up that mandates their removal they should be given their own "breaking changes" entry, and the documentation entry removed.

@GeoffreyBooth
Copy link
Collaborator Author

GeoffreyBooth commented Feb 9, 2017

Okay, we can keep a mention of executable class bodies, but this needs to be updated:

Finally, class definitions are blocks of executable code, which make for interesting metaprogramming possibilities. Because in the context of a class definition, this is the class object itself (the constructor function), you can assign static properties by using @property: value, and call functions defined in parent classes: @attr 'title', type: 'text'

Taking the last part first, the ES idiomatic way to call parent class functions is super.attr; since this is standard ES, I’m not sure we need to mention it. I already split mention of static methods into its own section, but I’m fine with a mention of static properties if you think they’re worth calling out. As far as “(the constructor function)”, that’s no longer correct, is it?

I tried to update it, but I don’t understand this as well as I should. Please let me know what I should change. I implemented all your other notes, though I’m leaving out the invitation for people to suggest more work for us 😄 I think executable class bodies and extends for prototypes both need examples.

…r classes and working with prototypes; make breaking changes examples editable whenever possible
@connec
Copy link
Collaborator

connec commented Feb 9, 2017

OK, I'll have a look and update that soon.

Sidebar on "(the constructor function)" - honestly I'm not sure. It still might be, with the additional caveat that you can't call it without new, but either way it gets pretty subtle so I'll try and phrase that out.

@GeoffreyBooth
Copy link
Collaborator Author

@connec I already rewrote that paragraph to make it more vague and hopefully not incorrect. Any other notes?

@connec
Copy link
Collaborator

connec commented Feb 9, 2017

Cool, that LGTM 👍

@GeoffreyBooth GeoffreyBooth merged commit d1d2c16 into jashkenas:2 Feb 9, 2017
@GeoffreyBooth GeoffreyBooth changed the title [WIP][CS2] Update docs for classes, breaking changes [CS2] Update docs for classes, breaking changes Feb 9, 2017
@GeoffreyBooth GeoffreyBooth modified the milestone: 2.0.0 Feb 9, 2017
@GeoffreyBooth GeoffreyBooth deleted the classes-docs branch February 9, 2017 21:29
This pull request was closed.
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.

3 participants