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 Discussion: Output: Classes #4922

Closed
coffeescriptbot opened this issue Feb 19, 2018 · 90 comments
Closed

CS2 Discussion: Output: Classes #4922

coffeescriptbot opened this issue Feb 19, 2018 · 90 comments

Comments

@coffeescriptbot
Copy link
Collaborator

From @rattrayalex on 2016-08-06 21:33

This is a core feature that we should rally around submitting to jashhkenas/coffeescript in the near term.

Does anyone know of current leading efforts to accomplish this in the community? Perhaps we can lend a hand there.

@coffeescriptbot
Copy link
Collaborator Author

From @kirly-af on 2016-08-07 13:36

This is discussed in jashkenas/coffeescript#4233 (and probably many others I'm not aware of).

Also, I just discovered this Redux PR thanks to @GeoffreyBooth which looks REALLY promising.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-08-10 07:04

So it appears that someone has actually already implemented CoffeeScript class to ES2015 class—in decaffeinate. Check it out!

@coffeescriptbot
Copy link
Collaborator Author

From @DomVinyard on 2016-08-15 15:16

I think there's probably a larger conversation to be had about decaffeinate specifically, how and where it intersects with this project, what can be borrowed, what can be adapted, etc.

@coffeescriptbot
Copy link
Collaborator Author

From @alangpierce on 2016-08-21 02:06

Hi, I'm a decaffeinate contributor. While decaffeinate does have basic support for classes, full support is one of the main missing features right now. I'm hoping to improve it over the next few weeks, but it's definitely a challenge, since my understanding is CoffeeScript allows arbitrary code in class bodies, while ES2015 classes only allow methods. Decaffeinate also compiles CoffeeScript class fields to the public class fields proposed feature (currently stage 2), but even that has pretty significant semantic differences that have caused bugs for me (CS class fields are on the prototype, JS class fields are assigned to the instance once the constructor finishes). Anything other than methods or fields causes decaffeinate to crash.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-08-21 02:28

Thanks @alangpierce. Yeah, after looking at it more I think the better solution is to redefine the class syntax in CoffeeScript to be something that translates directly into ES2015 classes, dropping support for ES2015-unsupported features like code in class bodies. This would be a breaking change in CoffeeScript, probably one that’s opt-in via a flag, but necessary if CoffeeScript is going to keep up with new features introduced by ECMAScript.

While the immediate focus is on adding support to CoffeeScript for missing vital ES2015 features (modules, classes), the longer-term goal especially of coffeescript6 is to update CoffeeScript’s output to use more of the latest standards. This essentially means we would be doing the work that decaffeinate is doing, but within the main CoffeeScript compiler. I skimmed through the decaffeinate repo, hoping to find some commits that highlight what differences it had from the main CoffeeScript compiler, but I couldn’t make sense of it. Do you mind giving whatever overview you can of how decaffeinate works, and where I should look for code that might be useful to repurpose in the main CoffeeScript compiler? For example, where is the code that generates an ES2015 class?

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on 2016-08-21 06:38

I would love to see some way of extending the current class functionality to generate the ES2015 code, and then decorate with everything else. I guess I don't really understand what actually breaks and where yet though. I am assuming that ES classes have extra restrictions, ie that they aren't just objects with prototypes set in a specific pattern?

From the Babel overview:

ES2015 classes are a simple sugar over the prototype-based OO pattern. Having a single convenient declarative form makes class patterns easier to use, and encourages interoperability. Classes support prototype-based inheritance, super calls, instance and static methods and constructors.

So I would think we could construct functions only, then decorate with everything else?

@coffeescriptbot
Copy link
Collaborator Author

From @alangpierce on 2016-08-21 07:00

@GeoffreyBooth Yep, dropping support for code in class bodies makes a lot of sense for your case.

Sure, I can give a quick overview of how decaffeinate works:

Unfortunately, I think the short answer is that decaffeinate's approach is pretty different from the CoffeeScript compiler, so you may not be able to use much code/logic. From my reading of the CoffeeScript compiler, it parses the input to an AST, then calls the recursive compileNode method (and related methods) to generate a big JavaScript string that becomes the output file. Decaffeinate works by parsing the input into an AST (and a token sequence), then using that AST to inform targeted transformations on the code (inserts, removes, and replacements) using the magic-string library. For example, to compile a b, it uses the AST to see that it's a function call, looks at the tokens to see that there's no open-paren or close-paren, and replaces the space with a ( and inserts a ) after the b (code here). This approach makes sense given decaffeinate's goal: it doesn't want to rewrite all of your code, it wants to tweak it (sometimes significantly) to be valid JavaScript, maintain formatting and comments as much as possible. But really, this approach ends up being a big pain sometimes, and transforming the code to a JavaScript AST would probably be more robust in a lot of ways (but would lose formatting).

