Add type to all internal button elements

[lseppala]

Description

Several Primer UI elements use the <button> element as part of rendering, such as Tab nav. Buttons without a type tag automatically default to type="submit" (MDN docs). If there's a form tag containing any Primer components that render buttons, interacting with the element will trigger the form submission action. This is surprising and undesirable, especially when the component doesn't make sense as a submission button.

(xref: https://github.com/github/internal-statuspage/pull/4091, https://github.com/github/incident-lifecycle/issues/737)

Steps to reproduce

I had some trouble setting up the latest version of primer on CodeSandbox (it complains about missing files from @oddbird/popover-polyfill, despite adding it as a dependency). Here's a minimum example that demonstrates the issue:

import { TabPanels } from "@primer/react";

function handleSubmit(event: React.FormEvent<HTMLFormElement>) {
  event.preventDefault();
  console.log("form submitted");
}

export default function App() {
  return (
    <form onSubmit={handleSubmit}>
      <TabPanels aria-label="Select a tab" id="tab-panels" defaultTabIndex={0}>
        <TabPanels.Tab>Tab 1</TabPanels.Tab>
        <TabPanels.Tab>Tab 2</TabPanels.Tab>
        <TabPanels.Tab>Tab 3</TabPanels.Tab>
        <TabPanels.Panel>Panel 1</TabPanels.Panel>
        <TabPanels.Panel>Panel 2</TabPanels.Panel>
        <TabPanels.Panel>Panel 3</TabPanels.Panel>
      </TabPanels>
    </form>
  );
}

Clicking each of the tabs will cause form submitted to be printed to the console, because each tab renders a button without a type attribute.

My opinion (not that anyone asked for it): it would be best to avoid unspecified type attributes for buttons with a linting check, like with https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/button-has-type.md

Version

v36.27.0

Browser

Chrome, Safari, Firefox, Edge, iOS Safari

[iansan5653]

Looks like public Button components have default types (#2806, #3730), but internal uses of button elements may be missing a type. I agree that a linting rule here would be a good idea.

[joshblack]

Hi all! Opened up a PR here for this: #4970

I enabled the rule that was mentioned above (react/button-has-type) and addressed the errors that came up. Unfortunately, these were in test files and it seems like components like TabPanels (now UnderlinePanels) are not being caught by this since this is an interop point with a custom element.

I updated the component specifically in that PR and will see if there is something we can do to help prevent this from happening in these kinds of scenarios in the future 🤔

Also wanted to leave a note that the experimental TabPanels component is being removed in favor of UnderlinePanels! Let me know if you any questions about that 👍

[lesliecdubs]

I think this should've gotten closed by #4970. Please reopen or respond if there's more to discuss here. Thanks all!