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

full classes compatibility (with static properties) #43

Closed
MoOx opened this Issue Mar 30, 2015 · 35 comments

Comments

@MoOx
Contributor

MoOx commented Mar 30, 2015

I just upgraded to latest version and I am getting issue with classes used as react component.
It seems some rules like displayName or prop-types are not able to read (or just don't look) static fields.

import React, {Component, PropTypes} from "react"

class Widget extends Component {

  static displayName = "Widget"

  static childContextTypes = {
    styleVariables: PropTypes.object,
    RouteHandler: PropTypes.func,
  }

  static propTypes = {
    styleVariables: PropTypes.object,
    RouteHandler: PropTypes.func,
  }

  getChildContext() {
    return {
      styleVariables: this.props.styleVariables,
      RouteHandler: this.props.RouteHandler,
    }
  }
}

This code give me 3 errors:

   6:0   error  Widget component definition is missing display name         react/display-name
  24:22  error  'styleVariables' is missing in props validation for Widget  react/prop-types
  25:20  error  'RouteHandler' is missing in props validation for Widget    react/prop-types

It would be really cool to support this.

@yannickcr

This comment has been minimized.

Owner

yannickcr commented Mar 30, 2015

ESLint do not seems to support static fields either.

Are you using it with another parser (like babel-eslint) ?

@MoOx

This comment has been minimized.

Contributor

MoOx commented Mar 31, 2015

Indeed I am :)

@yannickcr

This comment has been minimized.

Owner

yannickcr commented Mar 31, 2015

Ok :) I do not test with the babel-eslint parser for now, but I think I need to support it (since a lot of React developers are using it).

Will fix this.

@yannickcr yannickcr added rule accepted and removed question labels Mar 31, 2015

@MoOx

This comment has been minimized.

Contributor

MoOx commented Mar 31, 2015

Cool. Ping me if you need a hand on this.

@yannickcr

This comment has been minimized.

Owner

yannickcr commented Apr 5, 2015

After investigation it seems babel-eslint does not return AST for static properties, so I can't validate display-name and prop-types in theses cases.

Class properties are an ES7 experimental feature so it does not surprise me it's not quite finished yet.

I think we'll have to wait for babel-eslint to support Babel 5.0 to fix this.

However, I have fixed this for static methods #48 and it should land in next release.

@sebastienbarre

This comment has been minimized.

sebastienbarre commented Apr 14, 2015

@yannickcr just a quick heads up, babel-eslint just released 3.0, that might help.

@yannickcr

This comment has been minimized.

Owner

yannickcr commented Apr 14, 2015

@sebastienbarre Great! I'll have a look.

@yannickcr

This comment has been minimized.

Owner

yannickcr commented Apr 14, 2015

I confirm that babel-eslint 3.0.1 is now returning the AST for the class static properties, so I will work on it :)

@MoOx

This comment has been minimized.

Contributor

MoOx commented Apr 15, 2015

\o/

@yannickcr yannickcr closed this in 6006e24 Apr 16, 2015

@MoOx

This comment has been minimized.

Contributor

MoOx commented Apr 16, 2015

\o/

@AlexKVal

This comment has been minimized.

Contributor

AlexKVal commented Apr 16, 2015

👍

@sebastienbarre

This comment has been minimized.

sebastienbarre commented Apr 17, 2015

Hmmm, I'm not sure this is working.

