vue-virtual-scroller
vue-virtual-scroller copied to clipboard
Akryum
Rewrite view (re-)assignment logic
Description (from primary commit)
This commit rewrites the view (re-)assignment logic in a way that is shorter and therefore easier to read through. The performance is generally on par with the old implementation. It also fixes bugs in the old logic where:
- The item itself getting used as the keyField instead of the index.
- Sometimes a view could get stuck in a corrupted state where its key doesn't match what the index or item indicates.
These bugs were rather hard to reason about in the old logic due to the redundant code path it had (which weakened the invariants). The new logic unifies paths for simplicity, while keeping the time complexity aspect roughly the same.
Testing
I've done some brief tests on this using my own app and it has been working fine, even better because there were some annoying bugs before as described above.
Not all code paths are exercised in my app though, so additional testing would be useful.
Reviewing
This PR is split into a bunch of small refactoring commits that should be trivial to review, and a main commit (fix(RecycleScroller): Rewrite view (re-)assignment logic) after all the refactoring is done. It should be easier to review commit by commit.
@Akryum This resolves most of the critical issues I encountered when using this package in my project. Would appreciate a review!
We're longer setting the
typeand theusedanymore? Confused by this change.
type is only set once on view creation, the point is it should not change during the recycling process or it breaks the assumption of how the recycled views are stored.
Similarly for used, they only change when a recycled view get displayed or vice versa.
@patchthecode Wonder if you had a chance to try this PR? Does it solve your "stale data" issue?
I was not able to test it because i think i am using a later vue version?
i ran this command
npm i https://github.com/ishitatsuyuki/vue-virtual-scroller
and here was my results
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/vue
npm ERR! vue@"^3.2.37" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer vue@"^2.6.11" from [email protected]
npm ERR! node_modules/vue-virtual-scroller
npm ERR! vue-virtual-scroller@"https://github.com/ishitatsuyuki/vue-virtual-scroller" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR! See /Users/zuaaef/.npm/eresolve-report.txt for a full report.
npm ERR! A complete log of this run can be found in:
npm ERR! /Users/zuaaef/.npm/_logs/2022-08-27T12_46_28_826Z-debug-0.log
npm i https://github.com/ishitatsuyuki/vue-virtual-scroller
That won't work because source repos doesn't contain the dist artifacts.
I suggest cloning the repo to somewhere, npm build then reference that as a path dependency.
for reference, the vue-virtual scroller i used was the one compatible with vue3 as instructed by the repository owner --> https://github.com/Akryum/vue-virtual-scroller/tree/next/packages/vue-virtual-scroller
Sorry but my problem is that i am new to NPM, and i do not know how to install from a github repository
npm install --save "https://github.com/ishitatsuyuki/vue-virtual-scroller#logic-rewrite"
^C(##################) ⠧ reify:esbuild-android-64: timing reifyNode:node_modules/esbuild-android-arm64 Compl
npm ERR! git dep preparation failed
npm ERR! signal SIGINT
npm ERR! command /Users/t/.nvm/versions/node/v16.13.0/bin/node /Users/zuaaef/.nvm/versions/node/v16.13.0/lib/node_modules/npm/bin/npm-cli.js install --force --cache=/Users/zuaaef/.npm --prefer-offline=false --prefer-online=false --offline=false --no-progress --no-save --no-audit --include=dev --include=peer --include=optional --no-package-lock-only --no-dry-run
npm ERR! npm WARN using --force Recommended protections disabled.
npm ERR! npm WARN ERESOLVE overriding peer dependency
npm ERR! npm WARN While resolving: [email protected]
npm ERR! npm WARN Found: [email protected]
npm ERR! npm WARN node_modules/eslint-plugin-promise
npm ERR! npm WARN dev eslint-plugin-promise@"^5.1.0" from the root project
npm ERR! npm WARN
npm ERR! npm WARN Could not resolve dependency:
npm ERR! npm WARN peer eslint-plugin-promise@"^4.2.1" from [email protected]
npm ERR! npm WARN node_modules/eslint-config-standard
npm ERR! npm WARN dev eslint-config-standard@"^16.0.2" from the root project
npm ERR! npm WARN
npm ERR! npm WARN Conflicting peer dependency: [email protected]
npm ERR! npm WARN node_modules/eslint-plugin-promise
npm ERR! npm WARN peer eslint-plugin-promise@"^4.2.1" from [email protected]
npm ERR! npm WARN node_modules/eslint-config-standard
npm ERR! npm WARN dev eslint-config-standard@"^16.0.2" from the root project
i only know how to install from npm it self
I said you should clone the repo to some path and then use a path dependency, not a git dependency.
I'll have to check this another time then. my lack of understanding of npm and my very flakey setup is making this take some time to install properly. And i'm really low of time right now.
I'll have to revisit this in the future. In the mean time, i have forcefully reloaded by virtual scroller as a work around.
@patchthecode To verify this code, you need to build the source code and use it. The following is the general process for reference:
- Clone this repository using Git
- Use
git checkoutto switch the branch tologic-rewrite - Build according to the build command described by
scriptsinpackage.json - Use built local dependencies
Any chance that this can be reviwed and merged? I'm having the issue of the index being out of date after the list we pass to the virtual-scroller is updated a couple of times... @ishitatsuyuki @zsdycs
