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

MUI 5 compatibility #55

Closed
JazeDev opened this issue Jan 13, 2021 · 19 comments
Closed

MUI 5 compatibility #55

JazeDev opened this issue Jan 13, 2021 · 19 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request long-term This issue may be open for a while due to misc holdups/dependencies

Comments

@JazeDev
Copy link

JazeDev commented Jan 13, 2021

Description

see MUI 5 compatibility #2542

Additional Notes

Maybe add a @next branch for MUI 5

@JazeDev JazeDev added the question Further information is requested label Jan 13, 2021
@oze4 oze4 self-assigned this Jan 13, 2021
@oze4 oze4 added bug Something isn't working enhancement New feature or request and removed question Further information is requested labels Jan 13, 2021
@oze4
Copy link
Member

oze4 commented Jan 13, 2021

Thank you for this. Is that the best way to handle this? Just use another branch? We currently have a next repo which we are moving to TS.

@vmihalachi
Copy link
Contributor

We might also start to be compatible with only MUI 5, if maintaining two versions is too much.
It's true that it is still marked as alpha, but per my use I see it as pretty stable. And there a lot of nice things with moving to version 5 😄

https://medium.com/material-ui/2020-in-review-and-beyond-55d2cab0975c

@oze4
Copy link
Member

oze4 commented Jan 14, 2021

I think focusing on MUI compatibility in the next repo makes more sense. As you say, there are nice features, but it is still in alpha. So I don't see a need for this to take priority right now, or in this repo.

Thoughts?

@vmihalachi
Copy link
Contributor

Agreed!

@masbaehr
Copy link

Hi, i noticed that react 17 is already used, but in MUI 4 only react 16 is supported officially. Was this done on purpose. Could any problems arise ?

npm WARN @material-table/core@2.3.4 requires a peer of react@^17.0.1 but none is installed. You must install peer dependencies yourself.

@vmihalachi
Copy link
Contributor

@masbaehr on my projects I'm not running into any issue. I'm also using MUI 5.0.0-alpha.11 and looks like this library is compatible until that one version.

@VladLegkowski
Copy link

VladLegkowski commented Jan 29, 2021

Hi guys,

I am seeing these issues with MUI5 and material-table-core/core.

Warning: Failed prop type: The prop onPageChange is marked as required in ForwardRef(TablePagination), but its value is undefined.

Warning: Unknown event handler property onChangePage. It will be ignored.

Warning: Unknown event handler property onChangeRowsPerPage. It will be ignored.

Material-UI: The fade color utility was renamed to alpha to better describe its functionality.

Package.json

    "@material-table/core": "^2.3.24",
    "@material-ui/core": "^5.0.0-alpha.23",

@vmihalachi
Copy link
Contributor

vmihalachi commented Jan 29, 2021

@VladLegkowski looks like we will have an alpha version of the library that will support mui 5.

In the meantime you can go as far as version 5.0.11 of mui, because there are no breaking changes until then

@VladLegkowski
Copy link

Ah, I already did this migration in my codebase - https://next.material-ui.com/guides/migration-v4/.

I will need to roll back a lot of stuff to make it compatible. @vmihalachi, I know I don't deserve even to ask this question, because I can only imagine how hard it is to maintain OS package like this, but you have any idea when you will have the alpha version?

Thank you.

@masbaehr
Copy link

npm ERR! Could not resolve dependency:
npm ERR! peer react@"^17.0.1" from @material-table/core@2.3.26

Happened again with an older project i wanted to upgrade to table/core
I think react 16 should still be supported until MUI v5 is final, as react 17 is not supported by MUI v4

@villuv
Copy link
Contributor

villuv commented Feb 26, 2021

@oze4 Hi, I'm the author of the MUI5 conversion pull request on original repository. If this PR is any use here I could try to port it over here. Let me know if I could help you here.

@WesleyKapow
Copy link

Any word on this? Would love to see the onPageChange vs onChangePage changes get pulled into this.

@villuv
Copy link
Contributor

villuv commented Mar 25, 2021

Nothing either here or in original repo. Meanwhile I bumped my PR to latest alpha, there were no immediately noticable breaking changes.

@Domino987
Copy link
Contributor

I just started working on a migration to v5. I started with the third level imports to fix the tests and will move to the pagination issue next, which looks like is the only error for v5.30 as far as I can see.

@Domino987 Domino987 mentioned this issue May 8, 2021
@Domino987 Domino987 added the long-term This issue may be open for a while due to misc holdups/dependencies label May 10, 2021
@villuv
Copy link
Contributor

villuv commented May 19, 2021

I updated upstream pull request to v5.0.0-alpha.34 level

@vmihalachi
Copy link
Contributor

@villuv is there any way to download an alpha version with these changes?

@villuv
Copy link
Contributor

villuv commented May 19, 2021

@vmihalachi Just fork my version of it from https://github.com/villuv/material-table/tree/feature/mui5 and check out feature/mui5 branch. I haven't published a built package as I haven't thorougly tested it. It builds fine with yarn install; yarn build and you can use it locally with yalc, for example. Let me know if you find anything wonky.

@oze4
Copy link
Member

oze4 commented May 19, 2021

Feel free to send of a PR of your fork with MUI5 compatibility and we will get it merged. Thanks!

@villuv villuv mentioned this issue May 19, 2021
@Domino987
Copy link
Contributor

The next branch supports v5. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request long-term This issue may be open for a while due to misc holdups/dependencies
Projects
None yet
Development

No branches or pull requests

8 participants