jscs-dev / node-jscs

:arrow_heading_up: JavaScript Code Style checker (unmaintained)
https://jscs-dev.github.io
MIT License
4.96k stars 510 forks source link

JSCS with fat arrow functions and airbnb style and JSX, ES6? #1237

Closed justin808 closed 9 years ago

justin808 commented 9 years ago

Has anybody gotten jscs to work with with fat arrow functions and airbnb? I get this message:

Missing newline after closing curly brace at ./assets/javascripts/CommentBox.jsx :
    44 |      var newComments = React.addons.update(oldComments, {$push: [newComment]});
    45 |      this.setState({ajaxSending: false, data: newComments, formData: this.emptyFormData});
    46 |    }, (xhr, status, err) => {
-------------^
    47 |      this.logError(xhr, status, err);
    48 |      this.setState({ajaxSending: false});

The problem is that jscs does not understand that the braces are within a function call.

Here's the full code for that method:

  handleCommentSubmit: function() {
    // `setState` accepts a callback. To avoid (improbable) race condition,
    // `we'll send the ajax request right after we optimistically set the new
    // `state.
    var comment = this.state.formData;
    this.setState({ajaxSending: true});
    $.ajax({
      url: this.props.url,
      dataType: 'json',
      type: 'POST',
      data: {comment: comment}
    }).then(data => { // Server returns all data
      const newComment = data;
      const oldComments = this.state.data;
      var newComments = React.addons.update(oldComments, {$push: [newComment]});
      this.setState({ajaxSending: false, data: newComments, formData: this.emptyFormData});
    }, (xhr, status, err) => {
      this.logError(xhr, status, err);
      this.setState({ajaxSending: false});
    });
  },

My jscs file is:

{
  "preset": "airbnb",
  "fileExtensions": [".js", ".jsx"],
  "excludeFiles": ["build/**", "node_modules/**"],
  "esprima": "esprima-fb"
}

I checked in the run here: https://github.com/justin808/react-webpack-rails-tutorial/pull/26

justin808 commented 9 years ago

To run the example, sync up the branch integrate-jscs and run:

(cd client && npm run jscs .)

I get 2 failures:

Missing newline after closing curly brace at ./assets/javascripts/CommentBox.jsx :
    44 |      var newComments = React.addons.update(oldComments, {$push: [newComment]});
    45 |      this.setState({ajaxSending: false, data: newComments, formData: this.emptyFormData});
    46 |    }, (xhr, status, err) => {
-------------^
    47 |      this.logError(xhr, status, err);
    48 |      this.setState({ajaxSending: false});

Missing newline after closing curly brace at ./assets/javascripts/CommentList.jsx :
    19 |        <Comment author={comment.author} key={index} text={comment.text}/>
    20 |      );
    21 |    });
-------------^
    22 |    return (
    23 |      <div className='commentList'>

Basically, the problem is fat arrow functions as method params. The last brace in the code below is not passing.

    var commentNodes = reversedData.map((comment, index) => {

      // `key` is a React-specific concept and is not mandatory for the
      // purpose of this tutorial. if you're curious, see more here:
      // http://facebook.github.io/react/docs/multiple-components.html#dynamic-children
      return (
        <Comment author={comment.author} key={index} text={comment.text}/>
      );
    });
qfox commented 9 years ago

I'm not sure but looks like the problem can be in esprima-fb (or esprima itself).

mikesherov commented 9 years ago

I'm pretty sure the issue is us and not Esprima. I'll be looking into this shortly.

hzoo commented 9 years ago

What rule is this for? requireNewlineBeforeBlockStatements or something else ? @justin808 if you run jscs with jscs --verbose the error output will give you the rule name.

justin808 commented 9 years ago

@hzoo Here you go:

➜  ~/j/react/react-webpack-rails-tutorial/client (rubocop-integration u=) npm run jscs --verbose .                                                                                                                                                                  [0:00:54]
npm info it worked if it ends with ok
npm verb cli [ 'node', '/usr/local/bin/npm', 'run', 'jscs', '--verbose', '.' ]
npm info using npm@2.7.5
npm info using node@v0.10.33
npm verb run-script [ 'prejscs', 'jscs', 'postjscs' ]
npm info prejscs react-webpack-rails-tutorial@1.1.0
npm info jscs react-webpack-rails-tutorial@1.1.0

> react-webpack-rails-tutorial@1.1.0 jscs /Users/justin/j/react/react-webpack-rails-tutorial/client
> jscs .

Missing newline after closing curly brace at ./assets/javascripts/CommentBox.jsx :
    44 |      var newComments = React.addons.update(oldComments, {$push: [newComment]});
    45 |      this.setState({ajaxSending: false, data: newComments, formData: this.emptyFormData});
    46 |    }, (xhr, status, err) => {
-------------^
    47 |      this.logError(xhr, status, err);
    48 |      this.setState({ajaxSending: false});

Missing newline after closing curly brace at ./assets/javascripts/CommentList.jsx :
    19 |        <Comment author={comment.author} key={index} text={comment.text}/>
    20 |      );
    21 |    });
