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

[Table] Rendering a table fails silently when using unsupported child element #4726

Closed
corinnaerin opened this issue Jul 15, 2016 · 3 comments
Labels
component: table This is the name of the generic UI component, not the React module!

Comments

@corinnaerin
Copy link

corinnaerin commented Jul 15, 2016

Problem description

If you include a child of the Table component that is not TableHeader, TableBody, or TableFooter, the child component is never rendered but no error is thrown. Failing silently (or strictly speaking, ignoring the child silently) is an anti-pattern and resulted in several hours of frustration in trying to root cause why the components weren't rendering.

Possible solutions

  • In the render method for the Table component, throw an exception if the component is not supported.
  • Render the subcomponent as is even if it's not one of the TableHeader, TableBody, or TableFooter. After all, there are no such restrictions that TableBody's children must be TableRows, and it enforces long/complex components instead of being able to break down in to small parts.

Steps to reproduce

Breakdown the components of your table into subcomponents that wrap the MaterialUi subcomponents:

Table
    MyTableHeader
        TableHeader
            TableRow
                TableHeaderColumn
    MyTableBody
        TableBody
            TableRow
                TableRowColumn

The render methods of MyTableHeaderand MyTableBody are never executed, and no errors are logged to the console.

Versions

  • Material-UI: 0.15.1
  • React: N/A
  • Browser: N/A
@robertklep
Copy link

robertklep commented Jul 19, 2016

Similar issue here: I have a component that renders <TableBody>'s from an array, and those are being ignored completely.

<Table>{
  list.map(item => <Item.../>)
}</Table>

(where Item generates a <TableBody>).

EDIT: I tried a workaround by assigning Item.muiName = 'TableBody', but that presents the next problem: material-ui will only allow one <tbody> to be rendered (although multiple <tbody>'s are allowed in HTML).

@pkese
Copy link

pkese commented Jul 20, 2016

I've just been bitten by this bug too. I have spent most of the afternoon in tracing this issue.
The problematic code is in https://github.com/callemall/material-ui/blob/master/src/Table/Table.js#L251:

      const {muiName} = child.type;
      if (muiName === 'TableBody') {
        tBody = this.createTableBody(child);
      } else if (muiName === 'TableHeader') {
        tHead = this.createTableHeader(child);
      } else if (muiName === 'TableFooter') {
        tFoot = this.createTableFooter(child);
      }

Apparently someone made a too strong assumption about TableHead or TableBody being the only way of rendering the table.


A possible (albeit a bit ugly) workaround is to write one's code a bit differently...
If MyTableHeader and MyTableBody are simple pure-functional components that (when called) return TableHeader and TableBody, then instead of this

<Table>
  <MyTableHeader/>
  <MyTableBody/>
</Table>

one can write it like this:

<Table>
  {MyTableHeader()}
  {MyTableBody()}
</Table>

@mpontikes mpontikes mentioned this issue Aug 5, 2016
13 tasks
@oliviertassinari oliviertassinari added the component: table This is the name of the generic UI component, not the React module! label Dec 18, 2016
@oliviertassinari oliviertassinari changed the title Rendering a table fails silently when using unsupported child element [Table] Rendering a table fails silently when using unsupported child element Dec 18, 2016
@oliviertassinari
Copy link
Member

We have been porting the component on the v1-beta branch. We reimplemented it from the ground-up. While we haven't tested it, I think that the issue is most likely fixed on that branch. Hence, I'm closing it.
Still, we will accept PR fixes until v1-beta takes over the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: table This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

4 participants