Refactor virtual list to remove external dependencies#3416
Draft
jpzwarte wants to merge 2 commits into
Draft
Conversation
…plementation - Removed dependency on @tanstack/virtual-core and integrated a custom Virtualizer class. - Updated VirtualizerController to work with the new Virtualizer implementation. - Added comprehensive tests for the Virtualizer functionality, covering element and window scrolling, sticky items, and various options. - Cleaned up and optimized the code structure for better readability and maintainability.
|
Contributor
🕸 Preview deploys |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors @sl-design-system/virtual-list to remove the @tanstack/virtual-core dependency by introducing an in-repo, framework-agnostic Virtualizer implementation, and updates the Lit controller/component, styling, docs, stories, and tests accordingly.
Changes:
- Removed
@tanstack/virtual-corefrom dependencies/lockfile and replaced it with a new customVirtualizer. - Updated
VirtualizerControllerand<sl-virtual-list>to use the new virtualizer, including sticky item support and start-index scrolling. - Added/updated Vitest coverage, Storybook stories, styling, and README documentation for the new behavior and API.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes the @tanstack/virtual-core lock entry to reflect dependency removal. |
| packages/components/virtual-list/package.json | Drops @tanstack/virtual-core from dev/peer dependencies. |
| packages/components/virtual-list/index.ts | Exports the new Virtualizer API. |
| packages/components/virtual-list/src/virtualizer.ts | Adds the new dependency-free virtualizer implementation. |
| packages/components/virtual-list/src/virtualizer.spec.ts | Adds unit tests for the new virtualizer (element/window scroll, sticky, hidden, options). |
| packages/components/virtual-list/src/virtualizer-controller.ts | Reworks the Lit controller to mount/unmount and update options for the new virtualizer. |
| packages/components/virtual-list/src/virtualizer-controller.spec.ts | Updates controller tests to the new API and rendering approach. |
| packages/components/virtual-list/src/virtual-list.ts | Updates the component to absolute-position items, add sticky/start-index support, and expose scroll APIs. |
| packages/components/virtual-list/src/virtual-list.spec.ts | Expands/updates component tests for scrolling, gaps, sticky, hidden, dynamic sizing, etc. |
| packages/components/virtual-list/src/virtual-list.scss | Updates styles for the new wrapper/item positioning model. |
| packages/components/virtual-list/src/virtual-list.stories.ts | Adds/updates stories for start index, sticky, dynamic sizes, and revised rendering. |
| packages/components/virtual-list/README.md | Updates docs to describe the dependency-free approach and new features/APIs. |
Comment on lines
+218
to
+225
| // Anything that affects item positions invalidates the measurements. | ||
| if ( | ||
| !previous || | ||
| previous.count !== this.#options.count || | ||
| previous.gap !== this.#options.gap || | ||
| previous.paddingStart !== this.#options.paddingStart || | ||
| previous.estimateSize !== this.#options.estimateSize | ||
| ) { |
Comment on lines
+293
to
+301
| // Apply the actual scroll after the host has rendered, so the scroll content is tall enough | ||
| // for the scroll position to stick. | ||
| requestAnimationFrame(() => { | ||
| const [target] = this.#getOffsetForIndex(startIndex, 'start'); | ||
|
|
||
| this.#scrollOffset = target; | ||
| this.#applyScroll(target, undefined); | ||
| this.#notify(false); | ||
| }); |
Comment on lines
+305
to
+309
| unmount(): void { | ||
| this.#isMounted = false; | ||
|
|
||
| this.#unsubscribe.forEach(unsubscribe => unsubscribe()); | ||
| this.#unsubscribe = []; |
Comment on lines
+352
to
+361
| // Keep the most recent sticky item above the viewport rendered, so it can be pinned to the | ||
| // top. Also keep any sticky items inside the range (already added above). | ||
| if (getIsSticky) { | ||
| for (let i = start - 1; i >= 0; i--) { | ||
| if (getIsSticky(i)) { | ||
| indexes.add(i); | ||
| break; | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the dependency on
@tanstack/virtual-corewith a custom Virtualizer implementation, enhancing maintainability and performance. Comprehensive tests were added to ensure functionality, including support for sticky items and various scrolling options. Code structure was optimized for better readability.