Skip to content
This repository has been archived by the owner on Feb 19, 2018. It is now read-only.

CS2 Discussion: Output: Destructuring #69

Closed
GeoffreyBooth opened this issue Jan 23, 2017 · 17 comments
Closed

CS2 Discussion: Output: Destructuring #69

GeoffreyBooth opened this issue Jan 23, 2017 · 17 comments

Comments

@GeoffreyBooth
Copy link
Collaborator

GeoffreyBooth commented Jan 23, 2017

Building off of #18, we should consider changing CoffeeScript’s destructuring output from the current ES3 to ESNext syntax. So this:

numbers = [1, 2, 3]
pairs = { a: 1, b: 2, c: 3 }

[one, two, three] = numbers
{one, two, three} = pairs

foo = ->
  [@a, @b, @c] = numbers
  {@a, @b, @c} = pairs
  return

which currently compiles to this:

var foo, numbers, one, pairs, three, two;

numbers = [1, 2, 3];

pairs = {
  a: 1,
  b: 2,
  c: 3
};

one = numbers[0], two = numbers[1], three = numbers[2];

one = pairs.one, two = pairs.two, three = pairs.three;

foo = function() {
  this.a = numbers[0], this.b = numbers[1], this.c = numbers[2];
  this.a = pairs.a, this.b = pairs.b, this.c = pairs.c;
};

would instead compile to this (?):

var foo, numbers, one, pairs, three, two;

numbers = [1, 2, 3];

pairs = {
  a: 1,
  b: 2,
  c: 3
};

[one, two, three] = numbers;
({one, two, three} = pairs);

foo = function() {
  [this.a, this.b, this.c] = numbers;
  ({a: this.a, b: this.b, c: this.c} = pairs);
};

The array destructuring, as you can see here, is pretty straightforward; but the object destructuring presents issues. First, if we’re not declaring the variable at the same time (as we never do in CoffeeScript, hence the var line of declarations at the top of each scope) then we need to wrap the destructuring assignment in parentheses, per MDN. And if the target variable is attached to this, we need to use the “assigning to new variable names” shortcut to destructure into our intended destination variable, unless people have a better suggestion for how to handle this.

There’s also the question of whether this is worth the effort. The current output is already rather readable, so the gains aren’t great. Destructuring, whether in CS or ES, is just a time-saver; anything you can do with destructuring you can also already do in more verbose variable assignment, so there’s no compatibility need for us to change anything. If anything, the lack of need behooves us to make sure that if we make this change, it is done in a fully backward compatible way.

Edit: Fixed destructuring object with this per @lydell’s correction.

@lydell
Copy link

lydell commented Jan 23, 2017

Don't you mean this for destructuring objects with @:

({a: this.a, b: this.b, c: this.c} = pairs);

Note that there is one big difference between CS destructuring and ES. Generators. [a, b] = generator() pulls the first two values out of a generator in ES, but not in CS. So I think changing the output is needed.

I can't see any problems with your suggested compilation (other than what I mentioned above).

@GeoffreyBooth
Copy link
Collaborator Author

@lydell Thanks, you’re correct. Updated. I guess this means that no temporary variables are required? That simplifies things.

Not that I don’t want to do this, but it’s not like supported ES destructuring is required. Your example could simply be written as:

a = generator()
b = generator()

Though one could argue that anything Babel transpiles can be written in ES3 in some form or another. But this edge case is easier (and less boilerplatey) than most.

@lydell
Copy link

lydell commented Jan 23, 2017

You mean this:

gen = generator()
a = gen.next().value
b = gen.next().value

Anyhow, your suggested output change seems totally correct, and seems to only bring benefits (full ES compatibility for free, nicer code outout), so I can't see why we would not do this.

@GeoffreyBooth
Copy link
Collaborator Author

It’s not that we wouldn’t do this, it’s more of a question of priority. Like I think we can release CS2-alpha1 without this being ready (but not before jashkenas/coffeescript#4424 I think).

Are there any edge cases I’m not thinking of? The MDN page has some interesting things like default values, skipped values, matches via regex, etc.

@lydell
Copy link

lydell commented Jan 23, 2017

  • Default values work just like parameter defaults. Should be no problem.
  • Skipped values: [first, , third] = [1, 2, 3]. Good point. CoffeeScript currently doesn’t allow this. Not sure what to do. Shouldn’t hold up a PR implementing this change, though: It’s kind of a separate thing.
  • Matches via regex: There is no such thing. MDN only gives an example on how someRegex.exec returns an array and it can be useful to destructure that array ()
  • Priority: I agree, definitely no rush on this one.

@connec
Copy link

connec commented Jan 23, 2017

For @-arguments, we'll need to maintain the current codepath (in 2) that extracts this assignments for derived constructors (unless we drop that). I.e. this:

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

Should continue to compile to:

class B extends A {
  constructor (name) {
    super(...arguments)
    this.name = name
  }
}

Otherwise this could potentially clean up a lot of code!

@lydell
Copy link

lydell commented Jan 23, 2017

@connec Do you mean this?

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

class B extends A {
  constructor(arg) {
    super(...arguments)
    ({name: this.name} = arg);
  }
}

@connec
Copy link

connec commented Jan 23, 2017

Whoops, @lydell, I do indeed 😅

@edemaine
Copy link

Skipped values in array destructuring seems like a cool feature. I think ideally that could be added to CS1 and CS2. Perhaps worth forking off into a separate issue, at least for CS1.

@JavascriptIsMagic
Copy link

If I need skipped values I usually just assign to a veriable I won't use:

[a, _, _, b] = [1, 2, 3, 4]

Also there is another edge case from coffeescript that I do not believe ES6 covers?

[a, ..., b] = [1, 2, 3, 4]

I don't think the ... even makes sense in the context of iterables like generators:

[a, ..., b, c, d, e] = generator()

If the generator only emits 4 values or infinite values, what does the above snippet mean?

CS1 is assuming we are working with an [array-like], but if I am not mistaken, Javascript has been moving towards favoring iterable support in places CS1 previously could assumed where [array-like] things such as for.

@edemaine
Copy link

@JavascriptIsMagic _ is a fine idiom until you're using Underscore. So I think skipping values would still be useful, though minor.

Agreed, those ... examples seem impossible/meaningless with iterators. I think those examples should compile as-is (meaning they assume array-like), while destructuring without ... could be compiled to ES6 which would work with arrays or iterators.

@GeoffreyBooth
Copy link
Collaborator Author

@jashkenas or @lydell am I correct in understanding that the current destructuring implementation is in Assign::compilePatternMatch? This is a 107-line-long function . . . care to explain what’s going on here? Any general advice regarding how to refactor/replace this to output ES destructuring?

I have a hunch that this will turn into something similar to updating function parameters, where we can output ES syntax in some cases but need to fall back to the 1.x implementation for cases like expansions that aren’t compatible with ES. So maybe this function can shrink a little with outputting destructuring as ES, but big chunks of complexity will likely remain.

@lydell
Copy link

lydell commented Feb 5, 2017

Yes, that's the place. I added some quick comments to it for you: lydell/coffee-script@590cd3f

  • [a, ..., b] = c: I guess we could compile them to splats? var [a, ...hiddenVariable, b] = c.
  • {@a} = b: I guess we can use var {a: this.a} = b?
  • [splat..., b] = c: Not sure.

Also, it might be worth thinking about if we want to start supporting {a, b...} = c.

@connec
Copy link

connec commented Feb 6, 2017

Does this mean you're working on destructuring @GeoffreyBooth?

@GeoffreyBooth
Copy link
Collaborator Author

I’ve started a branch, but not really done any work yet: https://github.com/GeoffreyBooth/coffeescript/tree/destructuring

Feel free to work on this branch if you’d like. I’m not going to hold up CS2-alpha1 for this feature, though.

@GeoffreyBooth
Copy link
Collaborator Author

I’ve opened a pull request for discussion of that branch: jashkenas/coffeescript#4478

@coffeescriptbot coffeescriptbot changed the title Destructuring CS2 Discussion: Output: Destructuring Feb 19, 2018
@coffeescriptbot
Copy link
Collaborator

Migrated to jashkenas/coffeescript#4962

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants