Skip to content

perf: virtualize the notification list - #3319

Open
fmguerreiro wants to merge 1 commit into
gitify-app:mainfrom
fmguerreiro:perf/virtualize-notification-list
Open

fmguerreiro wants to merge 1 commit into
gitify-app:mainfrom
fmguerreiro:perf/virtualize-notification-list

Conversation

@fmguerreiro

@fmguerreiro fmguerreiro commented Sep 17, 2026

Copy link
Copy Markdown

Every row in the inbox mounts, so a large inbox paints tens of thousands of DOM nodes and re-renders all of them on each poll.

Virtualizes the list. Accounts, groups and rows flatten into one sequence, so one virtualizer windows across group boundaries. Rows unmount on scroll, so collapse and exit-animation state moved up to NotificationList; headers split out as AccountHeader and RepositoryHeader.

main this branch
1274 rows mounted, 56,562 DOM nodes 16 rows mounted, 848 DOM nodes
main branch

Row heights are estimated, then corrected by measurement, so the scrollbar settles while scrolling.

The cache write fix that was the first commit here is now #3320.

@fmguerreiro
fmguerreiro marked this pull request as ready for review September 17, 2026 07:55
@github-actions github-actions Bot added dependency Dependency updates refactor Refactoring of existing feature labels Sep 17, 2026
@afonsojramos

Copy link
Copy Markdown
Member

I've faced this freezing issue this week too; it might have been a recently introduced bug.
But this is awesome! I've been wanting to test out virtualized lists for a while. HOWEVER, tanstack's implementation is not the most performant one. I'd argue that LegendList does the job much better: https://www.legendapp.com/open-source/list/v3/react/examples/virtual-list-comparison/

@afonsojramos

Copy link
Copy Markdown
Member

In fact, the freezing was probably what you fixed in 6320175 (#3316)?

@setchy

setchy commented Sep 17, 2026

Copy link
Copy Markdown
Member

In fact, the freezing was probably what you fixed in 6320175 (#3316)?

Shall we pull this out to a separate PR

One repository can hold hundreds of notifications, so accounts, repository groups and rows are flattened into a single item sequence and windowed across group boundaries. Headers no longer own their rows, so collapse and exit-animation state moves up to the list and survives rows unmounting on scroll.
@fmguerreiro

fmguerreiro commented Sep 18, 2026

Copy link
Copy Markdown
Author

pulled the cache write fix out to #3320, that one is the mark-as-read freeze.

for trying out LegendList, benched both on the same 1274-notification list:

┌───────────────────────┬───────────────┬─────────────┐
│                       │ react-virtual │ LegendList  │
├───────────────────────┼───────────────┼─────────────┤
│ frame p50             │ 16.7ms        │ 16.7ms      │
├───────────────────────┼───────────────┼─────────────┤
│ frame p95             │ 33.4–66.6ms   │ 33.2–33.4ms │
├───────────────────────┼───────────────┼─────────────┤
│ frames >50ms per pass │ 0–14          │ 0           │
├───────────────────────┼───────────────┼─────────────┤
│ rows mounted at rest  │ 11            │ 15          │
└───────────────────────┴───────────────┴─────────────┘

happy to swap, one catch: legend measures nothing in happy-dom, no layout and its ResizeObserver never fires, so the list renders zero rows there. the windowing tests would have to move to a real browser. ok with that?

@setchy

setchy commented Sep 18, 2026

Copy link
Copy Markdown
Member

Thanks @fmguerreiro.

I'd be leaning to keeping within the tanstack family for "simplicity". It's a positive improvement from current state

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependency Dependency updates refactor Refactoring of existing feature

Development

Successfully merging this pull request may close these issues.

3 participants