Perf: 번들 사이즈 최적화 #30
Perf: 번들 사이즈 최적화 #30
Conversation
Summary of ChangesHello @976520, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 주요 컴포넌트에서 프라이빗 클래스 필드를 채택하여 코드 캡슐화를 개선하고 번들 크기를 최적화하는 데 중점을 둡니다. 이를 위해 빌드 구성이 업데이트되었으며, 특히 프라이빗 필드 지원을 위한 코어 패키지의 타겟 버전 조정과 여러 패키지에서 외부 의존성 처리가 명확해졌습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📊 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
이 PR은 TypeScript의 private 멤버를 네이티브 ECMAScript private 필드(#)로 대체하여 번들 사이즈를 최적화합니다. 이는 더 나은 코드 압축(minification)을 가능하게 하고 캡슐화를 개선하는 훌륭한 변경입니다.
FixedLayoutStrategy, VirtualScrollSource, Virtualizer 전반에 걸쳐 일관되게 변경 사항이 적용되었습니다. 이 문법을 지원하기 위해 core 패키지의 빌드 타겟이 es2022로 올바르게 업데이트되었습니다.
또한 react 및 react-native 패키지에서 @scrolloop/core와 @scrolloop/shared가 external로 처리되도록 변경된 점도 확인했습니다. 이는 의존성을 번들링하지 않고 애플리케이션의 번들러에 위임하는 좋은 아키텍처 개선입니다.
이러한 변경으로 인한 한 가지 중요한 점은, @scrolloop/react 또는 @scrolloop/react-native를 사용하는 프로젝트에서는 이제 node_modules 내의 ES2022 문법(특히 @scrolloop/core의 코드)을 처리할 수 있는 빌드 환경이 필요하다는 것입니다. 이는 일부 사용자에게는 breaking change가 될 수 있으므로 릴리스 노트에 명시하는 것이 좋겠습니다.
코드 품질과 타입 안정성을 더욱 향상시키기 위해, 생성자에서만 할당되는 필드들을 readonly로 표시하도록 몇 가지 제안을 추가했습니다. 전반적으로 훌륭한 성능 및 아키텍처 개선이라고 생각합니다.
| #count: number; | ||
| #overscan: number; | ||
| #plugins: Plugin[]; | ||
| #onChange?: (state: VirtualizerState) => void; | ||
|
|
||
| private layoutStrategy: LayoutStrategy; | ||
| private scrollSource: ScrollSource; | ||
| #layoutStrategy: LayoutStrategy; | ||
| #scrollSource: ScrollSource; | ||
|
|
||
| private state: VirtualizerState; | ||
| private unsubscribe?: () => void; | ||
| #state: VirtualizerState; | ||
| #unsubscribe?: () => void; |
There was a problem hiding this comment.
생성자에서만 할당되고 이후에 변경되지 않는 여러 필드들(#overscan, #onChange, #layoutStrategy, #scrollSource, #unsubscribe)이 있습니다. 불변성을 강제하고 의도를 명확히 하기 위해 이들을 readonly로 선언하는 것이 좋습니다.
| #count: number; | |
| #overscan: number; | |
| #plugins: Plugin[]; | |
| #onChange?: (state: VirtualizerState) => void; | |
| private layoutStrategy: LayoutStrategy; | |
| private scrollSource: ScrollSource; | |
| #layoutStrategy: LayoutStrategy; | |
| #scrollSource: ScrollSource; | |
| private state: VirtualizerState; | |
| private unsubscribe?: () => void; | |
| #state: VirtualizerState; | |
| #unsubscribe?: () => void; | |
| #count: number; | |
| readonly #overscan: number; | |
| #plugins: Plugin[]; | |
| readonly #onChange?: (state: VirtualizerState) => void; | |
| readonly #layoutStrategy: LayoutStrategy; | |
| readonly #scrollSource: ScrollSource; | |
| #state: VirtualizerState; | |
| readonly #unsubscribe?: () => void; |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
📊 Test Coverage Report (vitest)
|
📊 Test Coverage Report (vitest)
|
📊 Test Coverage Report (vitest)
|
|
/gemini review |
📊 Test Coverage Report (vitest)
|
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on optimizing bundle size and performance by adopting true private class fields (#) in TypeScript files across core and react packages, replacing private keywords. It also introduces micro-optimizations for integer truncation using the bitwise OR operator (| 0) in several calculation functions. For server-side rendering (SSR) in InfiniteList.tsx, the handling of initial data has been refactored for better memoization and conciseness. The useTransition hook and domPruner utility in the react package have been refactored for brevity and to use requestIdleCallback more directly. Build configurations (tsup.config.ts files) were updated to increase terser compression passes, add more aggressive unsafe terser options, and adjust target and external dependencies. A review comment suggests aligning terserOptions.compress.ecma with the es2022 target to fully leverage private field syntax benefits and avoid potential bundle size increases. Another comment recommends reverting a change in Virtualizer.ts that calculates renderRange using if conditions, proposing to use Math.max and Math.min for better readability and conciseness, as the | 0 operator is unnecessary for integer inputs.
| let renderRange = { | ||
| startIndex: Math.max(0, visibleRange.startIndex - this.overscan), | ||
| endIndex: Math.min(this.count - 1, visibleRange.endIndex + this.overscan), | ||
| startIndex: (visibleRange.startIndex - this.#overscan) | 0, | ||
| endIndex: (visibleRange.endIndex + this.#overscan) | 0, | ||
| }; | ||
|
|
||
| for (const plugin of this.plugins) { | ||
|
|
||
| if (renderRange.startIndex < 0) renderRange.startIndex = 0; | ||
| if (renderRange.endIndex > this.#count - 1) | ||
| renderRange.endIndex = this.#count - 1; |
There was a problem hiding this comment.
renderRange를 계산하는 방식이 Math.max와 Math.min을 사용하는 이전 코드에 비해 더 장황하고 가독성이 떨어집니다. visibleRange.startIndex와 overscan이 모두 정수이므로 | 0 연산은 불필요하며, if 조건문은 Math.max/Math.min과 동일한 역할을 합니다. 코드의 명확성과 간결성을 위해 이전 방식으로 되돌리는 것을 제안합니다.
| let renderRange = { | |
| startIndex: Math.max(0, visibleRange.startIndex - this.overscan), | |
| endIndex: Math.min(this.count - 1, visibleRange.endIndex + this.overscan), | |
| startIndex: (visibleRange.startIndex - this.#overscan) | 0, | |
| endIndex: (visibleRange.endIndex + this.#overscan) | 0, | |
| }; | |
| for (const plugin of this.plugins) { | |
| if (renderRange.startIndex < 0) renderRange.startIndex = 0; | |
| if (renderRange.endIndex > this.#count - 1) | |
| renderRange.endIndex = this.#count - 1; | |
| let renderRange = { | |
| startIndex: Math.max(0, visibleRange.startIndex - this.#overscan), | |
| endIndex: Math.min(this.#count - 1, visibleRange.endIndex + this.#overscan), | |
| }; |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
📊 Test Coverage Report (vitest)
|
Private Field 사용