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

react/require-render-return shouldn't warn if render is defined using (stage 1) class property syntax #564

Closed
jimbolla opened this issue Apr 22, 2016 · 9 comments

Comments

@jimbolla
Copy link

react/require-render-return will error if a class is defined with this syntax:

export default class UserList extends React.Component {
  render = () => (
    <div/>
  );
}

We use this syntax to make our class function declarations consistent between render and event handler functions.

http://babeljs.io/docs/plugins/transform-class-properties/

@ljharb
Copy link
Member

ljharb commented Apr 22, 2016

Instance properties are not yet standard - and doing this (instead of render() {) is actually much slower, and I think is something that should be discouraged.

@billyjanitsch
Copy link

billyjanitsch commented Apr 23, 2016

@ljharb why would it be much slower? Doesn't React already bind all standard component methods to the instance in the constructor anyway?

@ljharb
Copy link
Member

ljharb commented Apr 23, 2016

@billyjanitsch only in React.createClass, not in class-based components.

@ljharb
Copy link
Member

ljharb commented May 9, 2016

@yannickcr this change is breaking something in eslint-config-airbnb

Cannot read property 'name' of undefined
TypeError: Cannot read property 'name' of undefined
    at getPropertyName ($PROJECT/node_modules/eslint-plugin-react/lib/rules/require-render-return.js:54:20)
    at Object.ArrowFunctionExpression ($PROJECT/node_modules/eslint-plugin-react/lib/rules/require-render-return.js:93:40)
    at EventEmitter.updatedRuleInstructions.(anonymous function) ($PROJECT/node_modules/eslint-plugin-react/lib/util/Components.js:457:75)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:188:7)
    at NodeEventGenerator.enterNode ($PROJECT/node_modules/eslint/lib/util/node-event-generator.js:40:22)
    at CodePathAnalyzer.enterNode ($PROJECT/node_modules/eslint/lib/code-path-analysis/code-path-analynpm ERR! Test failed.  See above for more details.

@yannickcr
Copy link
Member

@ljharb Ho. Do you know what code makes it crash ?

@vdh
Copy link

vdh commented May 9, 2016

@yannickcr It looks similar to this issue in eslint-plugin-import: import-js/eslint-plugin-import#317

@ljharb
Copy link
Member

ljharb commented May 9, 2016

I'm afraid I don't - it doesn't tell me which.

@ljharb
Copy link
Member

ljharb commented May 9, 2016

fwiw I'm not using any proposed ES syntax.

@yannickcr
Copy link
Member

Ok, the following pattern trigger the crash:

var Hello = () => (
  <div></div>
);

I'll do a quick fix.

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

No branches or pull requests

5 participants