class Card extends PureRender {
  static displayName = 'Card';
  render() {
[...]

is triggering a Lint error Unexpected Token =
For kicks, I tried:

class Card extends PureRender {
  static displayName: 'Card';
  render() {
[...]

and this does not trigger an error. However that's not the ES7 Property Initializers syntax, the former is (with an = sign, see here and here). Am I correct?

@MoOx

This comment has been minimized.

Contributor

MoOx commented Apr 17, 2015

You don't need a semicolon.

class Card extends PureRender {
  static displayName = 'Card'
  render() {
  }
}
@sebastienbarre

This comment has been minimized.

sebastienbarre commented Apr 17, 2015

@MoOx yup, but that's not the issue here, it will complain about the = token with or without a semi.

@yannickcr

This comment has been minimized.

Owner

yannickcr commented Apr 17, 2015

@sebastienbarre I cannot reproduce your problem with eslint@0.19.0, eslint-plugin-react@2.1.1 and babel-eslint@3.0.1:

test.jsx

class Hello extends React.Component {
  static displayName = 'Widget';
  render() {
    return <div>Hello {this.props.name}</div>;
  }
}

.eslintrc

{
    "parser": "babel-eslint",
    "ecmaFeatures": {
        "classes": true,
        "jsx": true
    },
    "plugins": [
      "react"
    ],
    "rules": {
      "react/display-name": 1
    }
}
$ eslint --reset test.jsx # Does not find any error
@sebastienbarre

This comment has been minimized.

sebastienbarre commented Apr 17, 2015

@yannickcr oh, I didn't have ecmaFeatures.classes: true in my .eslintrc, sorry and many thanks for pointing that out.

@sebastienbarre

This comment has been minimized.

sebastienbarre commented Apr 17, 2015

@yannickcr Ha no, I take that back, that wasn't ecmaFeatures.classes. No problem on your side, it was JSCS that was complaining (a pretty useful tool nonetheless). It uses esprima-fb internally for JSX/ES6, which unfortunately doesn't support class properties (facebook/esprima#63) and is being deprecated (facebook/esprima#111). I'll investigate, maybe JSCS will use babel at some point (jscs-dev/node-jscs#1270). Thanks.

@sunjay

This comment has been minimized.

sunjay commented May 30, 2015

Was this ever fixed? I want to use static properties too.

@yannickcr

This comment has been minimized.

Owner

yannickcr commented May 30, 2015

@sunjay Yes, this is fixed.

But you still need to use the babel-eslint parser since espree (the default ESLint parser) does not support static properties.

@MoOx

This comment has been minimized.

Contributor

MoOx commented May 31, 2015

Note that using displayName with es6 classes is useless since React can now grab the name from the constructor :)

@unbalancedparentheses

This comment has been minimized.

unbalancedparentheses commented Jun 4, 2015

@MoOx we don't even have to set it on the constructor?

@jorge-pascual

This comment has been minimized.

jorge-pascual commented Jun 18, 2015

Hi!

Same problem here... but the proposed solution appears it's not working...

home.js

class Home extends React.Component {
    static contextTypes = {
        executeAction: React.PropTypes.func.isRequired
    }
    render() {
        return (
            <div>
                <h2>Home</h2>
                <p>Welcome to login!</p>
           </div>
      );
    }
}

.eslintrc

{
    "parser": "babel-eslint",
    "ecmaFeatures": {
        "classes": true,
        "jsx": true
    },
    "plugins": [
      "react"
    ],
    "rules": {
        "react/display-name": 1,
        "react/jsx-boolean-value": 1,
        "react/jsx-no-undef": 1,
        "react/jsx-quotes": 1,
        "react/jsx-sort-prop-types": 1,
        "react/jsx-sort-props": 1,
        "react/jsx-uses-react": 1,
        "react/jsx-uses-vars": 1,
        "react/no-did-mount-set-state": 1,
        "react/no-did-update-set-state": 1,
        "react/no-multi-comp": 1,
        "react/no-unknown-property": 1,
        "react/prop-types": 1,
        "react/react-in-jsx-scope": 1,
        "react/self-closing-comp": 1,
        "react/sort-comp": 1,
        "react/wrap-multilines": 1,
        "strict": 0,
        "indent": [2, 4],
        "quotes": [2, 'single'],
        "no-unused-vars": 0 // see https://github.com/babel/babel-eslint/issues/21
    },
    "env" : {
        "node": true
    }
}

output

/myapp/apps/core/services
/myapp/node_modules/babel/node_modules/babel-core/lib/babel/transformation/file/index.js:601
      throw err;
            ^
SyntaxError: /myapp/apps/profit/features/login/components/Home.js: Unexpected token (7:21)
   5 | class Home extends React.Component {
   6 |
>  7 |  static contextTypes = {
     |                      ^
   8 |      executeAction: React.PropTypes.func.isRequired
   9 |     }
  10 |
    at Parser.pp.raise (/myapp/node_modules/babel/node_modules/babel-core/lib/acorn/src/location.js:73:13)

package.json

...
 "devDependencies": {
    "babel-eslint": "^3.0.1",
    "babel-loader": "^5.1.3",
    "bundle-loader": "~0.5.0",
    "del": "^1.1.1",
    "eslint": "^0.21.2",
    "eslint-plugin-react": "^2.5.2",
    "gulp": "^3.8.11",
    "gulp-nodemon": "^2.0.2",
    "gulp-util": "^3.0.4",
    "gulp-webpack-build": "^0.11.0",
    "json-loader": "~0.5.1",
    "nodemon": "^1.2.1",
    "webpack": "^1.4.12",
    "webpack-dev-server": "^1.6.5"
  },
...

Any suggestion?

@mathieumg

This comment has been minimized.

Contributor

mathieumg commented Jun 18, 2015

Did you enable the classProperties stage 0 transform in your Babel config?

@jorge-pascual

This comment has been minimized.

jorge-pascual commented Jun 18, 2015

@mathieumg not sure... I got classes": true in .eslintrc. Isn't it enough?

    "parser": "babel-eslint",
    "ecmaFeatures": {
        "classes": true,
        "jsx": true
    },
@mathieumg

This comment has been minimized.

Contributor

mathieumg commented Jun 18, 2015

Isn't it enough?

It's not. It's a stage 0 proposal for ES7, classes are a ES6 feature. See https://gist.github.com/jeffmo/054df782c05639da2adb

You need to enable the es7.classProperties transform in Babel (by adding it to the optional property) (http://babeljs.io/docs/usage/experimental/) or all the stage 0+ transforms (by setting the stage property to 0). See http://babeljs.io/docs/usage/options/

@jorge-pascual

This comment has been minimized.

jorge-pascual commented Jun 19, 2015

Thank you @mathieumg . I'm using gulp+nodemon+webpack and I can't find out how to enter the "optional" property.

I got this is webpack.config.js

  module: {
        loaders: [
            { test: /\.css$/, loader: 'style!css' },
            { 
                test: /\.js$/, 
                exclude: /node_modules/, 
                loader: require.resolve('babel-loader'),
                query: {
                  optional: ["es7.decorators", "es7.classProperties"]
                }
            },
            { test: /\.json$/, loader: 'json-loader'}
        ]
    },

and this in .eslintrc

{
    "parser": "babel-eslint",
    "ecmaFeatures": {
        "classes": true,
        "jsx": true
    },
    "plugins": [
      "react"
    ],

    "rules": {
        "react/display-name": 1,
        "react/jsx-boolean-value": 1,
        "react/jsx-no-undef": 1,
        "react/jsx-quotes": 1,
        "react/jsx-sort-prop-types": 1,
        "react/jsx-sort-props": 1,
        "react/jsx-uses-react": 1,
        "react/jsx-uses-vars": 1,
        "react/no-did-mount-set-state": 1,
        "react/no-did-update-set-state": 1,
        "react/no-multi-comp": 1,
        "react/no-unknown-property": 1,
        "react/prop-types": 1,
        "react/react-in-jsx-scope": 1,
        "react/self-closing-comp": 1,
        "react/sort-comp": 1,
        "react/wrap-multilines": 1,
        "strict": 0,
        "indent": [2, 4],
        "quotes": [2, 'single'],
        "no-unused-vars": 0 // see https://github.com/babel/babel-eslint/issues/21
    },
    "env" : {
        "node": true
    }
}

I will try to figure out how to do it!. Thanks!

@MoOx

This comment has been minimized.

Contributor

MoOx commented Jun 19, 2015

FYI, "react/display-name" is useless with native class since react is using the class name from the constructor now.

@jorge-pascual

This comment has been minimized.

jorge-pascual commented Jun 19, 2015

Thanks @MoOx !

@rbholeraj

This comment has been minimized.

rbholeraj commented May 16, 2016

@MoOx, how i deal with static variable and properties now? My code is
class ParsedText extends React.Component {

static displayName = 'ParsedText';

static propTypes = {
...React.Text.propTypes,
parse: React.PropTypes.arrayOf(
React.PropTypes.oneOfType([defaultParseShape, customParseShape]),
),
};

static defaultProps = {
parse: null,
};}

I got error ,
ERROR in ./~/react-native-parsed-text/src/ParsedText.js
Module build failed: SyntaxError: /Users/MyMac/WebWork/MyProj/node_modules/react-native-parsed-text/src/ParsedText.js: Unexpected token (23:21)
21 | class ParsedText extends React.Component {
22 |

23 | static displayName = 'ParsedText';
| ^
24 |
25 | static propTypes = {
26 | ...React.Text.propTypes,

@MoOx

This comment has been minimized.

Contributor

MoOx commented May 16, 2016

You probably need babel-eslint as eslint parser

@jordanpapaleo

This comment has been minimized.

jordanpapaleo commented Oct 18, 2016

I had the same issue and I just got this config to work for my project. I pulled all but the relevant dependencies out of the package.json. Hopefully I did not pull something requried.

.babelrc

{
  "presets": ["es2015", "stage-1", "react"],
  "plugins":["transform-class-properties"]
}

.eslintrc

{
  "extends": ["plugin:react/recommended", "standard"],
  "parserOptions": {
    "ecmaVersion": 7,
    "sourceType": "module",
    "ecmaFeatures": {
      "impliedStrict": true,
      "jsx": true
    }
  },
  "env": {
    "browser": true,
    "es6": true,
    "node": true
  },
  "parser": "babel-eslint",
  "plugins": [
    "react"
  ],
  "globals": {
    "beforeEach": true,
    "describe": true,
    "expect": true,
    "it": true,
    "React": true,
    "xdescribe": true,
    "xit": true
  },
  "rules": {}
}

package.json

{
  "name": "",
  "version": "",
  "description": "this is a front end template using react, redux, webpack, enzyme and jest built by jordan with love",
  "scripts": {
    "test": "./node_modules/.bin/eslint . --color; exit 0"
  },
  "license": "",
  "engines": {
    "node": "6.3.1",
    "npm": "3.10.7"
  },
  "dependencies": {
    "babel-core": "6.14.0",
    "babel-loader": "6.2.5",
    "babel-plugin-transform-class-properties": "6.11.5",
    "babel-polyfill": "6.13.0",
    "babel-preset-es2015": "6.14.0",
    "babel-preset-react": "6.11.1",
    "babel-preset-stage-1": "6.13.0",
    "babel-register": "6.14.0",
    "standard": "8.0.0"
  },
  "devDependencies": {
    "babel-eslint": "7.0.0",
    "eslint-config-standard": "6.0.0",
    "eslint-plugin-react": "6.2.0",
    "eslint": "3.5.0"
  }
}

@MatthewHerbst

This comment has been minimized.

Contributor

MatthewHerbst commented Nov 15, 2016

@jordanpapaleo transform-class-properties is included in stage-2 preset which stage-1 includes, so do you need to specify it independently as well as you have? I got it working just by adding in babel-eslint as my parser since I already use stage-2. None of the parserOptions seem needed for this specific problem.

@jordanpapaleo

This comment has been minimized.

jordanpapaleo commented Nov 15, 2016

Hi @MatthewHerbst so much of FE dev is unfortunately pragmatic and once it works, yeah, thats a win in my book. Ill give your suggestion a go for sure, but for some reason thats where I landed with my research. Thanks! - Jordan

@benmarten

This comment has been minimized.

benmarten commented Jul 12, 2017

As of today, this got rid of the warning for me:

.eslintrc

{
  "extends": "eslint:recommended",
  "parser": "babel-eslint",
  "ecmaFeatures": {
    "classes": true
  },
  ...
}
@nshoes

This comment has been minimized.

nshoes commented Jul 12, 2017

@benmarten Yep, adding "parser": "babel-eslint" to my .eslintrc and installing it solved it for me as well.

tetsuo added a commit to tetsuo/16bitjs that referenced this issue Aug 15, 2017

ESLint using Babel as the parser
Adds support for linting JSX and classes.

See: yannickcr/eslint-plugin-react#43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment