State.slice() produces slices with undefined values if indices are out of bounds.

[cowboyd]

The standard behavior for slice is to truncate the slice for any values of start and end that are out of bounds, so for example, no matter how you slice an empty array, you still get the empty array:

[].slice(5, 5000) //=> []
['a','b'].slice(1,1000) //=> ["b"]

for the dataset state however, slicing with indices that fall outside the length of the state produces array slots that are all inhabited by undefined

[flexyford]

Thanks @cowboyd!

Sounds like a candidate to fix before a 1.0 release. I'll open a PR with a failing test and tackle this in the first half of this week.

[flexyford]

@cowboyd I was not able to reproduce the issue you're describing. Could you provide more detail?

I've got a branch up with the tests for state.slice https://github.com/flexyford/impagination/compare/task/slice-out-of-bounds

Since an instance of State is an array-like object and we're delegating to the Javascript slice method, I would expect this to truncate. I'm curious about the behavior your seeing.

[cowboyd]

Weird! We had to put in a workaround in to get what we thought was the proper slice behavior https://github.com/folio-org/ui-eholdings/blob/master/src/components/impagination.js#L20-L35

But maybe we were using it wrong 🤔

[flexyford]

Yea from the looks of it, does not seem different than what standard JS slice would be returning.

Using your extended ‘slice’ method as a wrapper to datasetState.slice, i’m curious what the values of ‘start’, ‘end’, ‘datasetState.length’ and ‘result.length’ would be.

[cowboyd]

Maybe @wwilsman could shed some light here since he did the bulk of the implementation.

[wwilsman]

Hey @flexyford! Sorry it took me so long to get around to looking at this!

So the problem is actually not that the array is the wrong length, but that the values are undefined.

state.slice(0, 2) //=> [undefined, undefined]

So this is the problem we were facing:

• We have a long list of records (~10,000) paged at 25 (~400 pages)
• We debounce calling setReadOffset during scroll so that if we quickly scroll half way down the list, we don't end up with ~200 page requests
• While scrolling, we are partially rendering the list viasliceing the state

Because we are waiting to call setReadOffset, sometimes the slice involves records outside of the load horizon (which return undefined via slice):

state = new State({ pageSize: 10, readOffset: 20 });
state.slice(9, 11); //=> [undefined, { page, content, ... }]

After some digging, this looks like it might be intended? After calling setReadOffset with a number within the visible list elements, the slice correctly receives records with isRequested, isPending, etc.

The confusion comes from when we use the stats.totalPages property. For example, with 400 pages at 25 records per page:

let state = new State({ pageSize: 25, stats: { totalPages: 400 } });

expect(state).to.have.lengthOf(10000); 
//=> true

expect(state.slice(5000, 5026)).to.have.lengthOf(25); 
//=> true

// this is the desired behavior
expect(state.slice(5000, 5026)[0]).to.deep.equal(state.getRecord(5000)); 
//=> false

// after we set the read offset, this is true
state = state.setReadOffset(5000);
expect(state.slice(5000, 5026)[0]).to.deep.equal(state.getRecord(5000)); 
//=> true

Since we are debouncing setReadOffset, our patch for slice simply uses getRecord directly instead of relying on state[5000] to exist. If there is a solution we could implement, maybe it would be setting up the getters for all records when totalPages is defined. Otherwise, this might be more of an implementation issue.