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

Default parameter values are not recognized #1375

Closed
mgol opened this issue May 14, 2015 · 22 comments
Closed

Default parameter values are not recognized #1375

mgol opened this issue May 14, 2015 · 22 comments

Comments

@mgol
Copy link

mgol commented May 14, 2015

With esnext enabled, the following is still rejected by JSCS:

function f({param = false}={}) {}

The message is:

Unexpected token = at app/modules/services/get-raw-text.js :
    1 | function f({param = false}={}) {}
---------------------------^
@hzoo
Copy link
Member

hzoo commented May 14, 2015

If this is because of Unexpected token then it probably means you need to use a different parser. Did you try esprima-fb or the latest esprima@2.2.0?

@difosfor
Copy link

And how do you go about using another parser? Is there an option for that?

Shouldn't this just work when you enable esnext?

@hzoo
Copy link
Member

hzoo commented May 14, 2015

Yeah it's the esprima option in cli or config - should be in the docs: http://jscs.info/overview.html#cli.

jscs ./ --esprima=esprima-fb if installed globally

// .jscsrc
// npm install esprima-fb
{
  "esprima": "./node_modules/esprima-fb"
}

Also I think it doesn't work because esnext turns on jscs' custom esprima branch (https://github.com/jscs-dev/esprima-harmony) which hasn't been updated to the latest yet. This is assuming the latest esprima does support this (and copying into http://esprima.org/demo/parse.html it does).

@difosfor
Copy link

Will this be included in jscs anytime soon? We use jscs in several ways so I'd really rather not have to introduce another dependency besides it. Or are you waiting for another parser to provide better ES6 support?

Also, if I'm not interested in JSX and other React things, but just in ES6+ will using esprima-fb not get in my way?

@difosfor
Copy link

Ah, just read in changelog (http://jscs.info/changelog.html) for v1.13.0:

"We eagerly wait for the Esprima 2.3 release, since soon after, esnext option will be set to true by default."

Hopefully then this will be supported out of the box.

Thanks for your pointers to help us out in the meanwhile hzoo!

@hzoo
Copy link
Member

hzoo commented May 14, 2015

If you aren't using JSX/etc, you can using esprima@2.2.0 as a dependency or wait for our release. I'm also working on https://github.com/jscs-dev/babel-jscs which will use babel as the parser for ES6+, which I'm thinking can just be released when the next jscs is released?

@difosfor
Copy link

Interesting, we're using Babel for the ES Next transpiling, so I guess it would make sense to use the same parser for jscs. Though theoretically at least in the end it should all be standard and not really matter.

@hzoo
Copy link
Member

hzoo commented May 14, 2015

Well the project has to convert acorn -> esprima so there will be a lot more bugs/issues initially, so it depends on if you are using newer features that aren't covered in esprima.

@mrjoelkemp
Copy link
Member

Closing because it seems like using another parser is a workaround in the mean time.

@mgol
Copy link
Author

mgol commented May 22, 2015

When is it planned to switch to a newer Esprima by default?

@hzoo
Copy link
Member

hzoo commented May 22, 2015

Maybe when esprima 2.3 is released - not sure yet?

@mgol
Copy link
Author

mgol commented Jun 2, 2015

I tried both esprima@2.2.0 & esprima-fb@15001.1.0-dev-harmony-fb and both fail on this code.

@hzoo
Copy link
Member

hzoo commented Jun 2, 2015

@mzgol - If you need it now you can use https://github.com/hzoo/babel-jscs. (Not on npm yet and will be when I move to https://github.com/jscs-dev/babel-jscs

{
  "node-jscs": "jscs-dev/node-jscs#master",
  "babel-jscs": "hzoo/babel-jscs#master"
}

@mgol
Copy link
Author

mgol commented Jun 2, 2015

@hzoo thanks for the info

@mgol
Copy link
Author

mgol commented Jun 12, 2015

@hzoo BTW, the Esprima playground uses Esprima 2.2.0 and it handles such things just fine:
http://esprima.org/demo/parse.html?code=%2F%2F%20Life%2C%20Universe%2C%20and%20Everything%0Avar%20answer%20%3D%20%5B%0A%20%20%20%20...a%2C%0A%20%20%20%20'creatorDetails.creatorCore'%2C%0A%20%20%20%20'titleDetails.titleCore'%2C%0A%20%20%20%20...leftColumn%2C%0A%20%20%20%20...rightColumn%2C%0A%5D%3B%0A%0Avar%20b%20%3D%20%7B%0A%20%20%20%20f()%20%7B%7D%0A%7D%3B

and yet JSCS with Esprima 2.2.0 installed and:

{
  "esprima": "esprima",
  "esnext": true
}

recongizes neither spread nor shorthand method definitions. Perhaps JSCS invokes Esprima with options that don't support ES6?

@hzoo
Copy link
Member

hzoo commented Jun 12, 2015

@mzgol It should work - it's working fine for me?
Oh (I am still confused about this), but I'm using "esprima": "./node_modules/esprima",. Also you don't need "esnext": true if you use the esprima option I think.

Also "esprima": "babel-jscs", works now if you npm install.

@mgol
Copy link
Author

mgol commented Jun 15, 2015

@hzoo I still can't get it to work with regular JSCS 1.13.1 & Esprima 2.2.0 with "esprima": "./node_modules/esprima":

$ ./node_modules/grunt-jscs/node_modules/jscs/bin/jscs .
Error: Unknown node type RestElement.
    at Controller.traverse (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/node_modules/estraverse/estraverse.js:517:31)
    at Object.traverse (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/node_modules/estraverse/estraverse.js:709:27)
    at Object.iterate (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/lib/tree-iterator.js:9:20)
    at Object.JsFile.iterate (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/lib/js-file.js:359:29)
    at Object.JsFile._buildNodeIndex (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/lib/js-file.js:752:14)
    at Object.JsFile (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/lib/js-file.js:39:28)
    at StringChecker._createJsFileInstance (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/lib/string-checker.js:202:16)
    at StringChecker.checkString (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/lib/string-checker.js:104:25)
    at null.<anonymous> (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/jscs/lib/checker.js:43:21)
    at Array.<anonymous> (/Users/mgol/Documents/projects/bn/cbn/repo/polona-gui/node_modules/grunt-jscs/node_modules/vow/lib/vow.js:711:39)

This is happening both with "esnext": true and without it.

@hzoo
Copy link
Member

hzoo commented Jun 15, 2015

Ok can reproduce @mzgol it's because it doesn't know about the new node types in the newer esprima (estraverse needs to be updated). So I guess you can't really use the latest esprima at the moment unless we updated that.

In the meantime you can try https://github.com/jscs-dev/babel-jscs and see if that works for you (seems to work for me).

Kind of related to jscs-dev/babel-jscs#3 @mrjoelkemp - if we patch the new types in for estraverse from babel or just update estraverse version.

@mgol
Copy link
Author

mgol commented Jun 16, 2015

@hzoo I'd love to try babel-jscs but I use grunt-jscs and it doesn't seem to be possible to swap the JSCS package used.

@mrjoelkemp
Copy link
Member

You should be able to just set the esprima option in your jscs grunt target
and it will be passed through to jscs.

@hzoo worth adding those Grunt jscs instructions to the readme of
babel-jscs.
On Jun 16, 2015 7:29 AM, "Michał Gołębiowski" notifications@github.com
wrote:

@hzoo https://github.com/hzoo I'd love to try babel-jscs but I use
grunt-jscs https://github.com/jscs-dev/grunt-jscs and it doesn't seem
to be possible to swap the JSCS package used.


Reply to this email directly or view it on GitHub
#1375 (comment)
.

@hzoo
Copy link
Member

hzoo commented Jun 16, 2015

Yeah @mzgol babel-jscs is a esprima parser option (just like esprima)
In the readme it does say

// .jscsrc
{
  "esprima": "esprima", // instead of this
  "esprima": "babel-jscs", // global
  "esprima": "./node_modules/babel-jscs", // local
}

It's the same for grunt right? I see

// Gruntfile.js
jscs: {
    src: "path/to/files/*.js",
    options: {
        config: ".jscsrc"
   }
}

@mgol
Copy link
Author

mgol commented Jun 16, 2015

@hzoo Ah, thanks, I assumed it's JSCS replacement, not Esprima one as technically speaking Esprima 2.2.0 supports the constructs I care about.

I found 2 blocker issues after the switch: one is an already-reported non-working validateQuoteMarks, another one is jscs-dev/babel-jscs#4.

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