A little more information on how decaffeinate is structured: it consists of five "stages" (one CoffeeScript -> CoffeeScript, one CoffeeScript -> JavaScript, and three JavaScript -> JavaScript), the most important of which is the CoffeeScript -> JavaScript stage, which is called MainStage. For that stage, every type of AST node corresponds to a Patcher class. Each Patcher is a class that knows what kind of modifications to make to transform that type of CoffeeScript node to JavaScript.

Here's a repl link that has some examples of what happens in different stages. You can change the "run to stage" selection to see the intermediate result after each stage (or intermediate lexer/parser results).

I actually haven't worked with the class generation code, but was able to mostly figure out how things work. If you look at the decaffeinate-parser output for your example, you can see that the main AST hierarchy is Class > Block > ClassProtoAssignOp > Function. From the node name to patcher class mapping, (or by just looking at the names), you can see that they map to the classes ClassPatcher, BlockPatcher, ClassAssignOpPatcher, and FunctionPatcher. The methods patchAsStatement and patchAsExpression are generally the most important ones. As one example, ClassAssignOpPatcher extends ObjectBodyMemberPatcher, which calls this.expression.patch with the method flag set appropriately. On the FunctionPatcher side, it knows that it should skip the function keyword if the function is being patched as a method.

Also, here's a writeup with some more details:
https://github.com/decaffeinate/decaffeinate/blob/master/docs/contribution-guide.md

Hope that helps!

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-05 21:02

I don’t think we can simply redefine the class keyword, even behind a flag. That would be too drastic a breaking change for current projects to handle, including the CoffeeScript compiler codebase itself (which uses class extensively). I think we need to provide a clear and easy upgrade path, especially if we’re adding ESNext class support to the original compiler.

How about this approach: we create a new CoffeeScript keyword, esclass, that has different syntax than the current class. It only allows things that can be output into an ESNext class, and it compiles into ES output (so class is in the generated JavaScript, not function).

With esclass, a CoffeeScript project could contain both new compile-to-ESNext-class classes and legacy old compile-to-function classes. People could convert a project gradually, or pick and choose which type of classes they want to use (for example, esclass could be used when extending a class imported from another library, with class used for all classes particular to your project). Having two types of classes would also let us keep the CoffeeScript compiler codebase intact without major changes.

Introducing a new keyword that wasn’t previously reserved is a breaking change, so we would need to introduce a flag. I think since esclass is so obscure a string, it’s unlikely that there are many (if any) projects out there using it already as a symbol; so maybe we could get away with shipping it reserved by default, so that the vast majority of projects need not do anything to take advantage. For the 0.001% of projects using esclass already as a variable or function name (and who don’t want to refactor), they would need to set the flag to force legacy support. The flag could work something like this:

  • Default, no flag: The class keyword would retain its current syntax, and esclass would become a new reserved word that introduces a new syntax that compiles into ESNext classes.
  • --legacy-classes: The current behavior. class is unchanged, and esclass is not a reserved word.

We could even add support for making class an alias for esclass and csclass an alias for the current class, if we want to let people use the class keyword for the new syntax; but I’m concerned that it’s probably a bad idea to not be able to look at code and know how it will get compiled, if you don’t know what flag will be set in the compiler.

Conversely, it might be a good idea to introduce csclass as a new reserved word that always aliases to the current class syntax, so that people could use esclass and csclass exclusively, leaving no ambiguity about what their intentions are.

What do you all think? @lydell, @rattrayalex, @JimPanic, others . . .

@coffeescriptbot
Copy link
Collaborator Author

From @JimPanic on 2016-09-06 05:54

I like the idea of csclass & esclass although we might want to find better names. I also am not sure yet what the differences in semantics are. It seems there is a bit more to it than just syntactic sugar as the ES201x class does not allow data properties, has different grammatical rules than object literals and cannot be called without new (if I read that correctly).

Apart from that, I think the only solution to introduce this to users is a) with conversion tools and b) through a process of deprecation warnings, deprecation and removal over several releases. This is something we have to plan, no matter what keywords we use in the end.

The only thing that could be dangerous is to keep the class keyword to stay semantically different from the one in ES201x. Its meaning has to change eventually.

@coffeescriptbot
Copy link
Collaborator Author

From @lydell on 2016-09-06 05:55

Here's my vote:

Add esclass for ES2015+ classes. Make no other changes whatsoever.

Release it by bumping the middle version number, keeping CoffeeScript's current versioning scheme of bumping the middle number when there are new features an slightly-but-not-very breaking changes. (As @GeoffreyBooth it is very unlikely that this will cause any trouble for anyone.)

No csclass. Do we want csin, csof, csfor etc. too?

CoffeeScript 2.0.0: Remove old class, rename esclass into class (and do similar things for other ES2015+ features) and switch to semver.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-06 05:58

