[WIP] makeStyles and functional components #3461

MohammedFaragallah · 2019-07-25T12:15:29Z

current status

And then there is the doc:

djhi

Cool :) I think you don't have to use createStyles when you use makeStyles (see https://material-ui.com/styles/basics/#hook-api)

Kmaschta · 2019-07-25T14:06:45Z

Ah! The tests are failing because with the withStyles HOC, the components were wrapped on the tests like WithStyles(Component).

This is no longer the case now with the hook.
Can you remove all the WithStyles(x) and only keep the x in the tests that are failing?

djhi · 2019-07-25T14:15:18Z

Can you remove all the WithStyles(x) and only keep the x in the tests that are failing?

Even better, if you feel up for it, convert them to react-testing-library 😍

MohammedFaragallah · 2019-07-25T20:42:19Z

Even better, if you feel up for it, convert them to react-testing-library 😍

sorry @djhi i don't think i can do this i'm not very good with testing

MohammedFaragallah

i need feedback on this if it's good i will continue converting the others to functional components
also how should i handle components with shouldComponentUpdate

Kmaschta · 2019-07-26T07:26:26Z

This looks very good to me.
But don't bother transforming class-based components into function-based ones.

You can use useStyles in a render() function of a class-based component.
That way, you can let all the lifecycle in place.

djhi · 2019-07-26T07:28:49Z

You can use useStyles in a render() function of a class-based component.

No, useStyles is a hook. Hooks can't be used in a class component

See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-from-react-functions

Kmaschta · 2019-07-26T07:33:44Z

My bad! Seems like we can't.

So, about your question, you can use React.memo instead of shouldComponentUpdate: https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-shouldcomponentupdate

MohammedFaragallah · 2019-07-26T20:34:12Z

@djhi some components are class based while they dot use any lifecycle methods or internal state like ra-tree-ui-materialui/TreeNodeContent should i convert them too ?

and could you give savebutton tests a look

Kmaschta · 2019-07-26T21:12:28Z

If you feel like you can convert them, else you don't have to.
We'll just mark them as non-converted, it's ok.

MohammedFaragallah · 2019-07-26T22:21:12Z

It's fine by me i would love to help this awesome project whenever i can

…seStyles

Kmaschta · 2019-07-31T09:22:28Z

Don't feel obligated to migrate all the components at once!
The smaller the PR is, the faster it will be reviewed and merged.
The bigger it is, the more you might get rebase to do and request changes.

This one tend to be huge, and a split might be a good idea IMHO.

fzaninotto · 2019-07-31T10:12:04Z

@MohammedFaragallah please fix the tests before converting more components. We can't merge this PRs if it breaks the tests.

MohammedFaragallah · 2019-07-31T16:40:38Z

@Kmaschta yes i won't be converting any more components this status list is for the maintainers to keep track whats almost done, but i'll focus on getting tests fixed as @fzaninotto asked, sorry for the delay but i'm really busy this days and my lack of experience with testing isn't helping very soon all will be resolved i hope

…seStyles

djhi · 2019-08-05T09:13:28Z

@MohammedFaragallah I'm going to change all inputs for react-final-form migration. I will update them to use makeStyle at the same time. You should omit them to avoid conflicts later in your smaller prs

Kunnu01 · 2019-08-07T23:42:14Z

@djhi , What to do in case of componentDidCatch?

fzaninotto · 2019-08-21T13:01:43Z

Thanks a lot for your effort, but your work isn't going to be merged soon, because of the rebases and failed tests. I'm going to close this, and I recommend that you cherry-pick the commits to open smaller PRs, migrating a handful of components only (less than a dozen), and making sure the tests pass. I'll do the same for your other PR.

makeStyles for functional components

fc223ec

djhi requested changes Jul 25, 2019

View reviewed changes

removed createStyles

1faad93

MohammedFaragallah added 2 commits July 25, 2019 22:29

fixes failing test

f0fe0c7

Merge branch 'next' into useStyles

a68ab7c

MohammedFaragallah added 2 commits July 26, 2019 04:34

merge

f165d35

class => functional components / use makeStyles

0fe39fc

MohammedFaragallah changed the title ~~makeStyles for functional components~~ [WIP] makeStyles and functional components Jul 26, 2019

MohammedFaragallah commented Jul 26, 2019

View reviewed changes

fix tests

4e6c658

MohammedFaragallah added 3 commits July 26, 2019 21:09

ra-tree-ui-materialui DragLayer

1cea7c2

ra-tree-ui-materialui DragPreview/NodeForm

b2437c6

ra-tree-ui-materialui RootDropTarget/tree

1b912c4

MohammedFaragallah added 8 commits July 27, 2019 00:27

prevent firing ra-tree-ui-materialui/Tree useEffect after every update

651c638

convert TreeNodeContent/TreeNodeWithChildren

34bb6c3

Merge branch 'next' of https://github.com/marmelab/react-admin into u…

ad0516a

…seStyles

makeStyles typescript components

28ecd0a

class => functional components

fd60887

FileInputPreview/SingleFieldList

2b23e4d

SimpleFormIterator

4e1cca9

makeStyles TabbedForm

2d16633

MohammedFaragallah added 2 commits July 28, 2019 04:06

save

db7c638

Merge branch 'next' of https://github.com/marmelab/react-admin into u…

50f69fc

…seStyles

Merge branch 'next' of https://github.com/marmelab/react-admin into u…

60f87af

…seStyles

m4theushw mentioned this pull request Aug 3, 2019

[RFR] Replace withStyles with useStyles #3485

Merged

2 tasks

fzaninotto closed this Aug 21, 2019

fzaninotto mentioned this pull request Aug 21, 2019

[WIP] convert ra-ui components to classes/useStyles #3478

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] makeStyles and functional components #3461

[WIP] makeStyles and functional components #3461

MohammedFaragallah commented Jul 25, 2019 •

edited

Loading

djhi left a comment

Kmaschta commented Jul 25, 2019

djhi commented Jul 25, 2019

MohammedFaragallah commented Jul 25, 2019

MohammedFaragallah left a comment

Kmaschta commented Jul 26, 2019

djhi commented Jul 26, 2019 •

edited

Loading

Kmaschta commented Jul 26, 2019 •

edited

Loading

MohammedFaragallah commented Jul 26, 2019

Kmaschta commented Jul 26, 2019

MohammedFaragallah commented Jul 26, 2019

Kmaschta commented Jul 31, 2019

fzaninotto commented Jul 31, 2019 •

edited by Kmaschta

Loading

MohammedFaragallah commented Jul 31, 2019

djhi commented Aug 5, 2019

Kunnu01 commented Aug 7, 2019

fzaninotto commented Aug 21, 2019

[WIP] makeStyles and functional components #3461

[WIP] makeStyles and functional components #3461

Conversation

MohammedFaragallah commented Jul 25, 2019 • edited Loading

djhi left a comment

Choose a reason for hiding this comment

Kmaschta commented Jul 25, 2019

djhi commented Jul 25, 2019

MohammedFaragallah commented Jul 25, 2019

MohammedFaragallah left a comment

Choose a reason for hiding this comment

Kmaschta commented Jul 26, 2019

djhi commented Jul 26, 2019 • edited Loading

Kmaschta commented Jul 26, 2019 • edited Loading

MohammedFaragallah commented Jul 26, 2019

Kmaschta commented Jul 26, 2019

MohammedFaragallah commented Jul 26, 2019

Kmaschta commented Jul 31, 2019

fzaninotto commented Jul 31, 2019 • edited by Kmaschta Loading

MohammedFaragallah commented Jul 31, 2019

djhi commented Aug 5, 2019

Kunnu01 commented Aug 7, 2019

fzaninotto commented Aug 21, 2019

MohammedFaragallah commented Jul 25, 2019 •

edited

Loading

djhi commented Jul 26, 2019 •

edited

Loading

Kmaschta commented Jul 26, 2019 •

edited

Loading

fzaninotto commented Jul 31, 2019 •

edited by Kmaschta

Loading