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] Destructuring invalid LHS generates invalid JS #4673

Closed
connec opened this issue Aug 30, 2017 · 5 comments
Closed

[CS2] Destructuring invalid LHS generates invalid JS #4673

connec opened this issue Aug 30, 2017 · 5 comments
Labels

Comments

@connec
Copy link
Collaborator

connec commented Aug 30, 2017

See http://coffeescript.org/v2/#try:%7B%20%7Bb...%7D...%20%7D%20%3D%20c

{ {b...}... } = c

Compiles to:

({} = c);
Object.assign({}, b) = objectWithoutKeys(c, []);

I suspect we're missing something when verifying that an object is a valid LHS. Specifically, when a splat is present it's name must be a valid LHS.

@GeoffreyBooth GeoffreyBooth added this to the 2.0.0 milestone Aug 30, 2017
@GeoffreyBooth
Copy link
Collaborator

@zdenko?

@connec
Copy link
Collaborator Author

connec commented Aug 31, 2017

I ended up fixing this whilst playing with an alternative fix for #4651 - 2...connec:fix-4651-alt

It will need rebased once #4652 is resolved.

Whoops, never mind - I was looking at a different example.

{ a: {b...} = {c...} } = z

@GeoffreyBooth
Copy link
Collaborator

@connec It’s actually a little worse than your initial comment. Look at the var declarations line:

var Object.assign({}, b), b,
  objectWithoutKeys = function(o, ks) { var res = {}; for (var k in o) ([].indexOf.call(ks, k) < 0 && {}.hasOwnProperty.call(o, k)) && (res[k] = o[k]); return res; };

({} = c);
Object.assign({}, b) = objectWithoutKeys(c, []);

var Object.assign({}, b), b?

@GeoffreyBooth GeoffreyBooth removed this from the 2.0.0 milestone Sep 1, 2017
helixbass added a commit to helixbass/copheescript that referenced this issue Sep 3, 2017
@helixbass
Copy link
Collaborator

Should be fixed by #4683. See its description, I think @connec's example here actually should compile, as eg ({...{...b}} = c) seems to be valid ES6 (works in Chrome console)

GeoffreyBooth pushed a commit that referenced this issue Sep 7, 2017
* destructuring fixes [Fixes #4673] [Fixes #4657]

* test for destructured @prop

* Add another test to cover #4657 cases

* don't declare actual params
@GeoffreyBooth
Copy link
Collaborator

Fixed by #4683.

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

No branches or pull requests

3 participants