I guess we probably don’t need a flag if we’re simply adding esclass (or whatever people decide to name it) and leaving class alone. It would be a breaking change to anyone who was already using esclass as a symbol, but maybe rather than giving them a flag to opt out we should just recommend they simply don’t upgrade (or that they refactor). It’s a very minor refactor to find and replace all instances of esclass with something else, we wouldn’t need to provide migration tools for that.

My god, did we just come up with a way to implement classes without a flag? 😛

@coffeescriptbot
Copy link
Collaborator Author

From @JimPanic on 2016-09-06 06:23

I think we did, yes. 🤔 I'm also very much in favour of implementing semver.

@coffeescriptbot
Copy link
Collaborator Author

From @greghuc on 2016-09-06 08:20

I like the idea of a separate esclass as a 'least breakage' way forward. However, I'm less comfortable with how the eventual replacement of Coffeescript's class with esclass could pan out.

At present class let's me do some nice things: instance functions, static functions, private static functions and 'constants'. I've written a short example class below. If esclass doesn't eventually support these class capabilities, then this is a step backwards for Coffeescript. Though you could argue that if ES2015+ classes can't handle this, then that's a step backwards too!

Example:

  class ClassExample

    CONSTANT_EXAMPLE = 'I am a constant example'

    constructor: -> console.log 'I am a constructor'

    instanceFunction: ->
      console.log '[start] I am an instance function'
      console.log CONSTANT_EXAMPLE
      privateStaticFunction()
      @constructor.staticFunction()
      console.log '[end] I am an instance function'

    @staticFunction: ->
      console.log '[start] I am a static function'
      console.log CONSTANT_EXAMPLE
      privateStaticFunction()
      console.log '[end] I am a static function'

    privateStaticFunction = -> console.log 'I am a private static function'

  # Example calls
  #ClassExample.staticFunction()
  (new ClassExample).instanceFunction()

@coffeescriptbot
Copy link
Collaborator Author

From @lydell on 2016-09-06 11:21

@greghuc I don’t really see the step backwards?

# ClassExample.coffee

CONSTANT_EXAMPLE = 'I am a constant example'

privateStaticFunction = -> console.log 'I am a private static function'

export class ClassExample

  constructor: -> console.log 'I am a constructor'

  instanceFunction: ->
    console.log '[start] I am an instance function'
    console.log CONSTANT_EXAMPLE
    privateStaticFunction()
    @constructor.staticFunction()
    console.log '[end] I am an instance function'

  @staticFunction: ->
    console.log '[start] I am a static function'
    console.log CONSTANT_EXAMPLE
    privateStaticFunction()
    console.log '[end] I am a static function'

# whatever.coffee
import {ClassExample} from 'ClassExample.coffee'
ClassExample.staticFunction()
(new ClassExample).instanceFunction()

@coffeescriptbot
Copy link
Collaborator Author

From @greghuc on 2016-09-06 11:48

@lydell I'm still getting up to speed with ES6 features, so please take my comments with a pinch of salt..

Your example code isn't that bad. It is essentially forcing a coffeescript user to have a closure wrapping both the class declaration and the private 'constant' and static function, to ensure private means private. But from my quick scan of ES6 modules, I suppose there's an implicit closure when importing a module from a file.

I could live with this, although I don't think your example code is as clean or explanatory as the current coffeescript code. In the current code, the class acts as an encapsulating namespace for all private and public functions.

For the esclass to class transition, there'll be a need to document how 'private' variables and functions are implemented in the new world.

@coffeescriptbot
Copy link
Collaborator Author

From @DomVinyard on 2016-09-06 15:02

How about, instead of esclass we go for Class, uppercase C?

@coffeescriptbot
Copy link
Collaborator Author

From @lydell on 2016-09-06 15:08

@DomVinyard In my opinion that would be very confusing. I also think that that will break more people's code – even CoffeeScript's source code has class Class in it!

@coffeescriptbot
Copy link
Collaborator Author

From @DomVinyard on 2016-09-06 15:24

@lydell I concede that point and do accept it would break more projects than esclass would (although still not a great deal in the grand scheme of things). It's just that Coffeescript seems too institutionally elegant and natural for keyword prefixes. Your illustration earlier of "why not esfor or esin?" describes the danger better than I could.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-08 03:46

@DomVinyard I have to agree with @lydell on this one. I’m not thrilled by the name esclass, but I think Class could be way too confusing. It’s easy to rename later if someone proposes a better name.

So I think the next step is to define what the syntax of esclass would be. I don’t really follow the example above. The relevant pages on MDN are here and here and here. Some interesting things:

  • Class declarations are not hoisted (unlike function declarations).
  • When used in a constructor, the super keyword appears alone and must be used before the this keyword can be used. This keyword can also be used to call functions on a parent object.

Here’s an extensive example.

It seems clear from the examples that we will need to implement get and set as part of this, and some way to trigger output of the static keyword. super will also behave differently when inside an esclass—it would be called like super.parentClassMethodName rather than just super, except in the constructor.

