using {virtual_mode: "vertical"} and "none" causes containing web page to fail to load

[telamonian]

Bug Report

Steps to Reproduce:

I'm trying out the new virtual_mode option for setDataListener:

https://github.com/jpmorganchase/regular-table/blob/6dbee4ffe1b3c1342ae3903a39456aa9d5131cda/src/js/index.js#L272-L279

The both and horizontal options seem to work fine, but trying to use either vertical or none causes the webpage containing the <regular-table> to fail to load.

Expected Result:

vertical and none functioning as described in #125

Actual Result:

Before that error pops up, the page will try to load for about 30 sec (while displaying only a white screen, with no loaded sources visible in devtools).

Environment:

regular-table@0.3.0

Additional Context:

This bug is happening in a regular table included in my <tree-finder-panel> element

[telamonian]

I'm able to see part of the problem by calling .setDataListener(..., {virtual_mode: "vertical"}) directly in the console after my grid has already loaded normally:

So it looks like somehow somewhere end_col is getting calculated as infinite when vertical mode is turned on. Which would likely explain why the containing page hangs on initial load

[texodus]

end_col is infinite when virtual scrolling is disabled (specifically it is set to Infinity) - there is no "smaller" alternative that is correct. One option is to supply num_cols here, but it somewhat breaks the API contract that separate page requests are "clean state" draw calls (as data may change between 2 calls to a data listener); we could simply omit end_col, but this will also error in naive data models.

I'm not against applying the former if this is a common error case, but pendantically I'll note that a data model that returns rows "outside" of its data bounds isn't really correct either, regardless of the window that regular-table asks the data listener for.

[telamonian]

I'll note that a data model that returns rows "outside" of its data bounds isn't really correct

@texodus That sounds right. Yeah, I have previously noticed cases where end_col will end up being around 2x the maximum specified by the data model. And yep, my data model is pretty dumb, it fetches data as:

const val = formatter(content.row[column]);

So when column is greater than content.row.length -1, the data model will return column with junk undefined entries.

but pendantically

So i do think you're right, but from a usability perspective it would be nice if regular-table can do the window range clamping for the user. For a user, it is nice to be able to express your data model simply, without extra instrumentation. And as you mentioned elsewhere, there are going to be a number of complex edge cases.

Additionally, since there do seem to be cases where end_col > num_cols regardless of virtual_mode, the generation of spurious [undefined, undefined, ...] data may also be something of a performance concern.

[telamonian]

finally had a sec to play around with this. Previously my code was iterating over a range of col indices:

for (let cix = start_col; cix < end_col - 1; cix++) {
      const column = this.model.columns[cix];

and was sad when end_col===infinity. But then I changed it to go over a slice of col names:

for (const column of this.model.columns.slice(start_col, end_col)) {

and now everything is great!

@texodus So I guess one resolution would be loudly document the wobbliness of end_col alongside the basic "how-to-datamodel" section of the docs, and how it can be dealt with using slice to ensure appropriate truncation. But I'm hoping we can do better

[telamonian]

@texodus Another way to look at it is that the wobbliness of end_col has been a built-in feature since day one, in the service of better flexibility and overall performance. It seems kind of like you were already acknowledging this via the fact that all of your example data models already correctly handle the edge cases that certain values of end_col can cause.

All of which kind of reinforces the idea that this is a docs problem, not a code problem