Material-ui: [Button] Is the 'type' prop really removed from ButtonBase?

[cmeeren]

According to the docs, ButtonBase no longer has a type prop (which had values submit, reset, and button). Is this a deliberate change, or a documentation bug?

I'm asking because I'm generating MUI bindings from the docs. ButtonBase props are inherited by many other components, so I don't want to remove it unless it's intended.

[oliviertassinari]

@cmeeren I think that we should document the custom default prop (type="button" over type="submit"), it can be confusing when building forms otherwise (I fell into this trap in the past building forms with our button). See https://github.com/mui-org/material-ui/pull/21002#discussion_r423994251

Should we remove the custom default type value?

[cmeeren]

If I understand you and the comment you linked to correctly, you're saying that the removal of type from the docs was deliberate and it's not coming back. The primary reason is that the allowed values depend on what's passed to the component prop.

Is that correct?

[cmeeren]

I just saw your original unedited comment, so it seems I didn't understand you correctly.

I think that for the purposes of my generator, it would be best if it was either not documented at all, or documented with all possible values. And if you're documenting just the default value, perhaps you could document it outside of the prop table? Just a suggestion.

But I can work around it anyway.

[cmeeren]

FYI, I just updated my generator to ignore any buttonBase.type prop. So feel free to do whatever you want.

[eps1lon]

I don't think we intended to remove it from the docs. Just that we don't want to restrict it at runtime because this might cause false positives. We never had any reports about <Button component="a" dowload type="audio/ogg" /> but this doesn't mean that there are none. The runtime warning could've prevented legitimate usage of other type values.

I think documenting the allowed types for the default case and then widening the allowed types at runtime to PropTypes.string is a good compromise. In the end html validators or other 3rd party tools are probably better suited for verifying the output.

[oliviertassinari]

@eps1lon I love the proposed tradeoff. I think that as long as we teach users that the default type is different with our Button and ButtonBase, it will help clarify potential behavior when building a form.

[oliviertassinari]

Would this diff make it?

diff --git a/packages/material-ui/src/ButtonBase/ButtonBase.d.ts b/packages/material-ui/src/ButtonBase/ButtonBase.d.ts
index abca5b994..ce3129e95 100644
--- a/packages/material-ui/src/ButtonBase/ButtonBase.d.ts
+++ b/packages/material-ui/src/ButtonBase/ButtonBase.d.ts
@@ -64,6 +64,10 @@ export interface ButtonBaseTypeMap<P = {}, D extends React.ElementType = 'button
      * Props applied to the `TouchRipple` element.
      */
     TouchRippleProps?: Partial<TouchRippleProps>;
+    /**
+     * The HTML `type` attribute, important for form validation.
+     */
+    type?: string;
   };
   defaultComponent: D;
   classKey: ButtonBaseClassKey;
diff --git a/packages/material-ui/src/Button/Button.d.ts b/packages/material-ui/src/Button/Button.d.ts
index 023222cf4..5d2e217f4 100644
--- a/packages/material-ui/src/Button/Button.d.ts
+++ b/packages/material-ui/src/Button/Button.d.ts
@@ -52,6 +52,10 @@ export type ButtonTypeMap<
      * Element placed before the children.
      */
     startIcon?: React.ReactNode;
+    /**
+     * The HTML `type` attribute, important for form validation.
+     */
+    type?: string;
     /**
      * The variant to use.
      */

It generates:

| Name | Type | Default | Description |
|:-----|:-----|:--------|:------------|
| type | string | 'button' | The HTML type attribute, important for form validation. |

and the CI is green

[zenje]

Hi, I can take this update!

Will we not mention the allowed values of 'button', 'reset', 'submit' anywhere in favor of conveying that various string values are allowed for type? Also, to reiterate: ButtonBase sets the type only if component = 'button', and by default in that case it's set to 'button', while Button always has by default type = 'button' - is that correct?

[inovux]

If no one is working on this I would like to take this issue :)

[inovux]

Somehow adding a type prop to the Button.d.ts will not show up on the documentation page after running yarn proptypes and yarn docs:api. Any idea how to solve this small issue @oliviertassinari

[eps1lon]

Somehow adding a type prop to the Button.d.ts will not show up on the documentation page after running yarn proptypes and yarn docs:api. Any idea how to solve this small issue @oliviertassinari

Best open a PR with your current progress. Then I can take a look.

[eps1lon]

Just realized that this prop is ignored for any component that is not undefined | 'button'.

[eps1lon]

We'll have this sorted out before v5 release where our API docs will get a major overhaul. It should then be more apparent which props are available and what components are responsible for it.

[croraf]

The HTML is setting default type to "submit" when the button is related to the form. I guess that means when the button is in the form or has a form attribute. (Not sure if only the first button is set to submit or all of them in the form.)

MUI is deviating comparing to this, which is confusing. I had a question from a MUI user today about this and it really is confusing that just changing from "button" to "Button" is not working. (Also the API docs do not mention anything on this.)

Why not just leave it as the HTML, without enforcing a default value?