Conversation
📊 Test Coverage Report (vitest)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the @scrolloop/preact package, which provides virtualization and infinite scrolling components for Preact. The implementation includes VirtualList, InfiniteList, and a useInfinitePages hook. Review feedback focuses on performance optimizations, specifically recommending the memoization of style objects and the extraction of inline styles into constants. Additionally, it is suggested to exclude the source directory from the published package to minimize its size.
| "dist", | ||
| "src" |
There was a problem hiding this comment.
Including the src directory in the files array increases the size of the published package. For a library, it's common practice to only include the compiled dist folder. Consider removing src unless there's a specific reason to publish the source files.
"dist"| const centerStyle: CSSProperties = { | ||
| height, | ||
| display: "flex", | ||
| alignItems: "center", | ||
| justifyContent: "center", | ||
| }; |
There was a problem hiding this comment.
The centerStyle object is recreated on every render. To optimize performance, you can memoize it using the useMemo hook, as it only depends on the height prop.
| const centerStyle: CSSProperties = { | |
| height, | |
| display: "flex", | |
| alignItems: "center", | |
| justifyContent: "center", | |
| }; | |
| const centerStyle = useMemo<CSSProperties>(() => ({ | |
| height, | |
| display: "flex", | |
| alignItems: "center", | |
| justifyContent: "center", | |
| }), [height]); |
| <div style={{ textAlign: "center" }}> | ||
| <p>Error.</p> | ||
| <p style={{ color: "#666", fontSize: "0.9em" }}>{error.message}</p> | ||
| <button | ||
| onClick={retry} | ||
| style={{ marginTop: 8, padding: "4px 12px", cursor: "pointer" }} | ||
| > | ||
| Retry | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
These inline styles are recreated on each render and use magic values. It's better to extract them into constants for performance and maintainability. Since these styles don't depend on props, they can be defined outside the component.
For example:
const errorWrapperStyle: CSSProperties = { textAlign: 'center' };
const errorMessageStyle: CSSProperties = { color: '#666', fontSize: '0.9em' };
const retryButtonStyle: CSSProperties = { marginTop: 8, padding: '4px 12px', cursor: 'pointer' };
// ... inside the component
<div style={errorWrapperStyle}>
<p>Error.</p>
<p style={errorMessageStyle}>{error.message}</p>
<button onClick={retry} style={retryButtonStyle}>
Retry
</button>
</div>| const containerStyle: CSSProperties = { | ||
| overflow: "auto", | ||
| height, | ||
| ...style, | ||
| }; |
There was a problem hiding this comment.
The containerStyle object is recreated on every render. To improve performance, you can memoize it using the useMemo hook since its dependencies (height and style) are props.
| const containerStyle: CSSProperties = { | |
| overflow: "auto", | |
| height, | |
| ...style, | |
| }; | |
| const containerStyle = useMemo<CSSProperties>(() => ({ | |
| overflow: "auto", | |
| height, | |
| ...style, | |
| }), [height, style]); |
📊 Test Coverage Report (vitest)
|
📊 Test Coverage Report (vitest)
|
1 participant
No description provided.