Refactored table virtualization real index state

[lucasbordeau]

This PR refactors the table virtualization real index main state that was creating performance problems.

The problem was that we kept track of all the real rows even if we only print 200 at a time in the application.

This caused a crash when some users with 10k+ rows on a table tried to iterate over this state 10.000 times or more.

The trade-off is that now we keep all real indices in a Map, and each virtual row uses a selector to go to this Map.

This way if we want to reset the full Map, we just have to overwrite the base state that contains the map, and the 200 selectors will recompute. This happens for example when we delete a row. This now has a limited and negligible performance impact.

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[cubic-dev-ai]

1 issue found across 19 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/twenty-front/src/modules/object-record/record-table/virtualization/hooks/useResetVirtualizationBecauseDataChanged.ts">

<violation number="1" location="packages/twenty-front/src/modules/object-record/record-table/virtualization/hooks/useResetVirtualizationBecauseDataChanged.ts:144">
P2: Incorrect dependency: `scrollWrapperHTMLElement?.clientHeight` should be `scrollWrapperHTMLElement`. The callback closes over the element reference, not the `clientHeight` value. If the element changes but has the same height, this callback won't be recreated and will use a stale element reference.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P2: Incorrect dependency: scrollWrapperHTMLElement?.clientHeight should be scrollWrapperHTMLElement. The callback closes over the element reference, not the clientHeight value. If the element changes but has the same height, this callback won't be recreated and will use a stale element reference.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/record-table/virtualization/hooks/useResetVirtualizationBecauseDataChanged.ts, line 144:

<comment>Incorrect dependency: `scrollWrapperHTMLElement?.clientHeight` should be `scrollWrapperHTMLElement`. The callback closes over the element reference, not the `clientHeight` value. If the element changes but has the same height, this callback won't be recreated and will use a stale element reference.</comment>

<file context>
@@ -43,18 +52,101 @@ export const useResetVirtualizationBecauseDataChanged = (
-      dataPagesLoadedCallbackState,
-      totalNumberOfRecordsToVirtualizeCallbackState,
       findManyRecordsLazy,
+      scrollWrapperHTMLElement?.clientHeight,
+      lastScrollPositionCallbackState,
+      totalNumberOfRecordsToVirtualizeCallbackState,
</file context>

Suggested change

	scrollWrapperHTMLElement?.clientHeight,
	scrollWrapperHTMLElement,

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:17083

This environment will automatically shut down when the PR is closed or after 5 hours.

[twenty-eng-sync]

Hey @lucasbordeau! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

[twenty-eng-sync]

Hey @lucasbordeau! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!