Fix memory crash when creating record in table view#16984
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
1 issue found across 1 file
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:102">
P2: `Math.min(...dataPagesLoaded)` will return `Infinity` and `Math.max` will return `-Infinity` when `dataPagesLoaded` is an empty array (which is the default value). Consider adding an early return when the array is empty to make the intent explicit and avoid relying on implicit behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
.../object-record/record-table/virtualization/hooks/useResetVirtualizationBecauseDataChanged.ts
Outdated
Show resolved
Hide resolved
.../object-record/record-table/virtualization/hooks/useResetVirtualizationBecauseDataChanged.ts
Outdated
Show resolved
Hide resolved
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:32909 This environment will automatically shut down when the PR is closed or after 5 hours. |
|
After discussing it with @charlesBochet, we choose to remove individual row loading states resetting. |
prastoin
left a comment
There was a problem hiding this comment.
As discussed should also be reviewed by @lucasbordeau
We might wanna be iterating over the displayed data total count instead of none instead of all of them in db previously
Sounds good to me though !
|
Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
1 similar comment
|
Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you! |
**Bug Fixed**
Creating a new record in the People table view caused a browser memory
crash ("Paused before potential out of memory crash").
**Root Cause**
In useResetVirtualizationBecauseDataChanged.ts, when creating a record,
a loop iterated from 0 to totalNumberOfRecordsToVirtualize (the total
database count).
**Fix Applied**
Changed the loop to only iterate through indices from actually loaded
pages
Fixes #16980
Bug Fixed
Creating a new record in the People table view caused a browser memory crash ("Paused before potential out of memory crash").
Root Cause
In useResetVirtualizationBecauseDataChanged.ts, when creating a record, a loop iterated from 0 to totalNumberOfRecordsToVirtualize (the total database count).
Fix Applied
Changed the loop to only iterate through indices from actually loaded pages
Fixes #16980