[jsx-no-bind] add option to allow it in stateless components

[silvenon]

For stateless components you don't really have a choice, do you? I see Dan Abramov use arrow functions all the time in stateless components. If Dan Abramov jumps of a cliff, it's ok for us to do it, right? 😁

[ALF-er]

Also, allow ref={c => this.myComponent = c} case please

[eadz]

This would be great. Currently the following is allowed

  <ButtonBlock onPress={function () { onContinue(8); }} text={'I Agree'} color={"black"}/>

but this is not:

  <ButtonBlock onPress={() => onContinue(8) } text={"I Agree"} color={"black"}/>

[silvenon]

@eadz really? I don't understand why the former is allowed 😕

Anyway, I since changed my mind about this issue. I don't see a reason for this level of control, we already have the allowArrowFunctions option.

[silvenon]

@eadz oh, I see now. Well, let's see… Is using an anonymous function directly bad? Does that create a new function on each render?

1. If yes, this should also be an error, bind or no bind.
2. If yes, this rule should only apply to bind, not arrow functions too.

Either way it seems like something needs to change. I don't know how to check whether a new function is created every time if you pass an anonymous function.

[Daniel15]

@eadz - That's an oversight, the function() { } syntax should be caught by the lint rule too. I didn't implement that in the lint rule since I haven't actually seen that style in any live code.

In any case, you can just disable the lint rule if you don't agree with it 😛

[ljharb]

@silvenon yes, creating any function inside render, including an arrow, is bad, because it creates a new one on each render.

Essentially, what's needed is a rule for "do not create a new function in a function that returns jsx".

[eadz]

@ljharb if it is a stateless component
a) won't it only re-render if props have changed?
b) how else do you pass parameters to a function passed as props.

e.g.

const SelectTimeDisplay = ({ selectTime }) => (
  <View>
  <TouchableHighlight onPress={ () => selectTime(12) }>
     <Text>12 Hour Display</Text>
  </TouchableHighlight>
  <TouchableHighlight onPress={ () => selectTime(24) }>
    <Text>24 Hour Display</Text>
  </TouchableHighlight>
  </View>
);

According to the linting rules, you can't use bind either, so what is the suggested solution using stateless components?

@Daniel15 I know I can disable the rule, but I'm struggling to find any solution that passes. ( that function code was one of them ;) ) I will say that I am not a javascript guru, so there may be something I am missing.

[ljharb]

@eadz yes, that is true. and in this case, i'd say that doesn't belong as a stateless component, since it has state - you're just hiding it inside a prop.

[eadz]

@ljharb where is the state? there are no variables to store state...

[ljharb]

@eadz which number you're passing to selectTime. I'd recommend maybe something like:

const SelectTime = ({ selectTime, children }) => (
  <TouchableHighlight onPress={selectTime}>
    {children}
  </TouchableHighlight>
);

class SelectTimeDisplay extends React.Component {
  constructor(props) {
    super(props);
    this.selectTime12 = this.selectTime.bind(this, 12);
    this.selectTime24 = this.selectTime.bind(this, 24);
  }

  render() {
    return (
      <View>
        <SelectTime onPress={this.selectTime12}>
          <Text>12 Hour Display</Text>
        </SelectTime>
        <SelectTime onPress={this.selectTime24}>
          <Text>24 Hour Display</Text>
        </SelectTime>
      </View>
    );
  }
}

[eadz]

@ljharb a number I pass into select time is a function parameter, not state surely? those numbers are not state, they are hard coded in the program.

you could rewrite the component like so

let component = {
  press12: function(selectTimeFunc) { selectTimeFunc(12) },
  press24: function(selectTimeFunc) { selectTimeFunc(24) },
}

// and run it
component.press12(function(x) { alert(x) });
component.press24(function(x) { alert(x) });

There is no state right?

I understand I could rewrite it the way you have suggested, but as functional components are new to react I think maybe not everyone has experienced the limitations of the eslint-plugin-react when it comes to them. Do you use them much?

[ljharb]

Yes, and in these cases we've concluded that we can't use them.

[Daniel15]

Yeah, I've found that this is one of the main limitations of stateless components. If you have a callback that's the function of a prop, it will create a new function on every render, and there's not really a way to avoid that. I still use React.createClass syntax in all but the most basic components.

[eadz]

Ok, so speed is the main reason? I've done some benchmarking, and it's really hard to find any difference ( even rendering 1000 things every second still no difference ) but I do understand there may be a very very small difference. For me it's not a big enough difference to worry about so I'll just disable the rule locally. I think if you were making a game it might matter, but even then, maybe not.

[Daniel15]

Performance is the main reason. Specifically, garbage collection. In a large app, it's good to try and reduce the number of allocations (eg. functions and temporary arrays) as pauses due to garbage collection make the app feel much slower whenever they occur.

[vdh]

Has there been a resolution to this conflict between rules? For components that need event handlers bound to props:

import React, { Component, PropTypes } from 'react';
import pureRender from 'pure-render-decorator';


@pureRender
export default class Example extends Component {
  static propTypes = {
    action: PropTypes.func.isRequired,
    id: PropTypes.number.isRequired,
  };

  handleClick = () => this.props.action(this.props.id);

  render() {
    return <button onClick={this.handleClick} />;
  }
}

Is it right to assume that jsx-no-bind is the more important rule here, and that the prefer-stateless-function error is a false positive?

[ljharb]

@vdh yes.

[idchlife]

@vdh encountered similar problem just now, I started writing stateless function component and then saw no bind and no arrow functions. Soooo... I think this is really the case, when your stateless function component suddenly needs to call an action that will trigger somewhere change of props, that is used in this stateless function component, it is kinda too interactive, alive and... staty. Also rule about stateless function component is not (as I see in my other files) saying there is something wrong with components that have only props but (!) calling some actions in onClick etc.

@silvenon about OP post, I think every developer should decide which way is better but one's developer breaking lint rules is not a case for everyone to jump off a cliff following him. Every developer counts and every developer should decide better way.

In resolution, "prefer stateless function components" if you call actions that changes props - is kinda yes, fallsy and not your case.

[giacomorebonato]

How bad is it to define the function outside of the component?
Ex.

let MyComponent = (props) => (<div onClick={handleClick} />)
let handleClick = (e) => { console.log(e) }

[Daniel15]

@giacomorebonato - No issue with that at all, although my personal choice is to write function handleClick(e) { as it looks more like 'regular' JavaScript and is shorter to write :)

[silvenon]

Also, you have to be careful with this, if you happen to use it.

[ljharb]

@giacomorebonato if the function does not depend on prop, that's absolutely where it should be defined (but above the component, if you follow Airbnb's no-use-before-define rule)

[davegri]

So still not thoughts about this?

[ljharb]

@davegri the thoughts remain the same - is there a specific question you have?

Basically, using functions created in the render path as a prop on a non-DOM component prevents that component from being able to avoid rerendering - that's really the slow part. There's no speed lost by creating an arrow function every render to pass as a callback to a DOM element, for sure.

[davegri]

So if I'm rendering a component like MyButton I should use a class component in order to define the handler? And if it's a Dom element it's fine to create on render?

[Daniel15]

And if it's a Dom element it's fine to create on render?

It'll still cause issues with garbage collection if it's on a hot codepath, as it creates a brand new function on every render call. Smaller apps likely won't encounter those issues though.