gregnb / mui-datatables

Datatables for React using Material-UI
MIT License
2.71k stars 932 forks source link

setTableProps ignored in prod #1074

Open wroughtec opened 4 years ago

wroughtec commented 4 years ago

When enabling dense table in my local with setTableProps as per example all works as expected. However when released the styles are ignored. The only difference seems to be when I have the env as production (although when in watch mode and production it works) bundling with parcel js.

Anyone else seen this?

Here you can see it working as expected: https://codesandbox.io/s/happy-bhaskara-ft2vo

And here is a a small repo to show it working on yarn start but fails on yarn build then yarn serve https://github.com/wroughtec/tableDemo

It seems to be an issue with production and parcel as the first version I tried with CRA exactly the same code only difference was the scripts worked.

Tech Version
Material-UI 4.7.0
MUI-datatables 2.13.0
React 16.12.0
gabrielliwerant commented 4 years ago

Hey @wroughtec, not sure offhand what the issue might be, but I can offer this bit of advice for debugging, which has definitely bitten me before. When using semver to manage your packages, it's possible that you have different versions of dependencies running in production vs local. I would check your production dependency versions against your local versions and see if that's the case. Any upgrade on any dependency might have broken something. If I'm on a project where I need to be extra cautious about this happening, I will stop using semver in my package files, and make sure to use only exact versions.

wroughtec commented 4 years ago

I wish that was the case but the small repo I set up shows the issue locally comparing dev with prod. I also used CRA with the same dependencies and that had no issue so I suspect its something Parcel is doing.

patorjk commented 4 years ago

@wroughtec Your example didn't have a setTableProps option on the options object. Also, while looking at the Material UI docs, I noticed that they recently removed the "dense table" property for the Table component. If your prod and dev versions are using different versions of the lib (like @gabrielliwerant suggested), that may be the source of the problem. Have you tried setting other properties on the Table object to see if those carry over?

The way setTableProps works is that it attaches the props you give it to the Table component, so it seems odd that Parcel would be interfering. I updated your example so that it sets the style property of the Table component:

https://codesandbox.io/s/upbeat-breeze-f9fr7

You may want to try something like that and see if your prod version gets correctly updated.

wroughtec commented 4 years ago

@patorjk It does on the repo example: https://github.com/wroughtec/tableDemo/blob/c79606ca100dfb60f8c95716b7ec42b04dc53f6e/src/index.js#L141

Must have not saved the codesand does now https://codesandbox.io/s/happy-bhaskara-ft2vo

I can confirm they are not using different versions of the lib as I had it running locally both dev and prod builds gave different results same libs and when I tried with a webpack build had no issues either.

i have updated the example to prove this: https://github.com/wroughtec/tableDemo

All you need to do is install yarn then run yarn serve

You can rebuild if you wish as well check the readme for info. I also locked down versions to further prove that I am not going mad ;)

patorjk commented 4 years ago

https://codesandbox.io/s/github/wroughtec/tableDemo

It seems to work fine. Is there something that is missing from how CodeSandbox executes that would affect things?

wroughtec commented 4 years ago

@patorjk that is my whole point it works fine in dev (codesandbox run as dev). The repo shows the differences comparing webpack and parcel builds when in production.

AlCalzone commented 4 years ago

I'm having the same issue with a datatable that has setTableProps: () => ({ size: "small" }):

Here's the same table with two different build options:

Step 1: rm -rf node_modules ; npm ci ; rm -rf .cache ; parcel build grafik

Step 2: rm -rf .cache ; parcel watch grafik

patorjk commented 4 years ago

So I setup a create-react-app project and installed mui-datatables, and these options seem to work fine:

    const options = {       filter: true,       filterType: 'dropdown',       responsive: 'simple',       setTableProps: () => (         {           style: {             backgroundColor: 'red'           },           size:'small'         }       )     };

In my production setup at work (which uses a fork of this table - though it has the setTableProps feature), it also seems to work. If you right click and inspect the table element, is the property not there?

AlCalzone commented 4 years ago

No, the small property is nowhere to be found. But that's not actually a html property though.

patorjk commented 4 years ago

If you input this:

  setTableProps: () => {
    console.log('setting table props');
    return {
      style: {
        backgroundColor: 'red'
      },
      size:'small'
    };
  }

Do you see the console statement in dev, but not production?

AlCalzone commented 4 years ago

I see it in both cases. However in the dev build (parcel watch), there is a css class MuiTableCell-sizeSmall applied, but I cannot find the corresponding rule in the prod build (parcel build).

patorjk commented 4 years ago

What if you add a "styling" property or some other property? Do you see it on the tag when you inspect in production? Also, what happens if you add a non-mui class like "test_class" to the table, does that appear?

AlCalzone commented 4 years ago
        setTableProps: () => {
            return {
                size: "small",
                style: {
                    backgroundColor: "red",
                },
                class: "foobar",
            };
        },
property parcel build parcel watch
size ✗ no rule with small padding (✓) class MuiTableCell-sizeSmall exists
style
class
patorjk commented 4 years ago

So I cloned the repo from @wroughtec and can confirm size doesn't show up, but it looks like the table is applying the dense styling. If I inspect the "td" components of the table, and then toggle the "Dense Table" option, I can see styling added and removed.

I'm thinking this is a material-ui thing. The setTableProps method is applying props to the Material UI Table component. I'm assuming this component reads in the "size" prop but doesn't apply it to the HTML table tag. It probably only passes on props that aren't apart of its API. One data point in support of this idea is if I add a component field:

    setTableProps: () => {
        return {
            size: "small",
            style: {
                backgroundColor: "red",
            },
            className: "foobar",
                            component: "table"
        };
    },

That doesn't get added to the table tag either. So I'm thinking everything is actually working correctly. Though I'm curious if you see any styling changes on the "td" tags. A "MuiTableCell-sizeSmall" class gets added to td tags when I set "size" to "small".

AlCalzone commented 4 years ago

it looks like the table is applying the dense styling

It doesn't for me. In the production build, there are no MuiTableCell-xyz classes, only jss-123. And none of those have the dense padding values.

patorjk commented 4 years ago

What occurs if you put a normal table below your datatable and manually put a size field on it:

import Table from "@material-ui/core/Table";
import TableRow from "@material-ui/core/TableRow";
import TableCell from "@material-ui/core/TableCell";

return (
    <>
        ...your datatable here...

        <Table size="small">
            <TableRow>
                <TableCell>Test</TableCell>
            <TableRow>
        </Table>
    </>
);

Does the table have any different classes than the datatable? It look like the size field puts classes on the "td" element. Are there any class differences between the td components?

AlCalzone commented 4 years ago

Test table: grafik

Datatable: grafik

The inline styles are my current workaround for the issue (setCellProps).

andre-rabold-tfs commented 3 years ago

I'm running in the same problem: running with parcel watch works as expected, parcel build doesn't add the MuiTableCell-sizeSmall class and thus doesn't apply my custom theme styling for small table cells.

So, seems that in release builds the props aren't passed down to the cell properly for some reason. Any ideas?

(Using @material-ui/core 4.11.2)