I think like with modules, our goal should be to simply create a way to output ES classes with all their features as they are, rather than trying to improve upon them the way that the original class improved upon “function classes.” At least at first; we could add sugar later.

Is anyone else interested in helping research/define this, and/or code it?

@coffeescriptbot
Copy link
Collaborator Author

From @rattrayalex on 2016-09-08 04:27

Exciting stuff.

Perhaps once esclass is implemented we'll even find that it can replace class. Either way, makes sense to me to take that tack.

I am curious though – what advantages do CS classes have over ES classes? Why would a developer prefer CS to ES output? (Honestly don't know).

@coffeescriptbot
Copy link
Collaborator Author

From @JimPanic on 2016-09-08 08:29

@rattrayalex ES201x classes cannot have executable code in their bodies. You cannot declare properties at runtime. You can do that with CS classes but I personally think it's bad practice to do so as it's not very explicit and mixes different styles of defining "types".

@coffeescriptbot
Copy link
Collaborator Author

From @JimPanic on 2016-09-08 08:39

Ok let me rephrase that a bit: of course you can declare properties at runtime, but not in the class declaration body like in CS. :)

@coffeescriptbot
Copy link
Collaborator Author

From @greghuc on 2016-09-08 08:54

Re-reading through this 'classes' thread, I think it's worth noting that are at least three distinctive positions to take on Coffeescript and ES6 classes. Two positions are at odds with one another. The positions:

  1. Coffeescript classes are semantically fine as they are. The current Coffeescript compiler implements classes using correct use of Javascript prototypes. This enables instance functions, static functions, private static functions and 'constants' - fantastic! In the meantime, ES6 has formalised one approach to implementing Javascript classes. Quoting from @mrmowgli's comment above and MDN: "JavaScript classes are introduced in ECMAScript 6 are syntactical sugar over JavaScript's existing prototype-based inheritance. The class syntax is not introducing a new object-oriented inheritance model to JavaScript". From this perspective, we should update the CS compiler to output CS classes as ES6 classes, and decorate them afterwards to provide class features that CS currently has and ES6 has not. @lydell's comment above shows how this could be done.
  2. Coffeescript classes are semantically wrong, now that ES6 classes have come along. From this perspective, we should reimplement ES6 classes, but in a Coffeescript syntax. We should remove all CS class features that ES6 currently doesn't support. We need to resolve backwards compatibility: we either define CS class behaviour as ES6 behaviour and accept existing code will break, or we postpone the issue by introducing a new esclass.
  3. Coffeescript classes and ES6 classes are technically incompatible. @GeoffreyBooth mentions that Coffeescript should output to ES6 classes in order to "interoperate with libraries that use ECMAScript classes". I haven't seen a concrete example of this yet - it would be great to have some broken test-cases showing this behaviour. If Coffeescript and ES6 are becoming technical inoperable, then this definitely needs fixing.

My personal position is a combination of 1 and 3: change the Coffeescript compiler to take advantage of ES6 class functionality, and fix any technical incompatibilities. But don't change (or remove!) existing CS class semantics.

I see position 2 as a risky one (Coffeescript classes are semantically wrong, now that ES6 classes have come along). This path leads to the ES6 tail wagging the Coffeescript dog. From now on, Coffeescript is no longer an opinionated "little language that compiles into JavaScript". Instead, we'll be continually playing catchup, discussing the best way to implement the newest EcmaScript feature in Coffeescript syntax. In essence, it will be Ecmascript committees driving the core direction of Coffeescript, not the Coffeescript committers themselves. Already, because ES6 classes don't currently support data properties, we might have to remove Coffeescript class data properties. What happens when Ecmascript adds them back in?

@coffeescriptbot
Copy link
Collaborator Author

From @lydell on 2016-09-08 09:19

