Is it okay make event handling functions return a function?

Question

Hello I am new to react and I was wondering if I could return a function in a event handler instead of passing to the element a arrow function with parameter? Like this:

deletePersonHandler(person) {
    return () => {
        const persons = [...this.state.persons];
        const index = persons.indexOf(person);
        persons.splice(index, 1)
        this.setState({ persons });
    }
}

and on component render:

function Person(props) {
    function Children() {
        if (props.person.children) {
            return <p>{props.person.children}</p>
        }
        else return null;
    }

    return (
        <div className="person">
            <p>Hello I am {props.person.name}, and I am {props.person.age} years old.</p>
            <input value={props.person.name} type="text" onChange={(event) => { props.change(event, props.person.id) }} />
            
            {/* Over Here!! */}
            <button onClick={props.onDelete(props.person)}>Remove</button>

            {Children()}
        </div>
    )
}

I tested the code and it runs fine but I'm asking if there are any setbacks or consequences? and if so what are the alternatives? Here is the full code's repository on my Github's account: https://github.com/EtshD1/react-components

Yes, you can do that, but it's still equivalent to defining the arrow function inside the `render` method, [which has drawbacks in general](https://stackoverflow.com/q/36677733/1048572). — Bergi, Jul 19 '20 at 21:25

score 2 · Accepted Answer · answered Jul 19 '20 at 21:32

Technically yes. Sure you're creating 1 extra unnecessary function, but JS is super efficient and you'll never hit an upper limit on number of functions able to be created.

However, by executing a function in the onClick prop, you're making your code a lot less readable. Developers are used to seeing a function definition for that prop, and by calling a function, the first thing I think is "that should be a definition, not execution", so my inclination would be to think there is a bug with your code, when there isn't.

You've also named the function onDelete, as in "this function gets called on delete", but it doesn't it gets executed immediately and the returned function gets called on the click event.

Part of programming is the ability to not just write working code, but write readable code. Your code is going to be read by other developers, and the more obscure and unnecessary code you write, the harder you're making it for the people reading it.

So in conclusion, is it fine to do this? Technically yes, but semantically, no.

Also a new function gets created every render – charlietfl Jul 19 '20 at 21:47 — charlietfl, Jul 19 '20 at 21:47

score 1 · Answer 2 · answered Jul 19 '20 at 21:33

You could set the name of the button as the id of the person you want to delete that way you can get the id from the event.target.name and avoid return an anonymous function. it would look like this:

deletePersonHandler(event) {
    const personId = Number(event.target.name) //only use Number() if the personId is a numer
    const persons =  this.state.persons.filter(({id}) !== personId)
    this.setState({ persons });
}

And on the component you would use:

function Person(props) {
function Children() {
    if (props.person.children) {
        return <p>{props.person.children}</p>
    }
    else return null;
}

return (
    <div className="person">
        <p>Hello I am {props.person.name}, and I am {props.person.age} years old.</p>
        <input value={props.person.name} type="text" onChange={(event) => { props.change(event, props.person.id) }} />
        
        {/* Over Here!! */}
        <button name={props.person.id} onClick={props.onDelete}>Remove</button>

        {Children()}
    </div>
)}

You should always avoid passing parameters to a function at the render() level as this will cause reconciliation

Is it okay make event handling functions return a function?

2 Answers2