What's the right way to do `onClick` in a NextJS component?

Question

Doing my project with NextJS I encounter a part where I made a component called app_buttonGray and it looks like this:

// /components/app_buttonGray.js
export default function AppButtonGray({ size, children }){
    return(
        <button className={`flex w-${size ? size : "36"} mt-2 p-1 rounded-md bg-gray-500 hover:bg-gray-800 shadow-lg justify-center`}>
            {children}
        </button>
        
    )
}

Later in my page I want to create multiple buttons but each of them have different purposes So I want to implement onClick like this:

<AppButtonGray size="48" onClick={ () => alert(1)}>New project</AppButtonGray>
<AppButtonGray size="48" onClick={ () => alert(2)}>Open project</AppButtonGray>

But that doesn't seem to work...

After multiple intents I come up with this modification that made it work:

// /components/app_buttonGray.js
export default function AppButtonGray({ size, onClick, children }){
    return(
        <button onClick={onClick} className={`flex w-${size ? size : "36"} mt-2 p-1 rounded-md bg-gray-500 hover:bg-gray-800 shadow-lg justify-center`}>
            {children}
        </button>
        
    )
}

So I had to pass by parameter the onClick and then call it inside the component...

Is that the right way to make this work? If not then what's the right way? Thanks

FYI your tailwind dynamic width classname will be removed at build because you can't use string concatenation - here is a more detailed explanation and fix I posted a few days ago - https://stackoverflow.com/a/68029107/15304814 — Sean W, Jun 30 '21 at 04:13
@SeanW thank you. I learned a lot. I think in my code this is not the case... for what I read in the links you posted I should `not use string concatenation to create class names`, instead I should` Do dynamically select a complete class name` which is my case — Ricardo, Jun 30 '21 at 13:16
Yes, in your case you could make the "size" the whole tailwind classname instead of just the width number - i.e. lg = "w-36" md = "w-30" etc and in your class className={`${size} flex`}. Lastly, since the size prop is optional you should set your default size in defaultProps to 'w-36'. AppButtonGray.defaultProps = { size: 'w-36' } https://reactjs.org/docs/typechecking-with-proptypes.html#default-prop-values — Sean W, Jun 30 '21 at 19:39

Chris Sandvik · Accepted Answer · 2021-06-30T19:36:33.400

3

Yes this is the right way to accomplish what you're trying to do. In React you always have to pass any custom props down to the elements you are returning if you want them to be applied.

One alternative way to accomplish this however is by using the rest syntax (...) to grab all of the remaining props passed to your component and spreading them onto the child component.

//                                                      Get the remaining props
export default function AppButtonGray({ size, children, ...props }) {
  return (
    <button
      className={`flex w-${
        size || "36"
      } mt-2 p-1 rounded-md bg-gray-500 hover:bg-gray-800 shadow-lg justify-center`}
      {...props}
    >
      {children}
    </button>
  );
}

This is an effective way to pass any props you'd like to your child component but it can be worse for readability when trying to understand the component. This is why some ESLint configs disallow this strategy (you can read more about that here).

Personally I think you should stick to the way you made it in most cases, you'll thank yourself in the long run when trying to understand the code you wrote.

edited Jun 30 '21 at 19:36

answered Jun 30 '21 at 01:29

Chris Sandvik

1,787
9
19

Great! Do you know why the first attempt didn't work? I thought Once the component is already made then I can just use `onClick` – Ricardo Jun 30 '21 at 02:27
1

It didn't work because you were not forwarding the `onClick` prop to the actual ` – Chris Sandvik Jun 30 '21 at 19:25
Nope, the rest syntax can be used for object and array destructuring or function parameters. In this case, the "rest syntax" part of this example makes use of the destructuring type of the rest parameter where as spread is used to forward the props on to the child component. Check the proposal if you don't believe me: https://github.com/tc39/proposal-object-rest-spread/blob/master/Rest.md#object-rest-destructuring or check the MDN docs on destructuring assignment https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment – Chris Sandvik Jun 30 '21 at 20:03
1

It's a weird scenario since it's a function parameter but in reality, what's happening is we're destructuring the one component parameter `props` (which is an object) into the various prop names passed. The term "spread" is always used when you're passing values into something, like an object, array, or function, and the term "rest" is used whenever you're getting values out of something, also an object, array, or list of function parameters. – Chris Sandvik Jun 30 '21 at 20:12
Learned something today - thanks. I deleted the comment to prevent confusion. I've always used defined the remaining variables as '...rest' for destructing but just always assumed it was part of the spread standard. – Sean W Jul 06 '21 at 00:50

What's the right way to do `onClick` in a NextJS component?

1 Answers1

Linked