-------------^
    22 |    return (
    23 |      <div className='commentList'>

2 code style errors found.
mrjoelkemp commented 9 years ago

Hmm, just me or does it seem like the verbose flag wasn't carried through with npm run? On Apr 6, 2015 6:02 AM, "Justin Gordon" notifications@github.com wrote:

@hzoo https://github.com/hzoo Here you go:

➜ ~/j/react/react-webpack-rails-tutorial/client (rubocop-integration u=) npm run jscs --verbose . [0:00:54] npm info it worked if it ends with ok npm verb cli [ 'node', '/usr/local/bin/npm', 'run', 'jscs', '--verbose', '.' ] npm info using npm@2.7.5 npm info using node@v0.10.33 npm verb run-script [ 'prejscs', 'jscs', 'postjscs' ] npm info prejscs react-webpack-rails-tutorial@1.1.0 npm info jscs react-webpack-rails-tutorial@1.1.0

react-webpack-rails-tutorial@1.1.0 jscs /Users/justin/j/react/react-webpack-rails-tutorial/client jscs .

Missing newline after closing curly brace at ./assets/javascripts/CommentBox.jsx : 44 | var newComments = React.addons.update(oldComments, {$push: [newComment]}); 45 | this.setState({ajaxSending: false, data: newComments, formData: this.emptyFormData}); 46 | }, (xhr, status, err) => { -------------^ 47 | this.logError(xhr, status, err); 48 | this.setState({ajaxSending: false});

Missing newline after closing curly brace at ./assets/javascripts/CommentList.jsx : 19 | 20 | ); 21 | }); -------------^ 22 | return ( 23 |

2 code style errors found.

— Reply to this email directly or view it on GitHub https://github.com/jscs-dev/node-jscs/issues/1237#issuecomment-89998206.

hzoo commented 9 years ago

@justin808 I think you need to add the --verbose to the jscs script in package.json because it looks like you are running npm run jscs --verbose instead of npm run jscs which calls jscs --verbose. Because when you look at the output it says > jscs .

justin808 commented 9 years ago

@hzoo Thanks. Is there a way to run jscs without the npm wrapper?

> react-webpack-rails-tutorial@1.1.0 jscs /Users/justin/j/react/react-webpack-rails-tutorial/client
> jscs --verbose .

requirePaddingNewLinesAfterBlocks: Missing newline after closing curly brace at ./assets/javascripts/CommentBox.jsx :
    44 |      var newComments = React.addons.update(oldComments, {$push: [newComment]});
    45 |      this.setState({ajaxSending: false, data: newComments, formData: this.emptyFormData});
    46 |    }, (xhr, status, err) => {
-------------^
    47 |      this.logError(xhr, status, err);
    48 |      this.setState({ajaxSending: false});

requirePaddingNewLinesAfterBlocks: Missing newline after closing curly brace at ./assets/javascripts/CommentList.jsx :
    19 |        <Comment author={comment.author} key={index} text={comment.text}/>
    20 |      );
    21 |    });
-------------^
    22 |    return (
    23 |      <div className='commentList'>

2 code style errors found.
hzoo commented 9 years ago

Ok cool so the rule is requirePaddingNewLinesAfterBlocks.

You shouldn't have to use npm run jscs. (I thought npm run meant that you had to have an entry in scripts). Are you not able to do jscs --verbose . from cmd line? If not then you need to do /node_modules/.bin/jscs --verbose . or install jscs globally with npm install jscs -g.

justin808 commented 9 years ago

@hzoo I did the global install. Weird that it doesn't work. I'd rather just add the script to package.json than typing the full path. I'm using nvm and I can run eslint from the command line just fine.

hzoo commented 9 years ago

Ok socd client && npm run jscs . Can you try cd client && jscs --verbose .?

Ok can reproduce with

a(b => {
  c();
}, (d) => {
  e();
});

So it doesn't look like it's necessarily an esnext issue since arrow functions have a BlockStatement inside.