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: Features: Getters and Setters #4915

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

CS2 Discussion: Features: Getters and Setters #4915

coffeescriptbot opened this issue Feb 19, 2018 · 60 comments

Comments

@coffeescriptbot
Copy link
Collaborator

From @carlsmith on July 30, 2016 23:48

ES6 getters and setters don’t have there own discussion yet, and they came up in the discussion of decorators, specifically the need the ensure that any syntax that CoffeeScript adopts for defining getters and setters must allow decorators to be applied to the function literals.

This is a general discussion, but to carry over what was brought up already: From the example @carlmathisen used when he raised the issue, we can currently do this (note that readonly is a decorator):

class Person
  name: readonly ->
    return "#{@first} #{last}"

That works well, but the getter and setter syntax needs to allow decorators to be applied in a similar way (or an alternative must be provided).

EDIT by @GeoffreyBooth Consensus from the below thread:

The get and set shorthand syntax is too infrequently used, and a discouraged practice, for CoffeeScript to support directly. Getters and setters can be created via the Object.defineProperty method, so they technically already are supported in CoffeeScript; supporting the shorthand syntax as well just makes them more convenient to use, but Douglas Crockford argues that we should rarely if ever be using them.

So the task for CS2 is to have the compiler throw an error when it appears that a get or set shorthand syntax keyword is being used. Things like the following:

class A
  get b: ->

c =
  get d: ->

e = ->
  get f: ->

And set for all of the same. The above all compiles, which isn’t a good thing. These should throw compiler errors, without prohibiting people from using variables or functions named get or set elsewhere in the code. Basically when a call to a function named get or set is given an argument that is an object with one property, and that property’s value is a function, we throw an error. In other words, this:

  get({
    b: function() {}
  });

If anyone needs to call a function named get and pass an object, well, they just need to define the object ahead of time and assign it to a variable, and then call get like get(obj). That’s a reasonable workaround for what should be a tiny edge case. What we shouldn’t do, even though we could, is make get and set keywords. They’re not keywords in JavaScript, and they strike me as quite plausible names for functions, so I don’t want to cause a breaking change for people who have such variable names when we can solve this problem with more precision.

Copied from original issue: coffeescript6/discuss#17

@coffeescriptbot
Copy link
Collaborator Author

From @carlmathisen on July 31, 2016 4:47

Not really a solution, but more of an FYI if anyone were thinking in the same direction I were initially:

If we were to just add functions to a prototype object of an ES6 class when transpiling, it won't work well since super only works when using ES6 syntax sugar:

This works, but we don't have decorators out of the box like CoffeeScript has today:

class Person {
  speak() {
    return "hey"
  }
}

class John extends Person {
   speak() {
    return super.speak() +  " dude!"
  }
}

The following code would allow us to wrap the prototype function with decorators, but super fails.

John.prototype.speak = () => { 
  return super.speak() + " dude!"
  }
}

Uncaught SyntaxError: 'super' keyword unexpected here

Here's the code in Babel REPL

In other words, we probably need get/set.

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on July 31, 2016 6:32

the example given,

class Person
  name: readonly ->
    return "#{@first} #{last}"

is flawed in the sense that the context of this (@first) is not the same context as that readonly has available.

@coffeescriptbot
Copy link
Collaborator Author

From @carlmathisen on July 31, 2016 11:29

Extremely good point, @dadleyy. Fat arrow binding doesn't work either, since the scope becomes the prototype declaration of Person instead if this.

@coffeescriptbot
Copy link
Collaborator Author

From @objectkit on August 1, 2016 13:39

I hope this is on topic. Explicit support for getters and setters in the new CS would be good. However, if there were something like this...

class DataObject
    # meta propName/setterName collison
    set id (@id) ->

the first thing to come to attention is the naming of the property 'id'. In the example above, there is a setter named id, and a property called id. This is obviously not going to work at runtime in ES, as unfortunately, setter and getter names cannot have the same name as any other property. An easy solution would be:

class DataObject
    # using a property named _id
    set id (@_id) ->

but in an ideal world, it would be great if that was taken care of automatically in transpilation e.g. with a getter/setter | propName collision being rectified in the output code, perhaps using the underscore convention? E.g. in the first example, the output code would have a setter named id, but would store the value on a property named _id.

Another thing which would be nice (but difficult) would be implicit getters. I know. I said I hated implicit returns, and now I'm asking for discussion about implicit setters and getters! But hear me out...

If a single setter was implemented, wouldn't it make sense in the majority of cases to also have an implicit getter? Such that even though we would only see the typed code from the first example, the output code would look like this:

class DataObject {
    set id (value) {
        this._id = value;
    }
    // this implicit getter was added automatically by the compiler
    get id () {
        return this._id;
    }

Data leakage is the risk of course, as is the case with implicit returns, but just throwing it out there.

Anyhow, back on point. Support for getter/setter syntax in CS6 would be very good. Especially in the context of decorators...

class DataObject
    @final @type String
    set id (@_id) ->

In the example above, decorators could be used to ensure that only values of type String are provided, and that the setter can only be used once. Subsequent attempts would throw an error.

But to really stretch the boat out there, what if property definition was changed slightly? A property is declared as it is now, but, at discretion of the developer, setters or getters could be added only if needed. I suppose I am couching on using decorators as type enforcements on setters here...

class DataObject

    # standard definition of a prototype property as today
    id: null

    # an optional override of the setter for this property
    # set/get | prop name collision would be taken care of by the compiler with rewrites    
    @type String
    set id (@id) ->

Thoughts?

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on August 1, 2016 14:57

great stuff @objectkit.

Another thing which would be nice (but difficult) would be implicit getters. I know. I said I hated implicit returns, and now I'm asking for discussion about implicit setters and getters! But hear me out...

This idea and the example given kinda remind me of objective-c @property syntax which I think was a successful feature in that language. I think it is worthwhile for us to continue exploring.

but in an ideal world, it would be great if that was taken care of automatically in transpilation e.g. with a getter/setter | propName collision being rectified in the output code

very interesting! I'm not sure if we need to address this since you're going to have the same problem in es6 - not being able to have a getter/setter with the same name as another property (specifically the one that it is intended to work with). I'd say that it is up to the developer to design around this; what are they trying to do? Is there value of having a getter with the same name as a property or is that just a side-effect of wanting to have logic in a setter but needing to support access to the underlying data?

I think there is tremendous appeal to have logic in the setting of values; I'm pretty sure this is how vuejs handles data binding and dom updates, but I also get scared that people might end up mis-using this feature - creating complex setter where they should be defining functions on the prototype; I'm not sure we would want to see things like this:

class User {
  set name(val) {
    let {errors} = this;
    this.$name = val;
    function success() {
      errors["name"] = false;
    }
    function fail() {
      errors["name"] = true;
    }
    this.save().then(success).catch(fail);
  }
  get name() { return this.$name; }
}

On the other hand, I feel like if some kind of mutation needs to happen to the value of a property it's better to come up with a new property and add the getter there. For example, if the developer is trying to:

class TokenID {
  get id() {
    return `${this.token}_${this.id}`;
  }
}

they really should probably be doing:

class TokenID {
  get token_id() {
    return `${this.token}_${this.id}`;
  }
}

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on August 1, 2016 15:7

We're also going to want to figure out how to handle defining getters and setters outside the initial definition of classes like the can do with functions now. For example we're currently allowed to:

class User
  constructor: ->

User::anotherFn = ->

what would this look like for getters/setters? If we decide to go with the get/set as keywords:

class User
  get full_name: ->

I think we might be hurting ourselves in the sense that I'm sure we'd want to avoid having to support something like:

User::get full_name = ->

Generally speaking I'm more in favor of a language's use of keywords than operators (isnt/is vs ===/!==) in situations like this but If we were to use a new function operator (e.g ~>) we would be able to have more flexibility in where we can define getters and setters:

class User
  full_name: ~>           # getter
  full_name: (new_val) ~> # setter

User::full_name = ~>           # getter
User::full_name = (new_val) ~> # setter

@coffeescriptbot
Copy link
Collaborator Author

From @carlmathisen on August 1, 2016 16:22

Just remember that by adding functions on the prototype object and not in the class definition, super isn't available anymore. Thus removing an important feature of ES6 classes.

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on August 1, 2016 16:53

@carlmathisen good call - I didn't know that. I wonder why super is restricted that way; isn't it possible to accomplish accessing a super class's prototype somehow anyways? Is there an ask that the compiler always "lifts" prototype definitions into the main definition of classes then? We'd still write:

class FriendListManager
  constructor: (@user) ->

FriendListManager::addFriend = (other) ->

but the compiler would raise addFriend into the compiled class definition:

class FriendListManager {
  /* ... */
  addFriend(other) {
  }
};

I think this raises complications regarding the compiler's ability to check the presence of the initial definition and what do to if it is missing. Anyways I think we'd want to do some sort of "lifting" for getters/setters right?

side note - the mdn website has an issue with their example on the super page; their Square class defines getters and setters for the area property but attempt to assign it a value in the setter:

class Square extends Polygon {
  constructor(length) {
  }

  get area() {
    return this.height * this.width;
  }

  set area(value) {
    this.area = value;  // bad: infinite call stack
  } 
}

@coffeescriptbot
Copy link
Collaborator Author

From @rattrayalex on August 5, 2016 4:47

I think ~> is interesting, but a tricky bit of syntax to have to remember for a relatively infrequently used feature.

I think the User::get full_name = -> concern likely does not apply; in this case, one would probably use Object.defineProperty in any case. And I don't think it's a style of programming that should probably be encouraged...

Are there likely technical barriers to using get and set keywords in a way that mimics ES6?

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on September 6, 2016 1:23

What’s our consensus here? Are getters and setters a feature we want CoffeeScript to support?

If so, can someone write an example showing the proposed syntax? Ideally the syntax would be backwards-compatible or as close to backwards-compatible as possible.

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on September 6, 2016 17:45

I think we should take the same stance on this issue that we are on the module syntax and classes in the next compiler - get the syntax as close to es6 as possible and then it's the job of the next transpiler (babel) to worry about older runtimes.

Therefore, my suggestion is that we support:

# inside class definition
class DietManager

