refactor(suite/components): improve typing and performance of the VirtualizedList #16592
Conversation
| @@ -90,7 +90,7 @@ export const SelectAssetModal = ({ | |||
| badge, | |||
| contractAddress, | |||
| shouldTryToFetch, | |||
| }: AssetProps) => ( | |||
| }) => ( | |||
There was a problem hiding this comment.
No need here anymore, due to generic types work properly.
There was a problem hiding this comment.
TODO: add type tests here
| } | ||
|
|
||
| // NOTE: typecast here + memo() and forwardRef() because of passing the ref for useShadow() | ||
| export const VirtualizedList = memo( |
There was a problem hiding this comment.
Unfortunatelly, memo() and forwardRef() eat the generic. This can be bypassed with typecast, but that kills passing of the ref. I think it should be possible to create some type helper for it, altough I am not sure if it makes sense here to dive that deep.
There was a problem hiding this comment.
Pls advice, if you have any idea why direct passing of the ref prop doesn't work and I am forced to use the forwardRef again, thanks
There was a problem hiding this comment.
This could have been, it, but it doesnt matter anymore.
| onScroll?: (e: Event) => void; | ||
| onScrollEnd: () => void; | ||
| listHeight: number | string; | ||
| listMinHeight: number | string; | ||
| ref?: React.Ref<HTMLDivElement>; |
There was a problem hiding this comment.
Unfortunatelly, due to typecast, this needs to be here to prevent TS errors.
There was a problem hiding this comment.
Oh yeah, forwardRef will be deprecated https://react.dev/reference/react/forwardRef
There was a problem hiding this comment.
yeah, this could have been better solution.
|
🚀 Expo preview is ready!
|
vojtatranta
force-pushed
the
refactor-virtualized-list
branch
2 times, most recently
from
9e94a54 to
b51c7e1
Compare
vojtatranta
force-pushed
the
refactor-virtualized-list
branch
from
b51c7e1 to
1f876c0
Compare
… by memoizing VirtualizedList while mantaining type safety
…n VirtualizedList component
vojtatranta
force-pushed
the
refactor-virtualized-list
branch
from
1f876c0 to
edacb7c
Compare
| @@ -22,6 +24,7 @@ | |||
| "@suite-common/suite-constants": "workspace:*", | |||
| "@suite-common/suite-utils": "workspace:*", | |||
| "@suite-common/validators": "workspace:*", | |||
| "@testing-library/jest-dom": "^6.6.3", | |||
There was a problem hiding this comment.
for some reason, linter forced me to put it here
| } | ||
|
|
||
| // NOTE: typecast here + memo() and forwardRef() because of passing the ref for useShadow() | ||
| export const VirtualizedList = memo( |
TODO
Add tests for:
Description
VirtualizedList didn't have the proper generic types passed on due to forwardRef(). The
forwardRef()is apparantly needed due touseShadow(). I tried to pass therefmanually, but the shad was broken.However, with typecast, types work and shadow too. Also, I added few perf improvements and simplify
Changes
Screenshots: