101

In my class, eslint is complaining "Expected 'this' to be used by class method 'getUrlParams'

Here is my class:

class PostSearch extends React.Component {
  constructor(props) {
    super(props);
    this.getSearchResults();
  }

  getUrlParams(queryString) {
    const hashes = queryString.slice(queryString.indexOf('?') + 1).split('&');
    const params = {};

    hashes.forEach((hash) => {
      const [key, val] = hash.split('=');
      params[key] = decodeURIComponent(val);
    });

    return params;
  }

  getSearchResults() {
    const { terms, category } = this.getUrlParams(this.props.location.search);
    this.props.dispatch(Actions.fetchPostsSearchResults(terms, category));
  }

  render() {
    return (
      <div>
        <HorizontalLine />
        <div className="container">
          <Col md={9} xs={12}>
            <h1 className="aboutHeader">Test</h1>
          </Col>
          <Col md={3} xs={12}>
            <SideBar />
          </Col>
        </div>
      </div>
    );
  }
}

What is the best approach to solve this or refactor this component?

Hinesh Patel
  • 1,161
  • 2
  • 7
  • 15
  • Where does the error point to? – Andrew Li May 29 '17 at 19:44
  • 3
    That sounds like a linting error. If you don't want that error, you can always disable the linting rule. You are getting the error because `getUrlParams` could just be a helper function in the module scope though. – loganfsmyth May 29 '17 at 19:46
  • 1
    It is essentially an helper function, what's the best way to refactor and avoid this error? – Hinesh Patel May 29 '17 at 19:49
  • 2
    Extract it to the outside of `PostSearch` class. You have to ask yourself question – „Should component be responsible for URL parsing?” If not – extract it outside according to **Single Response Principle** rule – one of the major rule that helps making decisions about OO. – Krzysztof Safjanowski May 29 '17 at 20:09

8 Answers8

72

you should bind the function to this as the ESLint error says "Expected 'this' to be used by class method 'getUrlParams'

getUrlParams = (queryString) => { .... }

as you are not using getUrlParams during render (like onClick()) so the above technique is good which we can call it "usage of arrow function in class property".

there are other ways of binding too:

  • binding in constructor this.getUrlParams=this.getUrlParams.bind(this)
  • arrow function in render e.g.onClick={()=>this.getUrlParams()} assumed that function does not have params.
  • and React.createClass which with ES6 does not make sense :)
Amir-Mousavi
  • 4,273
  • 12
  • 70
  • 123
29

This is a ESlint rule, see class-methods-use-this.

You could extract the method getUrlParams and put it into a helper, or to make it static.

What could you also do is to move the this.props.location.search inside the method, therefore calling the this.getUrlParams() method without parameter, as it seems you are using it only once.

Therefore, this could look like:

getUrlParams() {
    const queryString = this.props.location.search;
    ...
    return params;
  }

A last option would be to disable this ESlint rule.

Ioan
  • 5,152
  • 3
  • 31
  • 50
  • 1
    That got rid of the error, thanks for your help. Do you know why these errors occur, just for my reference? – Hinesh Patel May 29 '17 at 19:57
  • I've updated my answer; it's a ESlint rule, see http://eslint.org/docs/rules/class-methods-use-this. – Ioan May 29 '17 at 20:00
  • 1
    Do we have any permanent fix when you can not avoid parameters? – Balram Singh Oct 27 '17 at 11:17
  • 2
    But, does anyone know what that point of that eslint rule is? Why should we reference 'this' in a class method? I looked at the eslint doc for this rule, and it never explains why it's bad to *not* use 'this' in a class method. – mojave Sep 07 '18 at 16:58
  • 6
    @mojave Despite the rule's name, its point is not that you should reference `this` from a class method, but that any method that doesn't use `this` should be converted to a static method, to make it clear that the method doesn't do anything instance-specific. There's also the sometimes-useful fact that a class's static methods can be used without making an instance of the class. – Aaron Sep 18 '18 at 21:05
10
getUrlParams = queryString => { ... }
Luca Kiebel
  • 9,790
  • 7
  • 29
  • 44
rfdc
  • 1,665
  • 1
  • 10
  • 15
  • 14
    Thank you for this code snippet, which might provide some limited, immediate help. A [proper explanation would greatly improve its long-term value](//meta.stackexchange.com/q/114762/206345) by showing _why_ this is a good solution to the problem, and would make it more useful to future readers with other, similar questions. Please [edit] your answer to add some explanation, including the assumptions you've made. – Blue Aug 20 '18 at 12:30
5

My solution is to use this function outside of class and bind this function to class.

function getUrlParams(queryString) {
 const hashes = queryString.slice(queryString.indexOf('?') + 1).split('&');
 const params = {};
 hashes.forEach((hash) => {
  const [key, val] = hash.split('=');
  params[key] = decodeURIComponent(val);
 });
 return params;
}
class PostSearch extends React.Component {
  constructor(props) {
    super(props);
    this.getSearchResults();
    this.getUrlParams = getUrlParams.bind(this); // add this
  }

  getSearchResults() {
   const { terms, category } = this.getUrlParams(this.props.location.search);
   this.props.dispatch(Actions.fetchPostsSearchResults(terms, category));
  }
  render() {
   return (  "bla bla"  );
  }
}
Arjun G
  • 2,194
  • 1
  • 18
  • 19
4

Another use case could be.

Let's say you have method called handlePasswordKeyUp. The body of function can be seen like this.

handlePasswordKeyUp() {
  console.log('yes')
}

Above code will trigger that error. So at least use this inside the body function

handlePasswordKeyUp(){
   this.setState({someState: someValue})
}
Faris Rayhan
  • 4,500
  • 1
  • 22
  • 19
2

The current eslint documentation for this linter says:

This rule is aimed to flag class methods that do not use this.

And:

If a class method does not use this, it can sometimes be made into a static function.

So, applying their example to your code we'll have:

class PostSearch extends React.Component {
  constructor(props) {
    super(props);
    this.getSearchResults();
  }

  static getUrlParams(queryString) {
    const hashes = queryString.slice(queryString.indexOf('?') + 1).split('&');
    const params = {};

    hashes.forEach((hash) => {
      const [key, val] = hash.split('=');
      params[key] = decodeURIComponent(val);
    });

    return params;
  }

  getSearchResults() {
    const { terms, category } = PostSearch.getUrlParams(this.props.location.search);
    this.props.dispatch(Actions.fetchPostsSearchResults(terms, category));
  }

  render() {
    return (
      <div>
        <HorizontalLine />
        <div className="container">
          <Col md={9} xs={12}>
            <h1 className="aboutHeader">Test</h1>
          </Col>
          <Col md={3} xs={12}>
            <SideBar />
          </Col>
        </div>
      </div>
    );
  }
}

Notice the static modifier to the function and the class name before invoking the function (instead of this).

Rael Gugelmin Cunha
  • 3,327
  • 30
  • 25
0

One, who is facing the same issue, can do this and fix it. Start the function with /* eslint-disable class-methods-use-this */ comment and close with /* eslint-enable class-methods-use-this */. Like this:-

    /* eslint-disable class-methods-use-this */
    getUrlParams(queryString) {
      // some codes here
    }
    /* eslint-enable class-methods-use-this */
-9

A possible hack to this rule could be.

getMoviesByID(){
  //Add a reference to this in your function.
  this.funcName = 'getMoviesByID';
}
Shubhendu Vaid
  • 567
  • 3
  • 17
  • 8
    Having this as the accepted answer gives the wrong impression. Technically it answers the question, but in a really bad way by adding an unnecessary class var. – Christopher Moore Aug 24 '18 at 16:26
  • @ChristopherMoore exactly, it did not solved the question but just provided answer – Humoyun Ahmad Sep 18 '18 at 04:13
  • Bad Solution. Just to get rid of lint error warning isn't the right solution. If the behavior is not desired, the rule of that specified lint should be handled in eslint configuration file. – yuva Oct 04 '18 at 05:34
  • Do not recommend this and a better way to handle this is https://stackoverflow.com/a/49061102/555362 or https://stackoverflow.com/a/51814498/555362 – CuriousGuy Oct 19 '18 at 02:31