Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

Destructuring with get or set causes "Unexpected token" error. #1293

Closed
rwjblue opened this issue Apr 22, 2015 · 14 comments
Closed

Destructuring with get or set causes "Unexpected token" error. #1293

rwjblue opened this issue Apr 22, 2015 · 14 comments

Comments

@rwjblue
Copy link
Contributor

rwjblue commented Apr 22, 2015

When using --esnext option: destructuring assignment with the keys get or set result in an "Unexpected token" error.

Example syntax that triggers the error:

// Using `set`
let { set } = Foo;

// Using `get`
let { get } = Bar;

Actual output:

% jscs --esnext get-set-parse-error.js
Unexpected token = at get-set-parse-error.js :
     1 |let { get } = Foo;
---------------------^
     2 |

1 code style error found.
@hzoo
Copy link
Member

hzoo commented Apr 22, 2015

Since it's saying unexpected token, then it's most likely an esprima issue? We'll have to wait on a change there if so.

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 22, 2015

@hzoo - Yeah, makes sense. Should I close and reopen there?

@hzoo
Copy link
Member

hzoo commented Apr 22, 2015

Yeah , I guess just check if they are already working on supporting it or if it's a bug?

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 25, 2015

This seems to parse properly with Esprima 2.2.0. See this online demo for an example.

@hzoo
Copy link
Member

hzoo commented Apr 25, 2015

Ok then maybe the esprima used in jscs for esnext isn't the latest? @markelog

    "esprima": "^1.2.5",
    "esprima-harmony-jscs": "1.1.0-bin",

I think we are planning on updating to 2.2 if we didn't already and eventually making esnext default in 2.0. One option is to use the esprima option - you would need esprima as a dependency and use that instead of the one in jscs.

in package.json

"esprima": "~2.2.0",

in .jscsrc

"esprima": "./node_modules/esprima",

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 25, 2015

@hzoo - Awesome, thank you for the work around!!

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 26, 2015

A little follow-up:

  • A more updated version of estraverse is needed to use with 2.2.0 of Esprima (which is easy to fix).
  • Esprima 2.2.0 (and master) do not support parsing spread arguments yet, but the custom fork that JSCS uses does (must have been manually worked around?).

@qfox
Copy link
Member

qfox commented Apr 26, 2015

@rwjblue jquery/esprima#1099 — There is a roadmap for es6-features. I believe better to make all we need right in esprima core (to not repeat ourselves). 😼

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 26, 2015

@zxqfox - Yes, I completely agree. Unfortunately, we are in a weird position because some features that are needed (and are supported by other parsers) break parsing in both the custom fork that JSCS uses (let { get } = SomeThing;) and the main upstream Esprima version (someFunc(...someArray)) so we are forced to pick our poison (and decide what features are unsupported).

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 26, 2015

Also, is there an issue in JSCS-land for tracking the move to drop the custom fork of esprima-harmony in favor the main upstream version?

@qfox
Copy link
Member

qfox commented Apr 26, 2015

@rwjblue Well, if we should write something — better to write in upstream IMO.

But if it blocking something — it's another case and we should make something with our fork.

Also, is there an issue in JSCS-land for tracking the move

Not sure but we talking about it too often in past weeks.

@mikesherov
Copy link
Contributor

@rwjblue I'll be working to get spread into Esprima this week hopefully.

@rwjblue
Copy link
Contributor Author

rwjblue commented Apr 26, 2015

@mikesherov - That is awesome! Thank you for your continued hard work both here, and in Esprima itself.

@hzoo
Copy link
Member

hzoo commented Jun 10, 2015

Also, is there an issue in JSCS-land for tracking the move

Issues are #1382 (update esprima), #1353 (support babel)

brzpegasus added a commit to DavyJonesLocker/ember-suave that referenced this issue Aug 8, 2015
* Upgrade to `broccoli-jscs@1.0.0` for JSCS 2.0 support
* Fix test example code for `require-space-after-keywords` rule:
  - The new parser requires wrapping the example `return` statement in a
    function.
* Update the `require-object-destructuring` rule:
  - Remove the exception made to `get` and `set` as the
    [issue](jscs-dev/node-jscs#1293) has been
resolved in the Esprima version that JSCS now uses.

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

No branches or pull requests

4 participants