Skip to content

expose firstIndex in virtual scroller#823

Open
lygaret wants to merge 5 commits into
solidjs-community:mainfrom
lygaret:expose-virtual-indices
Open

expose firstIndex in virtual scroller#823
lygaret wants to merge 5 commits into
solidjs-community:mainfrom
lygaret:expose-virtual-indices

Conversation

@lygaret

@lygaret lygaret commented Dec 1, 2025

Copy link
Copy Markdown

I'd like to be able to use a virtual list with an <ol> element, since that's semantically correct for my application.

Currently, in order to correctly set the start attribute, I'm currently dividing viewTop by rowHeight, which is just how it's defined, and lets me get at the value correctly, but it's a very ugly hack to get the value which is just hidden under the lambda.

In use, this looks like:

  const [virtual, virtualOnScroll] = createVirtualList({
    items: entries,
    overscanCount: 5,
    rootHeight,
    rowHeight
  })

  return (
    <div class={styles['list-wrapper']} onScroll={virtualOnScroll}>
      <ol start={virtual().firstIndex} style={{top: `${virtual().viewerTop}px`}}>
        <For
          each={virtual().visibleItems}
          fallback={<li class={styles['list-empty-marker']} />}
        >
        /* whatever ... */
      </ol>
    </div>
  )

Summary by CodeRabbit

  • New Features

    • Virtual lists now expose firstIndex and lastIndex, making it easier to track the rendered item range.
    • lastIndex follows inclusive index behavior and is undefined for empty lists.
  • Tests

    • Expanded coverage to verify the new index values across scrolling, overscan, empty lists, and single-item lists.

lastIndex is actually _exclusive_, which would require math, and I don't
feel like it's likely to be as useful as firstIndex
@changeset-bot

changeset-bot Bot commented Dec 1, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: a3f87c1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@solid-primitives/virtual Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

containerHeight: items.length * rowHeight,
viewerTop: firstIndex * rowHeight,
visibleItems: items.slice(firstIndex, lastIndex) as unknown as T,
firstIndex,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while you're at it, you can also expose lastIndex.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can. I did originally, but removed it because its a little weird semantically: when a list is empty, should it return -1, or undefined?

firstIndex being zero makes sense, kinda, but I didn't know what to do with last's typing, so I skipped it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, it's more like nextIndex than last because it's an exclusive range.

This comment was marked as duplicate.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine, too.

@lygaret

lygaret commented Dec 2, 2025

Copy link
Copy Markdown
Author

@atk I ended up going with lastIndex: number | undefined -- semantically, it's undefined for an empty list, though I'm not allowing firstIndex to be undefined, and returning 0 in that case, maybe that's wrong?

nextIndex also has a weird semantic, but imo nextIndex: undefined means more like "there's no more of the list", which might be a better thing to expose?

let me know your thoughts and I'll move that direction, thank you!

visibleItems: items.slice(firstIndex, lastIndex) as unknown as T,
firstIndex,
lastIndex: lastIndex > 0 ? lastIndex - 1 : undefined,
// -1 because slice is an exclusive range

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be items.length if it is -1?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I'm not sure what you're asking.

The -1 is an explanation of the reason we're not just passing it in straight, and it's already based on items.length, hence the whole thing ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I read the whole thing in a different order. My bad, I'm currently down with a flu.

@atk

atk commented Dec 2, 2025

Copy link
Copy Markdown
Member

Now all that is left to do is add a changeset using npx changeset.

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c9f24518-c87d-4f53-9f7a-5693f8af4ccd

📥 Commits

Reviewing files that changed from the base of the PR and between bddbafc and a3f87c1.

📒 Files selected for processing (3)
  • .changeset/shaky-chicken-marry.md
  • packages/virtual/src/index.tsx
  • packages/virtual/test/index.test.tsx

📝 Walkthrough

Walkthrough

Adds firstIndex and lastIndex fields to the createVirtualList return value in the virtual package, replacing internal helper index calculations with inline computation. Includes expanded test coverage for these fields and a changeset documenting the patch.

Changes

Virtual list index exposure

Layer / File(s) Summary
Type and inline computation of firstIndex/lastIndex
packages/virtual/src/index.tsx
Extends VirtualListReturn with firstIndex: number and lastIndex: number | undefined; replaces getFirstIdx/getLastIdx helpers with inline calculations from offset(), rowHeight, rootHeight, and overscanCount, with lastIndex becoming undefined when computed end is 0.
Test coverage for new indices
packages/virtual/test/index.test.tsx
Adds assertions for firstIndex/lastIndex across initial render, scroll updates, overscan, and empty/singleton list edge cases.
Changeset entry
.changeset/shaky-chicken-marry.md
Adds a patch changeset describing the new exposed indices.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change by describing the new firstIndex exposure in the virtual scroller.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@davedbase

Copy link
Copy Markdown
Member

@lygaret would love to get this merged! Can you apply a changeset for us? Once done I'll migrate these to the new Solid Primitives 2.0 primitives as well.

@davedbase davedbase added the enhancement New feature or request label Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants