refactor createUseStyles to work with react-hot-loader #1237
Conversation
|
Tests are passing, ignore the CI, it's a permissions probl. Looks good at the first glance. Need to check if we have all tests for cases like
|
|
@HenriBeck @eps1lon do you see any problems with this approach? |
|
@HenriBeck do you see anything wrong with this PR? I would merge it tomorrow otherwise. |
| }) | ||
|
|
||
| useEffectOrLayoutEffect( | ||
| const [sheet, dynamicRules] = React.useMemo( |
There was a problem hiding this comment.
If I remember correctly there was a problem with using React.useMemo as it can be rerun by React at a given point of time.
I think @eps1lon knew more about this.
There was a problem hiding this comment.
I think you are talking about the theoretical situation in async mode, not sure if it was made real.
| if (state.sheet && state.dynamicRules && !isFirstMount.current) { | ||
| updateDynamicRules(data, state.sheet, state.dynamicRules) | ||
| if (sheet && dynamicRules && !isFirstMount.current) { | ||
| updateDynamicRules(data, sheet, dynamicRules) | ||
| } | ||
| }, | ||
| [data] | ||
| ) | ||
|
|
||
| useEffectOrLayoutEffect( |
There was a problem hiding this comment.
Can't we just move this above the updateDynamicRules hook?
There was a problem hiding this comment.
That cleanup depends on the sheet argument, the one above on data.
|
@bs85 going to release it together with one more important bugfix that I have been working on lately |
|
Great, thank you! |
isFirstRenderis broken when using react-hot-loader, as it unmounts and remounts the components, but preserves hook state all the way through, includingisFirstRender.The
removeDynamicRuleshook runs first, and an error is thrown onceupdateDynamicRulesruns as the rules have been removed.With those changes everything should run in the proper order.