  constructor: -> # ...

  get total_calories: ->
    total = 0
    total += calories for {calories} in @meals
    total

  set field: (new_value) ->

# inside object literal
current_user =
  get full_name: -> "#{@last_name}, #{@first_name}"

and then compile that down to it's es6 form:

class DietManager {
  constructor() { }
  get total_calories() { /* ... */ }
  set field(new_value) { /* ... */ }
};

var current_user = {
  get full_name() { /* ... */ }
};

I will say that I think this is a must-have. There are good reasons to avoid the over-use of these features but I say leave it up to the end user to know the right time and place for them - same is true for classes/generators/etc...

@coffeescriptbot
Copy link
Collaborator Author

From @rattrayalex on September 8, 2016 4:45

Personally I think it makes sense to build get and set if they're build-able. They're good features, and don't have downsides. But if it's too hard to build, I believe it's possible to get the benefits with Object.defineProperty.

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on September 8, 2016 5:43

I believe it's possible to get the benefits with Object.defineProperty

Most definitely. Currently, if I want getters on a class I usually do the following sugar inside my modules (with an example not split up into modules for brevity):

attr = (target, name, descriptor) -> Object.defineProperty target, name, descriptor
attr.access = (target, property, get) -> attr target, property, {get}

class Revisions

  constructor: (@original = {}) ->
    @revision_history = []
    attr.access this, "latest", @apply

Revisions::apply = ->
  {revision_history, original} = this
  result = Object.assign {}, original
  # ... use items in store to apply to result ...
  result

class UserManager extends Revisions

