search

gregnb / mui-datatables

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

Filter Table button crashes the page on Material-UI Next. #1559

Open mburakeker opened 4 years ago

mburakeker commented 4 years ago

Expected Behavior

I expect the Filter Table pop up to show up when I click to the button.

Current Behavior

It crashes whole the page.

Steps to Reproduce (for bugs)

https://codesandbox.io/s/muidatatables-custom-toolbar-forked-eff5p?file=/package.json

  1. Open CodeSandbox link
  2. Click the Filter Table button

    Your Environment

Tech Version
Material-UI 5.0.0-alpha.12
MUI-datatables 3.5.0
React 16.13.1
browser CodeSandbox
@emotion/core 10.0.35
@emotion/styled 10.0.27
react-dom 16.13.1
@material-ui/core 5.0.0-alpha.12
@material-ui/icons 4.9.1
nickraphael commented 3 years ago

Can confirm. This is killing me. Could do with some indication on when this could be resolved. It's holding up a production release. I've tried digging around but I've no idea.

patorjk commented 3 years ago

I haven't had a chance to test the library with version 5.0 yet. If anyone wants to take a look I'd be open to a PR for this as long as it's backwards compatible with Material UI 4.x.

davidcraddock commented 3 years ago

I have looked into this error. It is a breaking change from Material-UI version 4 to 5. They have renamed GridList to ImageList and GridListItem to ImageListItem along with changing tile to root in ImageListItem. Changing this in TableFilter.js fixes the problem.

https://github.com/mui-org/material-ui/pull/22363 https://github.com/mui-org/material-ui/pull/22385

So not sure a backwards compatible change is possible. I have forked the latest release in my own repository and have put the fix there and have attach the tableFilter.js file and the package.json file with the updated Material-UI version I used 5.0.0-alpha.14

https://github.com/davidcraddock/mui-datatables

fix.zip

wdh2100 commented 3 years ago

@davidcraddock

It looks like GridList(ImageList) should be changed to Grid.

https://material-ui.com/components/grid/ https://material-ui.com/components/grid-list/

If there is no problem even if the component changes, I think it is correct to change.

What do you think?

davidcraddock commented 3 years ago

@wdh2100 I really don't mind how the final solution is architected, I just needed this problem solved quickly for a project I am working on and thought I would share what I did.

FYI to add it to your project replace the version of mui-datatables in your package.json with "mui-datatables": "davidcraddock/mui-datatables"

and run npm update mui-datatables

wdh2100 commented 3 years ago

I really don't mind how the final solution is architected, I just needed this problem solved quickly for a project I am working on and thought I would share what I did.

Yes, that's right. thank you for share.

wdh2100 commented 3 years ago

https://github.com/mui-org/material-ui/pull/22900

Rename onChangeRowsPerPage, onChangePage

https://github.com/gregnb/mui-datatables/blob/1edfd405b5b1ebb1af98c67f3efb4053a04260a1/src/components/TablePagination.js#L101-L102

davidcraddock commented 3 years ago

I've updated my repo with the rename changes now as well.

Also onExited event no longer exists in Popover so I had to modify that to get it to work when you have an apply filter button

destegabry commented 3 years ago

Thank you @davidcraddock, saved me a fork!

@patorjk would you consider opening a next branch to consolidate all the changes needed for material-ui@5?

destegabry commented 3 years ago

@davidcraddock I'm using your fork in my project and it works perfectly when I run and build it from my local machine, but it breaks my CI.

It seems that npm ci drops the dist folder. Do you have any workaround for this?

davidcraddock commented 3 years ago

hmmm could be that my package.json and package-lock.json were out of sync and npm ci uses the package-lock.json file. I have updated it in my fork now, so see if that works. If not check that you are not missing an external dependency that mui-datatables rely on?

react prop-types lodash clsx react-dnd react-dnd-html5-backend react-to-print @material-ui/core

davidcraddock commented 3 years ago

Updated fork to incorporate changes from https://github.com/gregnb/mui-datatables/pull/1565

destegabry commented 3 years ago

@davidcraddock seems nothing has changed :(

This is what npm ls outputs after npm install:

rm -Rf node_modules && npm install && npm ls  mui-datatables
└── mui-datatables@3.6.0  (github:davidcraddock/mui-datatables#d5edac7b56d8eed103a2c731849c6afef26df387)

This is after npm ci:

rm -Rf node_modules && npm ci && npm ls  mui-datatables
└── UNMET DEPENDENCY mui-datatables@github:davidcraddock/mui-datatables#d5edac7b56d8eed103a2c731849c6afef26df387
patorjk commented 3 years ago

@destegabry have you tried 3.7.1? It was released last night and should have the needed changes in it.

davidcraddock commented 3 years ago

@patorjk I've just tests release 3.7.1 with Material-UI v5 and sadly it still does not work.

@destegabry looks like an UNMET DEPENDENCY, take a look at what versions of the dependenciess you are using in your project and compare them to what this package.json needs. For @material-ui/core I am using "^5.0.0-alpha.14" make sure you are using the same

wdh2100 commented 3 years ago

@davidcraddock @patorjk

Changed in compatibility(v5) in 3.7.1. However, there are still a few changes.

There are some changes in v5. What I found and what you reported.

https://github.com/mui-org/material-ui/issues/20012

There seems to be more. It looks like more testing is needed. It will not be easy to support both versions(v4, v5)

destegabry commented 3 years ago

@patorjk v3.7.1 it's not compatible with v5.0.0-alpha.12+ because of the changes that @wdh2100 have linked. I think it's not possible to have backward compatibility with ui-material v4.

@davidcraddock sorry but yesterday I didn't had time to check, this is my package.json, still getting UNMET DEPENDENCY only after npm ci:

{...
"dependencies": {
    "@datadog/browser-logs": "^1.25.0",
    "@date-io/date-fns": "^2.10.6",
    "@emotion/core": "^10.1.0",
    "@emotion/styled": "^10.0.27",
    "@material-ui/core": "^5.0.0-alpha.14",
    "@material-ui/icons": "^5.0.0-alpha.14",
    "@material-ui/lab": "^5.0.0-alpha.14",
    "@material-ui/pickers": "^4.0.0-alpha.12",
    "@material-ui/styles": "^5.0.0-alpha.14",
    "@top-solution/mui-inputs": "^0.9.0",
    "@top-solution/use-form": "^0.6.0",
    "@top-solution/utils": "^0.3.1",
    "axios": "^0.21.0",
    "clsx": "^1.1.1",
    "date-fns": "^2.16.1",
    "lodash.assignwith": "^4.2.0",
    "lodash.clonedeep": "^4.5.0",
    "lodash.debounce": "^4.0.8",
    "lodash.find": "^4.6.0",
    "lodash.get": "^4.4.2",
    "lodash.isequal": "^4.5.0",
    "lodash.isundefined": "^3.0.1",
    "lodash.memoize": "^4.1.2",
    "lodash.merge": "^4.6.2",
    "material-ui-chip-input": "^2.0.0-beta.2",
    "mdi-material-ui": "^6.20.0",
    "mui-datatables": "davidcraddock/mui-datatables",
    "prop-types": "^15.7.2",
    "react": "^17.0.1",
    "react-dnd": "^11.1.3",
    "react-dnd-html5-backend": "^11.1.3",
    "react-dom": "^17.0.1",
    "react-helmet": "^6.1.0",
    "react-redux": "^7.2.2",
    "react-router-dom": "^5.2.0",
    "react-sortable-tree": "^2.7.1",
    "react-to-print": "^2.8.0",
    "react-scripts": "^4.0.0",
    "redux": "^4.0.5",
    "typeface-roboto": "1.1.13",
    "yup": "^0.29.3"
  },
  "devDependencies": {
    "@types/mui-datatables": "^3.4.4",
    "@types/node": "^14.14.6",
    "@types/prop-types": "^15.7.3",
    "@types/react": "^16.9.55",
    "@types/react-dom": "^16.9.9",
    "@types/react-helmet": "^6.1.0",
    "@types/react-redux": "^7.1.9",
    "@types/react-router-dom": "^5.1.6",
    "@types/yup": "^0.29.9",
    "@typescript-eslint/parser": "^4.6.0",
    "eslint-config-prettier": "^6.15.0",
    "eslint-plugin-prettier": "^3.1.4",
    "eslint-plugin-react-hooks": "^4.2.0",
    "husky": "^4.3.0",
    "lint-staged": "^10.5.1",
    "prettier": "^2.1.2",
    "redux-devtools-extension": "^2.13.8",
    "typescript": "^4.0.5"
  },
...
}
davidcraddock commented 3 years ago

@destegabry I left @material-ui/icons to its default version of "@material-ui/icons": "^4.0.0", as you are using "@material-ui/icons": "^5.0.0-alpha.14" maybe that's the issue? try reverting that back and test otherwise compare whats in my forked package.json to your own and do some testing. I tried an npm ci on my project yesterday and it worked for me

destegabry commented 3 years ago

@davidcraddock I'm still confused but running npm config set unsafe-perm true have saved my day