Scroll not updating datagrid properly

exrhizo commented 4 days ago

Bug Report

Steps to Reproduce:

I haven't created a full reproduction yet, I may need to switch tasks from this

Upgraded from (read context below)

@finos/perspective": "2.5.1" -> "2.10.1"
@finos/perspective-viewer: "2.5.1" -> "2.10.1"
@finos/perspective-viewer-d3fc: "2.5.1" -> "2.10.1"
@finos/perspective-viewer-datagrid: "2.0.1" -> "2.10.1"

Expected Result:

The scroll in a datagrid to work

Actual Result:

The scroll mostly doesn't change the datagrid view, see this loom where I am scrolling left-right and then up-down.

https://www.loom.com/share/92d60335dac94001b292828e07682e4a?sid=fdcb37f1-c35f-4552-8221-8904d8a1dc97

Environment:

Vite dev (esbuild)
@apache-arrow/esnext-esm: "^16.1.0"
React in an iframe
table from arrow: perspectiveWorker.table(cloneArrayBuffer(data))
Chrome 125.0.6422.78

Additional Context:

This was working, and I upgraded perspective, arrow and vite at the same time - so I can do more to bisect the actual root cause. But hoped there might be an idea y'all have.

texodus commented 4 days ago

This looks like it is related to the wide column Source_Port and our auto-column sizing, but I cannot name anything that has changed substantially from 2.5.1 -> 2.10.1 with regard to this feature. A repro would help a lot - you can try Export To Html (Click Export -> select .html format) to get a copy of your app's data in a perspective if you're able to share it.

exrhizo commented 4 days ago

Thank you for looking at this, so far:

Export to html -> it works
When I hit sort -> it works

Here is exported html: reproduction I tried to mirror the CSS, and I may have missed something but that didn't recreate the issue yet..

Another exported html, where there isn't a wide column, and it has the same behavior in my app, but works as html.

here is .mov video, without the wide column: https://github.com/finos/perspective/assets/4206097/16f08109-4b92-4590-804c-8d131fc9cf0d

exrhizo commented 4 days ago

Okay - I created a reproduction repo!

(Maybe to do with how I configured perspective..)

(edit:)

repo
npm install
npm run dev

texodus commented 3 days ago

This isn't a sufficient repro. Please refer to this detailed description of a repro from @Rich-Harris, specifically points (3) and (4).

exrhizo commented 3 days ago

I added instructions and made it more minimal

texodus commented 3 days ago

This line is the issue.

The load() method is async and you don't await it - instead, this element re-renders multiple times before this call resolves (due to multiple missing dependencies in the surrounding context), causing the load() call to be called 3x times (in my browser) while it is still running.

<perspective-viewer> is a stateful component and virtually every method on it is asynchronous. Your must not call these methods multiple times without awaiting their results - and you shouldn't call them multiple times at all if you care about performance, you are making it 3x worse by doing this.

This modified example from the repo (without React) has the same issue by just calling load() twice without awaiting

finos / perspective