  constructor: (user) ->
    super user

# ... methods to update parts of the user, e.g:
UserManager::addFriend = (other_user) ->
  {revision_history} = this
  revision_history.push {type: "friendship", payload: other_user}

UserManager::commit = (callback) ->
  {latest} = this
  # save to API...

# ... elsewhere:

manager = new UserManager {name: "danny"}
{latest: user} = manager

I just think it would be nice to see this kind of thing be recognized as something that - considering writing it in es6 would be more concise - should make it into cs w/o the need for the user to provide that sugar. I think getters are a nice gateway into very cool patters seen in immutablejs and redux.

@coffeescriptbot
Copy link
Collaborator Author

From @connec on January 31, 2017 23:46

I'm not sure about adding the syntaxes get property and set property to CS. Not only do they look like totally valid method calls, they're actually ambiguous in most cases. Getting this to parse cleanly, without a bunch of hacks in the parser as well as in the AST, would be a challenge I expect.

class A
  get name: -> # get({ name: function () {} })
  set name: -> # set({ name: function () {} })

get name: -> # get({ name: function () {} })
obj = { get name: -> } # Probably a getter
obj =
  get name: -> # Getter, maybe?
obj = get name: -> # obj = get({ name: function () {} }) ?

key: get name: -> # { key: get({ name: function () {} }) }
key: { get name: -> } # Probably a getter

We could say it's only valid in explicit objects, except that would mean they're not valid in classes.

On top of that, I'm not sure that enabling getters and setters - which commit the sins of making expensive operations look cheap, and potentially side-effecting operations look pure - is a good idea.

Consider immutablejs, which explicitly uses defineProperty when creating its Record types. The definition of the Record constructor uses defineProperty, but you don't need to write any getters or setters in order to use them.

I think the Object APIs are sufficient for the few important uses of getters and setters.

@coffeescriptbot
Copy link
Collaborator Author

From @rattrayalex on February 1, 2017 0:16

I don't really think this would jibe super well with coffeescript, but in case it's the syntax that's a problem, here's how I'm solving it in LightScript:

class A:
  prop() -get> 
    this._prop
  prop(newValue) -set> 
    this._prop = newValue

@coffeescriptbot
Copy link
Collaborator Author

From @triskweline on February 1, 2017 9:39

On top of that, I'm not sure that enabling getters and setters - which commit the sins of making expensive operations look cheap

The Uniform access principle has a long tradition in language design.

Getters and setters also allow us to avoid breaking changes when we later decide that a stored property must be computed or delegated.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on February 1, 2017 22:51

I don’t think getters and setters rise to the level of something so abhorred that they should be banned from the language. They’re right there in the MDN example of a class. Ember.js uses them extensively.

I understand people can use defineProperty to create getters and setters today, but that seems like forcing people to use a workaround.

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on February 1, 2017 22:53

Indeed its an entrenched language feature at this point. I would love a
coffee-esque syntax but I would rather have the option to use them somehow
without going the long way round.

On Feb 1, 2017 2:51 PM, "Geoffrey Booth" notifications@github.com wrote:

I don’t think getters and setters rise to the level of something so
abhorred that they should be banned from the language. They’re right there
in the MDN example of a class
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes.
Ember.js uses them extensively.

I understand people can use defineProperty to create getters and setters
today, but that seems like forcing people to use a workaround.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
coffeescript6/discuss#17 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABy6x1HeOwsM0LyobENayx60VD4vRh5fks5rYQx4gaJpZM4JY63G
.

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on March 13, 2017 20:5

As requested by @GeoffreyBooth, moving a comment over to this ticket.

As a side note — it would be nice if discussion around language features could happen over on the coffeescript Issues pages, so that folks who are watching the project can get alerted that they're being talked about / decisions being made.

Now to the meat:

If there is some fundamental compatibility reason why we must have getters and setters in CoffeeScript, then that's a little sad, but it's alright.

But up until that point, getters and setters have always been an intentional omission from CoffeeScript (like named functions, manual variable declaration, loose equality ==, and so on). They're one of the bad parts of JavaScript that CoffeeScript as a concept benefits from leaving out.

As you write ES3, you know what's a function call and what's a property access. When you use getters and setters, it's suddenly opaque to you which is which, with the possibilities of serious performance considerations and also side effects in places where you might not expect them.

This uncertainty brings with it a little bit of syntax sugar — to make a method call look like a property access. Why not just let a method call be a method call?

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on March 13, 2017 20:9

Why not just let a method call be a method call?

It is nice to be able to use the "computed value"-ness of getters in browser-side rendering where templates come into play:

<p>{{diet_manager.total_calories}}</p>

although I suppose you could always just calculate the value in code before rendering or have some kind of call helper.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 13, 2017 20:14

So when I last commented I was thinking about Ember.js’ computed properties, which I assumed use get and set under the hood. (And maybe they do, I haven’t checked.) I tried to find an example of Ember requiring a developer to use getters and setters and I couldn’t, basically because they created Ember.get and Ember.set as wrappers for the functionality. The closest I could find is this: emberjs/rfcs#108

But I think my general point is still valid. There’s a lot of activity nowadays in frameworks that observe plain JavaScript objects (React, Vue, etc.) and re-render as a result of changes. I think it’s only a matter of time before some framework will require the object being observed to use getters and setters; or if one wants to write a component or plugin for one of these libraries, it will need to use getters and setters. (Imagine writing a plugin for a new type of Ember computed property.) Of course you can always go the “long way around” and use Object.defineProperty, but that feels worse than backticks. Granted, my worry is hypothetical, so maybe it’s not worth worrying about until there’s a concrete example. But seeing how rapidly frameworks like React adopt even non-finalized ES7 features like decorators, I feel like it’s only a matter of time before someone decides that getters and setters are a must-use, especially since they’re part of ES5.

One last “real” case: the JavaScript proxy pattern:

(function() {
    // log all calls to setArray
    var proxied = jQuery.fn.setArray;
    jQuery.fn.setArray = function() {
        console.log( this, arguments );
        return proxied.apply( this, arguments );
    };
})();

Sometimes I want to modify an imported library function, without forking the underlying library. I might use something like the above. If the library in question used a getter or setter instead of a function, I would need to use a getter or setter to override it. Again, I could use Object.defineProperty, but that feels like a hack. (Not that the proxy pattern is a hack 😉 .)

More fundamentally, if it’s not discouraged by MDN—and indeed, it’s right there in their basic class example—and it can already be achieved via Object.defineProperty, then what do we gain by denying people a more-convenient syntax?

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 13, 2017 20:35

And @jashkenas as to your other point, yes, going forward let’s discuss new features/revisions in the main repo. Maybe after we finish the last two items on the to-do list we can migrate this repo’s issues into that one.

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on March 13, 2017 20:49

<p>{{diet_manager.total_calories}}</p>

That can be:

<p>{{diet_manager.total_calories()}}</p>

There's no reason to hide it as a property access, when it isn't.

Granted, my worry is hypothetical, so maybe it’s not worth worrying about until there’s a concrete example.

Let's do that.

Sometimes I want to modify an imported library function, without forking the underlying library. I might use something like the above.

In that case of overwriting a specific property, using Object.defineProperty seems like a totally fine way to do it. In fact, you'll probably have to do that regardless — as you're going to be changing the class (or object) from the outside.

then what do we gain by denying people a more-convenient syntax?

CoffeeScript isn't about just including willy-nilly all of the syntax shortcuts under the sun. It's about trying to design a comfortable, minimal, enjoyable, smaller-than-JavaScript language. Getters and setters are not only unnecessary, they introduce the performance and potential side effect uncertainty (I described earlier) all over your codebase.

@coffeescriptbot
Copy link
Collaborator Author

From @balupton on March 13, 2017 20:58

When moving bevry/taskgroup from CoffeeScript to ESNext, we changed all our getSomeReadOnlyComputedProperty() methods to getters. There is no real advantage for this. It is the only use case that I can think of where I've ever used them.

However, one possibility for why getters/setters should be in CoffeeScript, would be that not adding them would make it impractical to override them in JavaScript classes that do use them. One would have to go the defineProperty route, which is quite tedious - perhaps people would just not bother with coffeescript for that class extension - although, who knows really, perhaps it is not an issue at all.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 13, 2017 21:3

FWIW, <p>{{diet_manager.total_calories()}}</p> isn’t possible in Handlebars or Handlebars-like templating libraries. Though one could argue that that’s a failing of those libraries, not CoffeeScript’s problem.

I had assumed that a lack of support for getters and setters would be a compatibility problem similar to how CoffeeScript 1.x’s inability to extend ES class presented a compatibility problem with libraries like React. If that’s not the case, I’m fine with moving this into the “out of scope” column; until it does become a compatibility issue, in which case we should address it. But since we already know the workaround, fixing this won’t be urgent whenever that compatibility issue arises.

Unless anyone knows of any compatibility issues already with a lack of support for get and set?

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on March 13, 2017 21:9

FWIW,

{{diet_manager.total_calories()}}

isn’t possible in Handlebars or Handlebars-like templating libraries. Though one could argue that that’s a failing of those libraries, not CoffeeScript’s problem.

That's exactly the point.

I had assumed that a lack of support for getters and setters would be a compatibility problem

There should be some old old tickets lying around from 2010-ish discussing this. As fair as I remember, defineProperty has you covered.

Edit:

To be fair, this is the best argument for adding getters and setters: https://en.wikipedia.org/wiki/Uniform_access_principle

... so that you can later change a property access into a computed property, without breaking all of the call-site consumers of your API.

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on March 13, 2017 21:33

I agree with both sides on this one, rough.
This might be an issue of documentation first - Putting up an explanation why get/set pairs are missing.

commenceRamble() {

I personally never liked the idea of getters and setters from my Java days, as it became a mandatory coding practice and just generally adds unnecessary cruft.

That being said, since get/set introduction in ES5 there have been a plethora of ways to lock down client side code:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty

and get/set is a part of that. I'm all for simplifying this type of pattern.

For anybody trying to use this path it's just not obvious that CS intentionally avoids it, hence the need for documentation.

As @GeoffreyBooth points out though this is very common point on example code, and I have had to work around porting get/set code on quite a few occasions. javascript2coffee transpilers usually break. I can also see the latest generation of ES users going "Wha wha wha??" every time they try to make this happen.

I mentioned before in this thread and in the ES6 Classes thread that there are a lot of edge cases where having the functionality makes sense, like in this example:
http://www.keithcirkel.co.uk/metaprogramming-in-es6-symbols/#symbolspecies
Where a getter is used to help with meta-programming.

I suspect though that these edge cases could be handled without direct get/set support in the language.

The real goal of get/set is to isolate read/writes to a property, optionally creating side effects.
in other words a property using get isn't directly readable or writeable without a function hook.

As @jashkenas points out, this is entirely possible just defining functions and hiding properties within a closure.

This pattern is still common enough where we should definitely think about a simplified way of accomplishing it.

Soooo IMO, lets at least document this for now, to avoid continuous confusion, and keep asking for examples where this is a required pattern.
}

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 13, 2017 21:40

Yes, at the very least there should be a section in the docs explaining things that CoffeeScript intentionally doesn’t support, so people don’t keep asking for them. get/set, let/const, etc.

@coffeescriptbot
Copy link
Collaborator Author

From @wcjohnson on March 13, 2017 21:56

We're all interoperating with a gigantic ecosystem of libraries written against a language where property accessors are now a documented standard feature. No substantial JS project is going to be able to avoid these implicit method calls.

I don't see taking a moral stand on the matter as generating a concrete gain. In fact, even a moral purist could regard this as a loss, given enough foresight. Consider that the huge impedance mismatch between CoffeeScript and the evolving ES standard has caused many projects to drop the language in recent months -- if that trend continues, any benefit from a shrinking minority of people writing morally pure code will be overwhelmed by the rising tide of those writing getters and setters.

I've been pretty much the only one on my team evangelizing for CS lately -- or, should I say, dragging people kicking and screaming because I can show them proof that I get the most done. And I owe a lot of that to CS, which has probably saved me man-months worth of typing compared to coworkers who stick to JS. I was thrilled about CS2 because it's really wiping out a lot of those impedance mismatches. But if I have to type Object.defineProperty(@.prototype, blah, ->blah) and they can just type "get" I'm still that much closer to losing the argument. And I think it is a real argument for a lot of teams out there.

To sum up, I think it's very unwise to deliberately cut off access to documented standard features of the language for moral reasons. I was sympathetic to the arguments about introducing further grammatical ambiguities. Less so to this.

@coffeescriptbot
Copy link
Collaborator Author

From @connec on March 15, 2017 11:49

As a separate point - the get/set syntax becomes a bit of a moot point once decorators are available. It should be easy to write or require a decorator that turns a method into a getter or setter.

import { get } from 'core-decorators'

({
  @get x () { return 42 }
})

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on March 15, 2017 14:28

Hah — decorators are even worse than get and set ;)

But your first point is well taken, and I agree wholeheartedly:

So this is in the language now, be really really careful with it.

It's been 'in the language' far longer than the syntax has, through Object.defineProperty. To me that feels like the right balance - if you have a really compelling reason to use getters and setters (e.g. DOM library, backwards compatibility), you can do it, but it will be obvious and you won't like doing it. Otherwise you will stay away from them, which is a Good Thing™.

I think you’d do much better to stay with a methodic approach.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 15, 2017

If we do add support for the syntax at some point, it would be a breaking change since currently get foo: -> in a class or object becomes get({foo: function(){}). I'd rather get all our breaking changes done before 2.0.0.

I think we should perhaps add some kind of check where if you start a line in an executable class body with what looks like a call to a function named get or set or static, an error is thrown. That way if we do ever add support for the short syntax, it's not a breaking change. If someone really does want to call a function named get, they can wrap it in parentheses. Most likely anyone who gets this error is probably assuming that CoffeeScript supports the get/set syntax, so such a check would likely be preventing a confusing bug.

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on March 15, 2017

I think we should perhaps add some kind of check where if you start a line in an executable class body with what looks like a call to a function named get or set or static, an error is thrown. That way if we do ever add support for the short syntax, it's not a breaking change.

Making get and set and static reserved words for 2.0, with a compile error saying that they're not supported (yet) — sounds like a great way to go.

@coffeescriptbot
Copy link
Collaborator Author

From @rattrayalex on March 15, 2017

CoffeeScript is still @jashkenas 's language; sounds like he's not interested in syntax to support this.

This seems as good a time as any to mention that LightScript, the syntax I've been working on since November, is now ready for non-production use: http://www.lightscript.org

It's a "mostly-superset" of ES7+JSX+Flow, so virtually 100% of JS features are supported, including getters and setters.

I'm eager for feedback from this community; send me an email (rattray.alex@gmail.com) or message on gitter (https://gitter.im/lightscript/Lobby).

@coffeescriptbot
Copy link
Collaborator Author

From @jashkenas on March 15, 2017

This seems as good a time as any to mention that LightScript, the syntax I've been working on since November, is now ready for non-production use: http://www.lightscript.org

Very cool! The more the merrier.

I particularly enjoy your now keyword and explicit shadowing. Also your "an indent is two spaces, period" attitude. Also your function naming syntax.

A couple of questions:

Your Big Table Of for is indeed a bummer. Since you're including flow typechecking as part of the core language, is there not a way to unify the iteration syntax, and compile based on the type of object being iterated over?

Why did you choose to use colons for your significant whitespace? The be-careful-with-your-whitespace rules at the very bottom of the documentation make it look a little bit messy. I guess that's the price you pay for aiming to be a near superset of JavaScript?

@coffeescriptbot
Copy link
Collaborator Author

From @rattrayalex on March 15, 2017

Thanks!! Means quite a lot coming from you.

At risk of derailing the thread a bit...

  1. for-complexity, yeah, I was hoping to be able to leverage Flow for that, but the implementation sounded risky (eg; leaning on the typechecker for compilation could slow things down a lot). I've been thinking about some alternatives that put the decision explicitly in the developer's hands; would love your thoughts on them. I'll shoot an email?

  2. re; colons, several reasons (less ambiguity for humans, easier parsing in the context of babylon, etc). I think the biggest reason was smooth transitions from single-line to multi-line syntax (just add/remove a newline+indent, rather than ruby, say, where you have to move things around).

  3. re; careful-ASI-rules, yeah... I'm hopeful that the docs are the messiest part of that, and that it just won't be something developers have to think about in practice, since using good style means the compiler does what you want it to, period.
    The plan would be to fork prettier for LightScript and bundle it such that most people using LightScript are using the formatter (and, similarly, bundle the typechecker and a consistent set of ESLint rules, so there is One Right Way for most things). If/when that happens, developers will get good style (and thus good ASI) without having to do any work at all.

I'll reach out for further conversation – I've actually been hoping to reach out for mentorship on the project for a while 😄

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 15, 2017

Making get and set and static reserved words for 2.0, with a compile error saying that they’re not supported (yet) — sounds like a great way to go.

It’s not quite that simple, unfortunately, as get and set aren’t reserved words in JavaScript (though static is, oddly enough). We would have to great these like we currently handle from, which is to treat it as a reserved word in special situations, but otherwise allow its use as an identifier. But I think it shouldn’t be too much more difficult to do so. @connec ?

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on March 15, 2017

I realize this may not completely apply but:
I really like this comment by @carlsmith : Annotations are just higher order functions I believe it could apply to get and set as well. What if we just document this style choice and show them how to achieve this in a functional way? Then quote the Crockford section as well.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 15, 2017

@mrmowgli Yes I think a section in the docs might be in order, where we explain the get/set policy, as well as let/const and other common things people trip over or complain about. Feel free to write it and submit a PR against 2 😉

@coffeescriptbot
Copy link
Collaborator Author

From @dadleyy on March 15, 2017

CoffeeScript is still @jashkenas 's language; sounds like he's not interested in syntax to support this.

It's also a language that people have been forced to part from for exactly that reason, and the same reason this organization and this discussion started in the first place. I love coffeescript and will always be thankful for its role in pushing javascript towards a compiled language with features that encourage developers to think about the design of their code. If the sentiment here is that the future of coffeescript is wed to its past, so be it. It was a huge player in the advancement of modern javascript development and I'm sincerely forever grateful.

Since the inception of this "lets modernize coffeescript" conversation I've fallen in love with vanilla es6, typescript, golang, etc... I've been wrong in my initial judgment about stuff and will probably continue that trend, so whats scares me the most is this notion of preserving the language's unyielding flexibility as it relates to right vs wrong vs leave-it-in-the-developers-court.

I introduced coffeescript to previous codebases I was a part of when object destructuring was a revelation. I was also a part of the same codebases when babel and libraries like vuejs, react, ember, etc... ran laps around my argument to continue the use of language that did not adapt in an environment that prescribed exactly that. There are a whole bunch of naughty things developers can do with any language. To me, getters (again, I'm not sure I can make the same argument for setters) embody the type of terseness and beauty that I got out of writing coffeescript in the first place and am saddened to hear this kind of resistance to it.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 22, 2017

So the PR for documentation is in progress over at #4469 (thanks @mrmowgli). I think the other thing that we have consensus that we should do is have the compiler throw an error when it appears that a get or set keyword is being used. Things like the following:

class A
  get b: ->

c =
  get d: ->

e = ->
  get f: ->

And set for all of the same. The above all compiles, which isn’t a good thing. These should throw compiler errors, without prohibiting people from using variables or functions named get or set elsewhere in the code. Basically when a call to a function named get or set is given an argument that is an object with one property, and that property’s value is a function, we throw an error. In other words, this:

  get({
    b: function() {}
  });

If anyone needs to call a function named get and pass an object, well, they just need to define the object ahead of time and assign it to a variable, and then call get like get(obj). That’s a reasonable workaround for what should be a tiny edge case. What we shouldn’t do, even though we could, is make get and set keywords. They’re not keywords in JavaScript, and they strike me as quite plausible names for functions, so I don’t want to cause a breaking change for people who have such variable names when we can solve this problem with more precision.

Is everyone okay with this proposal? @connec, what do you think, and how difficult do you think it would be to implement? I’m thinking that perhaps it might not require a change in grammar, since we’re just throwing an error?

@coffeescriptbot
Copy link
Collaborator Author

From @connec on March 24, 2017

Whilst it is probably a pragmatic way of ensuring we don't get bug reports about get and set, it does seem a bit hacky - would documenting a 'gotcha' not be sufficient?

@GeoffreyBooth I reckon those could be caught reasonably easily by adding a check to Call nodes that throws if the variable is an identifier named get or set, and the first argument is an implicit object literal (obj.generated is true). I suppose the error message would be something along the lines of

error: getters and setters are not supported - disambiguate with explicit parentheses or braces.

The case that would be harder to catch with a helpful error would be { get f: -> } - currently that throws error: unexpected ( which might be interpreted as a bug in the compiler when handling get and set. That error looks like an error from the grammar, which I'm not sure how to trap.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on March 24, 2017

Whilst it is probably a pragmatic way of ensuring we don’t get bug reports about get and set, it does seem a bit hacky - would documenting a ‘gotcha’ not be sufficient?

My other motivation for throwing an error is to avoid breaking changes in the future if it turns out we need to support this syntax for some reason. If we make the get/set shorthand syntax throw an error now, then if we change our minds and decide to support the shorthand syntax someday, it wouldn’t be a breaking change.

@coffeescriptbot
Copy link
Collaborator Author

From @PandaWhisperer on May 7, 2017

Apologies in advance for just barging in here and commenting on a closed issue, but I've been a big fan of CoffeeScript for quite some time, and I was really stoked when I found out recently that not only did ES6 not kill it, but version 2 is inching closer to a release. 😃

However, I'm a bit curious about your stance on getters and setters. Now, I'm really not in a position to discuss the technical merits much, since I haven't coded much in CoffeeScript or ES6 recently. But I would like to point out that they're in ES6, and CoffeeScript aims to be a superset, so that kinda makes me feel that they should be supported. Is it really that complicated? Or is the argument just that Crockford doesn't like them?

Anyhow, whatever the final decision will be, perhaps it would be nice to mention them on the CS2 website.

Also, I'd be happy to volunteer if there's anything I could help out with. After having taken a much-needed break from coding, I feel ready to jump back in the game. And what better way to do that than giving back to the community that's given me so much cool stuff to play with in the past?

Cheers! 🍻

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on May 7, 2017

A mention is in the docs on the 2 branch, that will get published to coffeescript.org with the next beta release: https://rawgit.com/jashkenas/coffeescript/2/docs/v2/index.html#unsupported-get-set

Something to keep in mind is that getters and setters are supported in CoffeeScript, even version 1.x. It’s just the shorthand syntax that isn’t supported. The note in the docs explains how to use the verbose syntax. We could’ve found some way to support the shorthand syntax or something similar to it, but the short answer is that the grammar is ambiguous (is it get foo or get(foo)?) and they’re considered a bad practice anyway, so we shouldn’t be making them easier to use.

Also, CoffeeScript is intentionally not a superset of JavaScript: https://rawgit.com/jashkenas/coffeescript/2/docs/v2/index.html#unsupported. It never has been, ever since the beginning and the lack of var or function declarations. That said, CoffeeScript does try to support all the “good parts” of JavaScript, including good new ES features that reach Stage 4.

Help is most welcome! https://github.com/jashkenas/coffeescript/issues?q=is%3Aopen+is%3Aissue+label%3Apriority

@coffeescriptbot
Copy link
Collaborator Author

From @CliffS on May 7, 2017

they’re considered a bad practice anyway, so we shouldn’t be making them easier to use.

I think there is a danger of CS evangelising over this when the battle has effectively already been lost in ES6. I have to say that I agree with @wcjohnson above when he said:

But if I have to type Object.defineProperty(@.prototype, blah, ->blah) and they can just type "get" I'm still that much closer to losing the argument. And I think it is a real argument for a lot of teams out there.

Please reconsider.

@coffeescriptbot
Copy link
Collaborator Author

From @mrmowgli on May 7, 2017

Getters and Setters have been in ECMAScript since ES5, when CoffeeScript was still in its infancy. I sincerely doubt the decision was arbitrary, and I know this has been heavily discussed in issues for years.

Getters and setters aren’t even being adopted by full ES6 shops like AirBnB (https://github.com/airbnb/javascript/blob/master/README.md#accessors–no-getters-setters) who are on board with everything else.

While it seems like this is a reasonable request, getters and setters weren’t intended to be used extensively in the ECMAScript.

@coffeescriptbot
Copy link
Collaborator Author

From @joeldrapper on May 8, 2017

FYI, there’s a really elegant way to use getters / setters in CoffeeScript version 1 and 2 (thanks @carlmathisen) by creating a new method on the Object class. I’ve called it property, but you could call it defineProperty, accessor or anything else.

Object::property = (name, accessors) ->
  Object.defineProperty @::, name, accessors

Example use

class Person
  @property "name",
    set: (value) ->
      names = value.split " "
      @firstName = names[0]
      @lastName = names[names.length - 1]

    get: ->
      "#{@firstName} #{@lastName}"

joel = new Person
joel.name = "Joel Drapper"

console.log joel.firstName
# "Joel"

console.log joel.lastName
# "Drapper"

console.log joel.name
# "Joel Drapper"

If you’d rather not risk extending the Object class, you can create your own BaseObject class with a property class method and extend that instead.

class BaseObject
  @property: (name, accessors) ->
    Object.defineProperty @::, name, accessors

class Person extends BaseObject
  ...

@coffeescriptbot
Copy link
Collaborator Author

From @carlmathisen on May 8, 2017

@joeldrapper This also works in CS1 btw 👍

@coffeescriptbot
Copy link
Collaborator Author

From @CliffS on May 8, 2017

@joeldrapper That's great. If the devs aren't going to add getter/setters to the language, at least could we have this trick in the unsupported section of the manual?

@coffeescriptbot
Copy link
Collaborator Author

From @joeldrapper on May 8, 2017

@CliffS Yes, it might be helpful to mention the technique here.

@coffeescriptbot
Copy link
Collaborator Author

From @GeoffreyBooth on May 8, 2017

The docs are in the repo. See the markdown files in documentation/sections, and the examples they reference in documentation/examples, on the 2 branch. Please feel free to submit a PR.

@coffeescriptbot
Copy link
Collaborator Author

From @PandaWhisperer on May 8, 2017

@GeoffreyBooth thanks for clearing that up. When I searched the page I didn't find anything, but I looked again yesterday and now the lack of the shorthand syntax (including the motivation for it) is present.

@joeldrapper I've seen this trick mentioned elsewhere before and I think it's fairly neat, save for the need to extend Object or insert a new base class into your own object hierarchy. I do agree with @mrmowgli that while the Object.defineProperty syntax might work, it's definitely too cumbersome and lengthy to use on a regular basis. But I can see the issue the core devs are having with the proposed syntax, which would potentially definitely break backwards compatibility with CS1.

+1 for @CliffS idea to at least mention the workaround in the main documentation.

@coffeescriptbot
Copy link
Collaborator Author

From @Romaboy on Nov 18, 2017

Did anyone mentioned that you can delete get and make lazy loaded property, but you can't delete defineProperty?

@GeoffreyBooth Are you seriously offering to use forks in issues? So that project readme should contain "if you gonna work with this code you need to install fork, cause vanilla coffeescript is incompatible with modern js"

@coffeescriptbot
Copy link
Collaborator Author

From @joeldrapper on Jan 1, 2018

Call me crazy, but would it be such a bad idea to define getters and setters like Ruby? Methods without arguments are getters and methods ending in = with just one argument are setters.

class Person
  name=: (value) ->
    names = value.split " "
    @firstName = names[0]
    @lastName = names[names.length - 1]

  name: ->
    "#{@firstName} #{@lastName}"

  save: () ->
    ...

You could force a normal method with no arguments like this save: () ->.

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