Update onClick Prop TypeScript Definition to Include Event Argument in BaseButton Interface

[daviareias]

Summary

In the current Polaris version, the TypeScript definition for the onClick prop in the BaseButton interface does not specify that it passes the React event argument to the callback function (onClick?(): unknown;). This is misleading as the actual behavior does include passing the React event. The request is to update the TypeScript definition to accurately reflect the behavior, enhancing developer experience and type safety.

Expected behavior

The onClick TypeScript definition should explicitly include the event argument to inform developers that the callback will receive the React event. This change will align the documentation with the actual behavior and improve type checking and developer expectations when using the Button component.

Actual behavior

The TypeScript definition suggests that the onClick callback does not receive any arguments, which does not match the actual implementation where the React event is passed to the callback. This discrepancy can lead to confusion and improper use of the onClick prop in TypeScript projects.

Example where this might lead to confusion

const [text,setText] = useState("");
const handleClick = (newText="") => setText(newText);

<BlockStack gap="100">
<Text as="p">Your text here:</Text>
<Text as="p" fontWeight="bold" tone="success">{text}</Text>
  <Button onClick={()=>handleClick("A piece of text that gets easily written in a click")}>
     Add text in a click
 </Button>
  <Button 
  // This will pass an object instead of a string and throw an error in React
  // we'll get no warning because the type in Polaris is incorrectly set to  onClick?(): unknown;
  onClick={handleClick}
    >Clear text</Button>
</BlockStack>

Sugested Change:

Update the onClick prop definition in src/types.d.ts within the BaseButton interface to:

/** Callback when clicked, receives the click event */
onClick?(event: React.MouseEvent<HTMLButtonElement, MouseEvent>): void;

Steps to reproduce

polaris sandbox reproduction

1. Create a TypeScript project using Polaris most recent version.
2. Implement a Button component with an onClick prop.
3. Notice the TypeScript definition suggests no arguments should be passed to the callback, contrary to the actual behavior.

Are you using React components?

Yes

Polaris version number

12.11.0

Browser

Not applicable

Device

Not applicable

[pklitscher]

+1
We have some buttons where it's necessary to use stopPropagation(). To make our code "type safe" we have had to hack around it and make the event optional. However, it would be good to reliably know if this event is being passed or not. We tell the compiler it is. But if it were suddenly removed, we wouldn't know unless we check every time. This would degrade user experience.