Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FormControl] Cannot render FormControl as React.Fragment #38094

Open
2 tasks done
thany opened this issue Jul 21, 2023 · 5 comments
Open
2 tasks done

[FormControl] Cannot render FormControl as React.Fragment #38094

thany opened this issue Jul 21, 2023 · 5 comments
Assignees
Labels
component: FormControl The React component enhancement This is not a bug, nor a new feature

Comments

@thany
Copy link

thany commented Jul 21, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example:
https://codesandbox.io/s/zealous-benz-3zmg7x?file=/src/App.tsx

Steps:

  1. Use a FormControl component anywhere
  2. Make it render as a Fragment
  3. See error in the console.

Current behavior 😯

It produces an error in the console:

Warning: Invalid prop `className` supplied to `React.Fragment`. React.Fragment can only have `key` and `children` props.

Expected behavior 🤔

No error.

Context 🔦

I'm doing this because I need FormControl not to produce extra elements around my form elements, because of CSS Grid.

Applying display: contents is not sufficient for a CSS Grid to work, and subgrid is sadly still not widely enough supported.

Your environment 🌎

npx @mui/envinfo
  System:
    OS: Windows 10 10.0.19045
  Binaries:
    Node: 18.15.0 - C:\Program Files\nodejs\node.EXE
    Yarn: Not Found
    npm: 9.8.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 115.0.5790.102
    Edge: Spartan (44.19041.1266.0), Chromium (114.0.1823.86)
  npmPackages:
    @emotion/react:  11.10.6
    @emotion/styled:  11.10.6
    @mui/base:  5.0.0-alpha.124
    @mui/core-downloads-tracker:  5.11.16
    @mui/material:  5.11.16
    @mui/private-theming:  5.12.0
    @mui/styled-engine:  5.11.16
    @mui/styles:  5.12.0
    @mui/system:  5.11.16
    @mui/types:  7.2.4
    @mui/utils:  5.12.0
    @types/react:  18.2.0
    react:  18.2.0
    react-dom:  18.2.0
    typescript:  4.9.5

I'm using Firefox 115, just in case it matters.

@thany thany added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 21, 2023
@zannager zannager added the component: FormControl The React component label Jul 21, 2023
@thany
Copy link
Author

thany commented Jul 21, 2023

One possible (but silly) workaround:

const Noop: React.FC<React.PropsWithChildren> = ({ children }) => (
  <>{children}</>
);

const MyAwesomeFormLine: React.FC = () => {
  <FormControl component={Noop}>
    // Label + Input
  </FormControl>
};

So basically Noop in place of Fragment where Noop eats away the error the Fragment would normally spew out to warn about props getting passed. Not very elegant, iyam, but at least it cleans up the console.

@mj12albert
Copy link
Member

@thany I'm pretty sure this cannot work as FormControl needs to receive props for it's intended functionality, but a Fragment cannot receive anything other than key and children as the error message explains

I'm doing this because I need FormControl not to produce extra elements around my form elements, because of CSS Grid.

Can you share more about your use-case or the layout you are trying to achieve?

@mj12albert mj12albert assigned mj12albert and unassigned michaldudak Jul 21, 2023
@mj12albert mj12albert changed the title Cannot render FormControl as React.Fragment [FormControl] Cannot render FormControl as React.Fragment Jul 21, 2023
@mj12albert mj12albert added status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 21, 2023
@thany
Copy link
Author

thany commented Jul 24, 2023

Each FormControl contains a label and an input control. I (we & our designer) despise the abel-inside-or-above-the-control layout, and we have to deliver a classic but proven layout, labels on the left, fields on the right.

This is impossible to achieve if each FormControl produces a <div>, because for CSS Grid to work, all cells are required to be its children.

Currently, having multiple FormControl components for a form, produces roughly this (pseudo code):

<form>
  <div> <!-- FormControl -->
    <label>
    <input>
  <div> <!-- FormControl -->
    <label>
    <input>
  <div> <!-- FormControl -->
    <label>
    <input>

Obne cannot apply CSS Grid on this and expect the labels and inputs to magically become the grid cells. CSS Grid just simply doesn't work like that. The only correct way to get this to render correct as a grid, is to eliminate those divs:

<form>
  <> <!-- FormControl -->
    <label>
    <input>
  <> <!-- FormControl -->
    <label>
    <input>
  <> <!-- FormControl -->
    <label>
    <input>

So that the labels and inputs become children of the form, which is the CSS Grid.

Now, to be brutally honest, I don't see why FormControl would absolutely need to render itself as a div. What for? The way I see it, FormControl is merely a empty component to provide some functionality to its contained form element components. It doesn't need to render anything itself. It current does (or tries to anyway 😒) but to what end? I don't need its classnames to exist in the HTML structure, and I certainly don't like to be limited to having each label+input combination wrapped in another element - I need and want the freedom not to require this.

To put this more generally - a UI framework should not dictate what HTML elements should be rendered outside its components. It should never render any HTML elements that only it needs, but the browser doesn't. Any wrapping functional-only component should not render any HTML, but should only provide the intended functionality to its child components. That's a lot of "should" so feel free to disagree, as long as we can solve the problem described above 😉

Now, if there's any other component than FormControl I should be using instead, while still providing states and whatnot to its child input components, I'm all ears to hear about it 🙂


Edit:

@thany I'm pretty sure this cannot work as FormControl needs to receive props for it's intended functionality, but a Fragment cannot receive anything other than key and children as the error message explains

Btw if I take that literally - FormControl can still take all the props in the world without rendering anything. Recieving props has nothing to do with whatever a component returns as. I'm sure you know this, but I just wanted to make sure we're on the same page.

The issue is that FormControl, being able to be told what component to render as, blindly assumes that everything it passes to that component, it can receive on its turn, which is clearly an unsafe assumption.

@michaldudak
Copy link
Member

@mj12albert, I think we should be able to allow this. The FormControl doesn't set or rely on event handlers. We could either allow React.Fragment or null as component/slots.root or add a new prop that would control if the FormControlRoot is rendered at all.

@thany
Copy link
Author

thany commented Jul 24, 2023

@mj12albert, I think we should be able to allow this. The FormControl doesn't set or rely on event handlers. We could either allow React.Fragment or null as component/slots.root or add a new prop that would control if the FormControlRoot is rendered at all.

I would rather like the latter, because it's entirely possible to build a component that also cannot take a className prop, or whatever else FormControl is trying to pass on. Add some strict propType validation and it too may spew out an error similar to the one from Fragment.

@mj12albert mj12albert added enhancement This is not a bug, nor a new feature and removed status: expected behavior Does not imply the behavior is intended. Just that we know about it and can't work around it labels Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: FormControl The React component enhancement This is not a bug, nor a new feature
Projects
None yet
Development

No branches or pull requests

4 participants