Declaring function inside function in React

Question

I just came across a React code and I'm not sure whether it's is a good way to do it or not. This is a sample implementation of that code.

class App extends React.Component {
  renderMessage = () => {
    function getMessage() {
      return "Hello"
    } 
    function getName() {
      return "Vijay"
    }
    return (
      <h1>{getMessage()} {getName()} !!!</h1>
    )
  }
  render() {
    return (
      <div>
        {this.renderMessage()}
      </div>
    )
  }
}

Here we are calling a function renderMessage inside render. In renderMessage there are two inner functions which are called inside renderMessage only. My question now are:-

• Is it a good approach to do? Won't it redeclare method getName and getMessage at every render call.
• If I make getName and getMessage class methods and call them inside renderMessage would it be an improvment?

Thanks :)

Solution 1

Is it a good approach to do? Won't it redeclare method getName and getMessage at every render call

Definitely not a good approach. As JavaScript is either having functional or block or global scope. Whatever you defining at this scope will be part of this scope only.In your case, these function getMessage and getName will be part of renderMessage which is functional scope.

At every call, new functions are getting defined instead reusing previously defined.

If I make getName and getMessage class methods and call them inside renderMessage would it be an improvement?

Depend. If this function need an access to any component properties or method then you should place it inside the component or If this is only utility function then place it inside helper library separate from component. Surely, this will make difference.

Solution 2

It is possible, but it is not a good solution at all.

Yeah, it creates new function on each re-render, but it's not a case if you don't pass them as props.

The problem is that you can get a giant monster component with very ambiguous responsibilities in the nearest future.

Even more. It is bad in terms of performance. The bigger your component is, the bigger time it needs for re-render. And each smallest change will re-render the whole component, not only its part.

You should move renderMessage method to the new component for the sake of readability and scalability.

class App extends React.Component {
  render() {
    return (
      <div>
        <Message/>
      </div>
    )
  }
}

class Message extends React.Component {
    getMessage() {
      return "Hello"
    } 
    
    getName() {
      return "Vijay"
    }
    
    render() {
      return (
        <h1>{this.getMessage()} {this.getName()} !!!</h1>
      )
    }
}

Solution 3

Is it a good approach to do? Won't it redeclare method getName and getMessage at every render call.

It will redecleare functions getName and getMessage at every render call. It's not great, but not terrible. This approach reduces bugs with rerendering - the function declaration is part of the render result. Although in your exact case it doesnt matter, as the functions will always return the same result and it doesnt depend on the state (although in that case it'd be more readable to just inline the strings)

If I make getName and getMessage class methods and call them inside renderMessage would it be an improvment?

It will make _virtual life easier for the garbage collector. The chances are, once you start depending on global state, this will start to go wrong, e.g.:

class App extends React.Component {
  getMessage => () {
      return "Hello"
    } 
  getName => () {
    return this.props.name
  }

  renderMessage = () => {
    someHttpAction().then(() => {
        alert(getMessage() + ' ' + getName());
    })
  }
  render() {
    return (
      <div onclick={this.renderMessage}>Say my name! (Hint, its {this.props.name})</div>
    )
  }
}

(Note that name is passed in as an argument to App)

When you render the first time, you might expect that, after clicking the text, you'll see an alert of "Hello Vijay". That's true, most of the time. But what happens if, after you click the text, you rerender with a different value for name, say Heisenberg, while someHttpAction promise still has not resolved? You might expect to see your name - Vijay, but actually you'll see the new value, "Hello Heisenberg". By declaring the function inline (as per your example), the function's scope is locked and you'll get the expected result "Hello Vijay".

Imagine a bit more complex scenario where you switch between multiple user profiles and the async messages start showing up on the wrong user...

While yes, we could pass name as an argument to getName, in reality, people think "its fine this time" or forget, and this is how bugs get introduced. Much less difficult to make the same mistake with inline functions. Unless this becomes a bottleneck, stick with the less errorprone approach.

Also, I suggest skimming through How Are Function Components Different from Classes?