@greghuc The biggest problems, in my opinion, are that you currently cannot extend an ES2015+ class (#4233) and that CoffeeScript’s super is both very complicated and very different from super in ES2015+ (#3796 might be informative).

@coffeescriptbot
Copy link
Collaborator Author

From @greghuc on 2016-09-08 12:19

Also, to put a slight dampener on 'ES6 classes are the new awesome', here's a link to a curated list of resources on why ES6 (aka ES2015) classes are NOT awesome.

I'm not aiming to start an argument over the pros and cons of ES6 classes. However, it's worth keeping in mind that ES6 classes are a new and unproven technology, and it's unclear how things will play out with them. This is independent to how well ES6 classes are supported by browsers and nodejs.

This raises the possibility of Coffeescript nextgen locking itself to a feature that doesn't stand the test of time.

It doesn't preclude Coffeescript being able to interoperate with ES6 classes.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-11 18:08

@greghuc I think your preferred approach (1 and 3) is what the esclass proposal does. We leave the current class alone, in all its awesomeness, and introduce a new esclass that matches ECMAScript’s class definition. If ECMAScript adds data properties, we add data properties to esclass. Yes, this means we are following the ECMA standards body. But . . . CoffeeScript is a language that compiles to JavaScript. By definition the ECMA committee tail will wag the CoffeeScript dog.

What your “data properties” example illustrates, however, is that we should think long and hard before we add any sugar over esclass. Since ECMAScript’s class is subject to change, we very well could get ourselves into trouble if we add custom features on top of it. In general now that JavaScript is no longer static, we should refrain from implementing any new custom features that might someday get implemented natively in ECMAScript (especially if they might get implemented in an incompatible way, like class or ...). Presumably ECMAScript won’t break backward compatibility with the current spec for class, so we can at least implement that; but no more than that, until ECMA finalizes those new additions.

I don’t think it’s so bad to follow ECMA. They’re moving way faster than we ever could. It’s not a bad thing to keep up with the JavaScript world.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-09-14 03:14

I’ve been thinking about how Decaf manages to parse most CoffeeScript classes into ES2015 classes. If we’re going to introduce ESNext output via a flag or 2.0.0 breaking-change version bump, then we could take Decaf’s approach and output ES2015 class whenever the CoffeeScript class isn’t doing anything incompatible (like code in class bodies) and the current function otherwise. What would people think of this approach?

Out of curiosity I ran Decaf on nodes.coffee. It throw an error on any statement like [..., last] = body.expressions, but if I commented them out, the only class it couldn’t parse was Op, because of the lines starting with CONVERSIONS = and INVERSIONS = that technically are code in a class body. But I feel like those could probably be refactored as @CONVERSIONS = and just placed inside the constructor. Ditto for the children: lines that Decaf simply loses in its conversion.

It’s another question whether these ES2015 classes are truly functionally equivalent to the CS-generated function versions. To figure that out, we’d have to either refactor all the [..., last] lines throughout the code or fix Decaf to handle such lines; and refactor any other classes with class bodies; and update the test runner to use a version of CoffeeScript that was run through Decaf and then Babel. No small task, but perhaps worth it if we learn that most uses of class could be output as ES2015 as is. Here is Decaf’s output for my modified nodes.coffee (that comments out the troublesome lines); aside from the missing children: properties, is there anything here in its conversion of classes that stands out to anyone as being problematic?

Assuming we wouldn’t use Decaf but would reimplement similar logic ourselves, is this an approach we want to take?

@coffeescriptbot
Copy link
Collaborator Author

From @kirly-af on 2016-09-14 14:14

To be honest, I'm not so sure what are the exact points where CS classes and ESnext classes clash.
All I'm aware of are: super behavior is different and CS classes have CS classes have an executable body.

Another issue is this.

However, this suggestion looks good to me.

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on 2016-10-02 09:32

Still going through everything, but I ran into this reference of Node class support and versions. Handy reference: http://node.green/#class

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2016-10-03 13:34

I got a bit carried away "adding some [tests] to exercise the ordering" and created a WIP PR @ #4330. I don't intend for this to clash with anything anyone else is doing, but hopefully it can highlight some of the potential issues and pitfalls.

Some salient points:

  • CS currently allows a lot of shenanigans with super such as soaked invocations, and super in external functions. This is tricky to deal with, particularly in constructors. If we want to use ES2015 super we probably need to reign those in a little if we want to limit the number of code paths we have to deal with.
  • If we continue supporting executable class bodies, hoisting method definitions and their comments is a bit messy.
  • If we continue supporting executable class bodies, dealing with dynamic method names is tricky since the names could be updated throughout the class body, but the methods must all be hoisted into the class.
  • @ arguments in constructors need special treatment, since super must be called before this can be accessed.

@coffeescriptbot
Copy link
Collaborator Author

From @svicalifornia on 2016-10-23 12:45

Another way that ES6 classes (at least as compiled to ES5 by Babel) differ from CS classes is that in ES6, an extend hook on the superclass will be called when it is subclassed.

Cross-post from #4330:

When using Objection.js Model with CoffeeScript, I had to add a line of boilerplate after each CS subclass definition to call Model's extend hook:

Model = require('objection').Model
class MyModel extends Model
  ...

Model.extend MyModel

Babel compiles ES6 subclasses in such a way that the hook was called in the resulting ES5 code, so the explicit (and redundant) call to extend was not necessary when using Babel.

To achieve backward compatibility with CS code and derive from classes in ES libraries, it would be great if CS 1.x could detect and extend ES6 classes as follows:

  1. Create a CS subclass with executable class bodies, static properties, and instance properties.
  2. Call the extend hook (if there is one) when extending an ES class, as Babel-compiled code does.
  3. Make super work properly with the ES6 superclass.

Going the other direction — compiling CS classes to ES6 classes that can be extended by ES6 code — would be a breaking change and should probably be held for 2.0. (Extending from CS to ES subclasses is the much less common use case, given that most of the ES community doesn't care much for CoffeeScript. At least until CoffeeScript is modernized to full ES compatibility, we are much more likely to want to use ES classes than they are likely to want to use modules originally written in CoffeeScript.)

If CS 1.x generates CS subclasses, and CS 2.0 generates ES classes and subclasses, then I agree with @jashkenas that a separate esclass keyword is unnecessary and likely to cause confusion.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-10-26 06:10

Trying to push along #4330: can anyone write a test that, if it were to pass, would prove that CoffeeScript could extend a React class such that CoffeeScript can interoperate well with React?

As far as I can tell the big interoperability concern with classes is that ES classes cannot currently be extended in CoffeeScript; and the most high-profile case where this is an issue is React. I would love a failing test where, if we were to get it to pass, it would prove that our interoperability problem with React was solved.

Please don’t write the test such that it actually imports React, just create the simplest possible ES class to demonstrate the issue. There could be a test/importing/es_class.js file that exports an ES class (using CommonJS) and inside test/classes.coffee there would be a test that extends the ES class and does something with it, similar to whatever people are trying to do with React (or Objection.js or other) classes.

@coffeescriptbot
Copy link
Collaborator Author

From @mitar on 2016-12-11 20:33

I really love how CoffeeScript has classes and I do not really have much opinion on to what is it compiled internally. But I do love one thing from ES6 classes: that you can define instance properties inside the class body. I love that:

class Foo {
  bar = 3;
}

Is compiled to:

var Foo = function Foo() {
  this.bar = 3;
};

Because otherwise doing an instance-based assignment you can do only inside a constructor, which sometimes makes it much less cleaner to read. I wanted this feature so many times, and on the other hand I have never seen a need for executable class bodies which I could not simply resolve with creating dynamic classes at runtime.

@coffeescriptbot
Copy link
Collaborator Author

From @lydell on 2016-12-12 06:07

But I do love one thing from ES6 classes: that you can define instance properties inside the class body.

That's not ES6, but a proposal for a future specification.

@coffeescriptbot
Copy link
Collaborator Author

From @mitar on 2016-12-12 06:11

That's not ES6, but a proposal for a future specification.

Oh, sorry. It works with Babel, so I just assumed it is part of E2016 already. Do you know which Babel rule it is/more about this proposal/its name?

In any case, I love this feature and I would propose that CoffeeScript has it even it does not become part of JavaScript.

Edit: found it: https://babeljs.io/docs/plugins/transform-class-properties/

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-12-12 06:47

See also: #4354 (comment)

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on 2016-12-16 07:57

Ok, I have finished getting through the existing class tests, and a lot of the current rules are covered well. I will be adding roughly thirty or so tests. One thing was very "interesting" to me:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/species

It's definitely referred to in the core MDN docs, and Symbols are an area we will probably have to support in iterators and classes.

I'm not entirely sure I understand why this exists, if anyone knows I would love to hear a use case?

class C {
  ident() {return 'Class C!';}  
};

class MyArray extends Array {
  // Overwrite species to the parent Array constructor
  static get [Symbol.species]() { return C; }
  ident() {return 'Class MyArray!';}
}

var a = new MyArray(1,2,3);
var mapped = a.map(x => x * x);

console.log(mapped instanceof C); // true
console.log(mapped instanceof MyArray); // false
console.log(mapped instanceof Array);   // false
console.log(MyArray.prototype); // MyArray
console.log(a.ident()); // Class MyArray!
console.log(mapped.ident()); // Class C!
console.log(mapped);  // C { '0': 1, '1': 4, '2': 9 }

@coffeescriptbot
Copy link
Collaborator Author

From @triskweline on 2016-12-16 08:08

I found this article slightly more helpful than MDN:
http://www.keithcirkel.co.uk/metaprogramming-in-es6-symbols/#symbolspecies

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2016-12-16 08:20

@mrmowgli: I will be adding roughly thirty or so tests.

You, sir, are my hero.

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on 2016-12-16 08:24

Also super is an interesting dilemma. This is a core feature of CS and I would want to keep it pretty much as is, however super in ES6 has the following 'problems':

  • can only exist in constructors
  • must only be called with parens eg: super()
  • MUST be called in an extended constructor to access this
  • this doesn't exist in an extended class until super() is called.

and likely more that I don't know about yet. Almost all of the tests for classes would need to be rewritten if super really tries to emulate the ES6 super(). One of the biggest divergent areas is subclassing of member functions. CS6 doesn't seem to allow calling super for anything besides constructors.

I have no idea what @connec is putting together for super, but I suspect the final version will be split between generating the "official" CS6 when super is used correctly, ie in a constructor and with parens, and a tokenized version of super that continues to behave as the existing CS base. I don't think this needs to break any existing CS code.

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on 2016-12-16 08:35

@henning-koch - Interesting article. I suspect Symbol.species in this format will end up being used quite a bit.

Which means that we probably need to add get/set fairly soon to classes.

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2016-12-16 11:57

@mrmowgli regarding super:

Firstly, the way super is compiled in CS, roughly, is that a "super reference" is determined based on the containing method's name and context, some current examples:

Context.name = -> super             # doesn't compile
Context.prototype.name = -> super   # super reference: Context.__super__.name

class Context then @name = -> super # super reference: Context.__super__.constructor.name
class Context then name: -> super   # super reference: Context.__super__.name

# constructor is not special
Context.prototype.constructor = -> super   # super reference: Context.__super__.constructor
class Context then constructor: -> super   # super reference: Context.__super__.constructor

In #4354 (the active ES class PR) the only change for super is the last example above, all others have been left alone.

class Context then constructor: -> super # super reference: super

Ordinarily, the "super reference" is compiled with a .apply(this, arguments) or a .call(this, arg1, arg2, ...) depending on how super is called (bare super vs. super(arg1, arg2), etc). This doesn't work in an ES constructor because super in a constructor seems to be a very restricted syntactic form, rather than an 'auto' reference like arguments, hence when compiling a constructor super call there's a special case when the super reference is simply "super".

this doesn't exist in an extended class until super() is called.

That is an interesting case indeed. The way I chose to handle it in my PR is to add an implicit super to all derived constructors unless there's already an explicit one, e.g.:

class A
class B extends A then constructor: ->
# ...
#   class B extends superClass {
#     constructor() {
#       super(...arguments);
#     }
#   }
# ...

class A
class B extends A then constructor: -> super()
# ...
#   class B extends superClass {
#     constructor() {
#       super();
#     }
#   }
# ...

The reason I chose to do this is because it's the most reasonable way I could think of to deal with @-params and bound methods. Consider:

class B extends A
  constructor: (@name) ->

  greet: ->
    console.log "Hi #{@name}"

class C extends A
  constructor: (name) ->
    super()
    @name = name

  greet: =>
    console.log "Hi #{@name}"

Without an implicit super, class B is clearly broken: it doesn't call super so attempting to access its internal state will always fail. However, we can't simply add an explicit super as CS will compile the @-param into an assignment at the top of the constructor:

class B extends A {
  constructor (name) {
    this.name = name
    // super() - this is already too late
  }
}

C is also broken, though slightly more subtly: although it doesn't have @-params or reference this before calling super, it does make use of bound methods (greet: => ...). CS compiles method bindings into calls at the top of the constructor:

class C extends B {
  constructor (name) {
    this.greet = bind(this.greet, this)
    super() // - this is also too late
  }
}

Given these cases, there are basically 3 options:

  1. Disallow @-params in constructors and disable bound instance methods in extended classes (serious blow to backwards compatibility and utility, imo).
  2. Insert @-param assignments and method binding calls after the first super in a derived constructor. I haven't really explored this option, but I'm concerned that performing the insertion would be non-trivial and error-prone if super was used as part of an expression (e.g. result = super() is a valid expression, as is result = [super(), this.name]). I expect it could be done but have no doubt it would generate pretty ugly code.
  3. Add an implicit super, at the top of the function, to constructors with @-params or bound methods, and disallow explicit super in these cases. This preserves the original compilation of @-params and bound methods and results it more predictable behaviour, imo.

I went with an extension of 3., and decided to add an implicit super in all cases, which is easier to explain and understand. If a user strongly cares about how super is called, they would need to forgo using @-params or bound methods.

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2016-12-16 12:09

Sorry for the wall of text! Figured I'd try and explain the current status as fully as I could.

Having written that all out, I'm starting to feel like option 2. should be explored a little more. I guess the idea behind that would be to generate code that made it seem like @-param assignments and method bindings occurred as part of the call to super.

// class A
// class B extends A
//   constructor: (@foo) ->
//     console.log super(), @foo, @bar
//   bar: =>
class A {}
class B extends A {
  constructor (foo) {
    var ref;
    console.log((ref = super(), this.foo = foo, this.bar = this.bar.bind(this), ref), this.foo, this.bar)
  }

  bar () {}
}

This would behave reasonably well I think, as attempting to access @name or @greet before calling super would throw this is undefined, so from the user's perspective it doesn't matter that @name hasn't been assigned yet.

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on 2016-12-17 04:25

Ok, my bad: Super CAN be called in other methods, and super isn't required except in constructors for this:

var A = class A {
  constructor (token) {
    this.token = token;
    console.log('Token in base was: ', token); 
  }  
  objectLocal(origin) {
    console.log('The '+ origin + ' called in Parent method.'); 
  }
}

var B = class B extends A {
  constructor (token){
    super(token);
    this.token = token;
    console.log('Secondly, token `'+ this.token + '` was called in child.');  
  }
  objectLocal() {
    console.log('Overriding objectLocal now with "super"'); 
    console.log('Current token: ' + this.token) 
    super.objectLocal(this.token)
  }
}

let n = new B('coffee-script');
n.objectLocal();

/* Output:
  Token in base was:  coffee-script
  Secondly, token `coffee-script` was called in child.
  Overriding objectLocal now with "super"
  Current token: coffee-script
  The coffee-script called in Parent method.
*/

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2017-01-19 06:20

@connec @mrmowgli, fantastic heroic work on #4354. Now that that’s merged in, what work related to classes remains to be done?

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on 2017-01-19 11:17

I realize we didn't create browser specific tests, however I believe we are expecting Babel to cover the bases there. Other than that it seems like it's mostly edge cases surrounding super(), and those need actual usage to shake out a bit. I'm of the opinion we should go ahead with the release, the tests are comprehensive and everything works far better than I originally expected. @connec did an amazing job!

Realistically I believe we should start trying to cover lower priority items like get/set, species, and new modifiers like new.target . I don't believe these should hold up a 2.0 release however.

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2017-01-19 12:17

The only thing we probably want to finish off for 2 is super in regular methods. Ultimately this is optional (things will work as they are), however it would be a pretty big win for output clarity and reducing complexity in the compiler.

Unless someone else picks it up I will work on this once I make some time.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2017-01-19 16:48

@mrmowgli the tests really only cover Node, aside from the browser-specific ones. We treat Node as more or less our reference JavaScript runtime to test against, for JavaScript features that should behave identically in Node and browser-based runtimes.

@connec what do you mean by super in regular methods? Can you give some examples of current output and desired output? And how soon do you think you might have more time?

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2017-01-19 17:05

CS source

class Base
  method: ->
    console.log 'Base#method'

class Child extends Base
  method: ->
    super
    console.log 'Child#method'

Current CS2 output

// Generated by CoffeeScript 2.0.0-alpha
var Base, Child;

Base = class Base {
  method() {
    return console.log('Base#method');
  }

};

Child = (function(superClass) {
  class Child extends superClass {
    method() {
      Child.__super__.method.call(this, ...arguments);
      return console.log('Child#method');
    }

  };

  Child.__super__ = superClass.prototype;

  return Child;

})(Base);

Desired CS2 output

// Generated by CoffeeScript 2.0.0-alpha
var Base, Child;

Base = class Base {
  method() {
    return console.log('Base#method');
  }

};

Child = class Child extends Base {
  method() {
    super.method(...arguments);
    return console.log('Child#method');
  }

};

I could have time any day now 😄 Watch this space at the weekend...

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2017-01-19 17:13

There will be some 'tough choices' to make for this, I expect. I've not explored it much yet but if we wanted to clean up properly we would effectively lose our current support for super in assignment-based classes, e.g.

Base = ->
Base::method = -> console.log 'Base#method'

Child = ->
Child extends Base

Child::method = ->
  super
  console.log 'Base#method'

Some alternatives:

  • Leave the existing system hanging around, and lose some of the compiler-cleanup wins.
  • Replace __super__ with .getPrototypeOf and continue to support super in plain functions through that. This would clear up classes, but would make plain function super slower (would need to call Object.getPrototypeOf for every super call).

I'll flesh all these out a bit once I started looking at it.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2017-01-19 17:26

We already removed some tests that assumed a class was really a function (i.e. the pre-ES2015 class implementation). Your example that begins with Base = -> assumes that implementation, which I think should be considered unsupported in CS2. I think we should only allow super within methods that belong to classes, and in CS2 the only way to create a class is to use the class keyword.

When you get a chance, you should start documenting the class-related breaking changes in the wiki. You’re far more familiar with what’s changed than I am.

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on 2017-01-19 17:26

ES6 super doesn't work in methods added to the class prototype from other locations? Seriously? That's insane.

Edit: Regardless — moving to ES6 classes for CS2 means that you take the lumps that come with. We should definitely not still have the original system still lying around in the codebase for the CS2 release.

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2017-01-19 17:53

Nope @jashkenas, it's a 'syntactic form' that is only valid within a class initializer.

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2017-01-19 17:55

I think it's interesting that we could opt to compile super into Object.getPrototypeOf(this)[method] in basically any context. Only cost is the extra call to Object.getPrototypeOf, rather than caching it somewhere (such as __super__, currently).

@coffeescriptbot
Copy link
Collaborator Author

From @connec on 2017-01-19 18:44

Threw together #4424 with the bare bones.

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on 2017-01-19 19:10

Nope @jashkenas, it's a 'syntactic form' that is only valid within a class initializer.

It's just madness. It straightjackets class construction unnecessarily, and makes ugly lots of interesting patterns for mixing methods into classes, or composing traits. One step forward, three steps back.

I think it's interesting that we could opt to compile super into Object.getPrototypeOf(this)[method] in basically any context. Only cost is the extra call to Object.getPrototypeOf, rather than caching it somewhere (such as super, currently).

That seems like a pretty nice CoffeeScript feature that would improve upon ES6 classes.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on 2017-02-04 20:10

Closed via #4424. For further changes or improvements upon CS2 classes, please open new issues.

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

No branches or pull requests

1 participant