Trouble with integrating with Vue

[OrkhanAlikhanov]

Hi, I'm running into an issue where Vue updates dom a bit late than the flow expects. Basically the updates happen after run callback returns. It would be great if we had run's signature as something like (items: Item[], done: () => void) => {} where we call done() after dom update.

  new Workflow({
      consumer,
      element: this.$el as HTMLElement,
      datasource: this.dataSource as any,
      run: (items: Item[]) => {
        if (!items.length && !this.dataItems.length) {
          return
        }

        this.dataItems = items /// vue will detect update and patch the dom after some time
        this.$nextTick(() => {
            console.log('dom patched. now vscroll can continue')
            /// done() // something like this would do the job
        })
      },
    })

What do you think?

[dhilt]

@OrkhanAlikhanov Greets and congrats, you are the first issuer of the vscroll repo! Your request seems quite important, but I have one doubt, the point is that the scroller engine has its own asynchronicity related to updating the DOM:

vscroll/src/processes/render.ts

Lines 13 to 15 in b16a4d2

	render.renderTimer = setTimeout(() => {
	render.renderTimer = null;
	if (Render.doRender(scroller)) {

This is the Render process, the heart of the VScroll Workflow. I would agree, it looks not perfect, but it should be enough to catch 1 tick. I wonder if you need more time... We can think about how to control this asynchronicity, something like

 new Workflow({
      element: ...,
      datasource: ...,
      run: ...,
      onRender: done => this.$nextTick(() => done()) // default: done => setTimeout(() => done())
  })

and then replace setTimeout in the Render process:

onRender(() => {
   if (Render.doRender(scroller)) { ...
   ...
})

Also, there should be a way to cancel render, something like onRender: (done, off) => .... This is interesting story. Do you want to try this with me? I'd gladly participate in Vue integration (never dealt with Vue), and it would be nice if we could schedule a 2-hours meeting. What do you think? Send me your time (when available) if you like the idea: [email protected]

[OrkhanAlikhanov]

Hey! Thank you for quick response! That approach seems more appropriate way of solving this. Having said that, I patched the library locally and rendering works, but now I'm running into another issue where reload is inconsistent. Calling reload(n) does result in updating dom with specific nodes but it keeps display: none on some, if not all, item elements on the screen. It turned out to be an issue with Buffer.reset, specifically this.item = [] line. That assignment results in calling run which does not immediately update DOM. I made it await run method to complete rendering and seems to work now. What do you think?

---

Thank you for the meeting offer 🙂, I'd like to dive into this a bit more and get it working with Vue so that I can become more helpful in the Vue integration process

[OrkhanAlikhanov]

I couldn't reproduce late rendering issue except locally, but was able to reproduce reload issue. Would be great if you could take a look at it, maybe my setup is broken. Let me know if you have any questions. https://stackblitz.com/edit/vue-vscroll

[dhilt]

@OrkhanAlikhanov Well, vscroll v1.0.0 have been just released, a new chapter of this story begins, and I'd love to have onRender updates in 1.1.0. Do you have your efforts published somewhere?

You are right with Adapter.reload -> Buffer.reset -> Workflow.run call stack.

vscroll/src/classes/buffer.ts

Lines 44 to 47 in 640abc4

	reset(force: boolean, startIndex?: number): void {
	this.items.forEach(item => item.hide());
	this.pristine = true;
	this.items = [];

Line 45 is responsible for display-none, line 47 forces Workflow.run to be invoked, and as it follows from the doc (run-callback requirements section):

old items that are not in the list should be removed from DOM

Explicit hiding occurs internally, it is synchronous and allows to avoid any blinking. But physical removal is a consumer responsibility, it means when you get [] in Workflow.run, you have to remove all items from DOM by yourself, using items[].element references.

PS Feel free to ping me by email, I'm pretty interested in Vue, though I still know nothing about it 😁

[dhilt]

@OrkhanAlikhanov I looked at your stackblitz. The main problem with reload as far as I see is how Vue detects changes. The following is my assumption, not knowledge. The line :key="item.nodeId" provides vue-template identity, and when you do reload staying at 0 position, the first rows after reload and before reload will be the same from the Vue's point of view, because theirs "key" props will not change. To prove it, you may scroll down before reload (for the first row key to be greater than 0), and then reload will work properly.

The suggestion is quite simple: add reload counter to :key="item.nodeId" value. Something like :key="prefix + item.nodeId", where

prefix = workflow.scroller.adapter.id + '.' + workflow.scroller.adapter.reloadCounter + '.';

or (equivalent)

prefix = workflow.scroller.adapter.reloadId + '.';

This will guarantee key uniqueness across reload/reset.

In addition I would state a few basic points that you should implement in your project:

• Datasource entity should be defined in the App module (this.datasource), not in the Scroll one; instantiate Datasource in the App module and pass it to the Scroll module as a param; it is important due to separation of concerns, and this will let you to extract vue-vscroll module into a separate generic solution (a consumer in terms of vscroll);
• the Scroll module must not expose the Workflow instance to the App module; the Datasource instance (from the previous point) should be enough to make any manipulation (and it would be much-much safer for the end user); like reload: this.datasource.adapter.reload(this.reloadIndex)

[OrkhanAlikhanov]

Thank you for taking a look at it. That prefix will definitely solve it 💪🏽

I think I know why it happens. The way vue renders is through diffing 2 virtual DOMs and patching the differences. Before reload, DOM has everything in place from item 0 to 10 let's say. When we hit reload, this.items.forEach(item => item.hide()); updates each element with display: none and eventually calls run method where we set new the given items as the new data and that triggers rendering. Vue generates new virtual dom, and compares it with the new one. It sees no difference and therefore does not update anything and that display: none sticks around.

Giving :key="prefix + item.nodeId", will solve the problem because when vue diffes the virtual doms, it compares the key value in the first place. Different keys in the virtual doms will force vue recreate the element.

What do you think about vscroll adding display: none but not removing it?

Thank you for your suggestions about only exposing datasource. I have some questions if you don't mind.

1. How should I update data source when my list of items change? Let's say I had 50 items, and then got new data of 25 items and I want to replace the old data. I've seen adapter.replace method, but how can I update maxIndex and startIndex?
2. Is there any way to set some scroll offset value for the reload method? Currently what I do is call reload and await isLoading$ to be false, then update window.scrollTop with correct offset. The reason I need of set is because I have a sticky header.

I'm just dumping my experience here to give you some ideas based on my usage. Not trying to force anything into the library 🙂 Thank you for the awesome work!

[dhilt]

@OrkhanAlikhanov 👍 That's it. On hiding/removal please refer to this comment. In two words: a consumer must clean up the DOM, but the Scroller is not sure if it will happen immediately and synchronously hides the elements to be removed. Questions:

1. This depends on the data flow you suppose to have in the end App

• linear expanding of the datasource could be easily achieved via Adapter append/prepend methods; they maintain boundaries and also allow to expand datasource in virtual area
• any changes over current Scroller's Buffer (non-virtual items) should be done via Adapter.update (or insert/replace); Adapter append/prepend/remove also can work with Buffer; they all maintain boundaries
• Adapter.remove allows to remove virtual (non-buffered) items by indexes
• total replacement should be accomplished with Adapter reload/reset; "reset" allows to provide new Datasource implementation with new settings
• max/min indexes values can be changed via Adapter.fix API (see AdapterFixOptions); this is helpful if you need to insert some items in the virtual area before edges, as this case is the only limitation of the common Adapter methods

2. Adapter.reload just reloads the viewport. The following I would use

async doReload(index: number, scrollPosition?: number) {
  await this.datasource.adapter.reload(index);
  if (scrollPosition !== void 0) {
    this.datasource.adapter.fix({ scrollPosition });
  }
}

[OrkhanAlikhanov]

Thank you for your help! You did help me a lot. Here is the state so far https://stackblitz.com/edit/vue-vscroll

reload does not return a promise, why did you await it above?

When the given vscroll data set changes, UI should be updated accordingly. There can be 2 types of change:

1. Array content is the same but only deep object change. like [{id: 1, checked: false}, {id: 2, checked: true}] becomes [{id: 1, checked: true}, {id: 2, checked: false}]
2. A more complex change like [{id: 1}, {id: 2}] becomes [{id: 2}, {id: 1}] or [{id: 3}, {id: 4}]

For the second case, vscroll has to reload.
For the first case, there is no need to remove and re-add dom elements which causes flashing, we can just let vue update those items on the screen. I tried adapter.update but it caused flashing issue (because it was adding display: none), so I resorted to setting this.dataItems[].data to new data which updates dom. The way I detect the first case is by generating a string based on uniqueKey value. identity([{id: 1}, {id: 2}]) gives 1-2:

  watch: {
    items(newItems, oldItems) {
      const identity = (items: any[]) => items.map(x => x[this.uniqueKey]).join('-')

      if (identity(newItems) === identity(oldItems)) {
        /// just update displayed data
        this.dataItems.forEach((item) => {
          item.data = newItems[item.$index]
        })
      } else {
        /// else reload completely
        this.datasource = this.generateDataSource()
        this.workflow.scroller.adapter.reset(this.datasource)
      }
    },
  },

I understand this is not efficient, there are things that can be optimized further like identity function. Even dataItems should not be marked reactive as it has performance cost. But it works nicely for my case.

How does ngx-ui-scroll handle such dataset changes?

[dhilt]

@OrkhanAlikhanov I made some updates in your demo: https://stackblitz.com/edit/vscroll-vue-integration. The main point is that the App data is separated from the Vue Scroller component. I had to expose the Datasource class as a component property, hope this doesn't violate standard practices. Please check if it looks in agreement with Vue way, I'd like to have it as a shareable consumer proto-sample....

Adapter.reload does return promise, it resolves when the Scroller stops after the reload process is started and completed. This vscroll functionality has spec in ngx-ui-scroll repo.

Speaking of in-place updates which should not force the rows to fully re-render, I'd suggest to look at Adapter.fix({ updater }) API. It provides access to buffered items and allows modifications without updating the Buffer items array reference (and without Workflow.run call). But as I can see, this also works when we make a simple assignment, like in the demo:

  this.items[index].checked = !this.items[index].checked // just don't do `this.items = ...`, the reference must persist

I also removed watch logic (after I failed to figure out where we are getting richer), for me the result works as expected. In ngx-ui-scroll I totally rely on framework's internal mechanisms with only 1 important thing: I use OnPush change detection strategy rather than default (link, info), that's why I have this line in the Workflow.run implementation.