Closed rfbotto closed 5 years ago
Hey @joshwcomeau!
I hope you had a great time at React Boston, looking forward to seeing your whimsy talk! :)
I updated the PR with the changes you request. In the meantime, while https://github.com/joshwcomeau/guppy/pull/265 doesn't get merged, i will also try to investigate into the reason for the performance constraint with the usage of forwardRef
in the TextInput
.
Oh! Something really interesting: I was assuming the problem was with <CircularOutline>
, since the only visible animation is the outline around the project icons in the sidebar... but when I disable that animation, the performance problem still exists in the profiling tools.
So, there's clearly a <Spring>
somewhere re-rendering a bunch when it doesn't have to 🤔
Will continue looking later, but feel free to do some digging if it interests you :D
I'm afraid master
has diverged too substantially to be able to reasonably recover this PR. I'm going to open a new PR and do my best to cherry pick all novel contributions from this. @rfbotto please feel free to critique/add to the new PR - I will link it here when it's open.
Related Issue: https://github.com/joshwcomeau/guppy/issues/213
Summary:
injectGlobal
by the newcreateGlobalStyle
innerRef
forref
React.forwardRef
in theTextInput
component in order to be able to forward theref
through the wrapper. This lead to a flow error as the flow types are not up to date with React > 16.3, hence the addition of the$FlowFixMe
annotation.ref
variables as maybe instance types (https://flow.org/en/docs/react/refs/) which in result led to some extra conditional checking before their usage..extend
forstyled()
Heading
component to use the newas
prop, instead ofwithComponent