Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bi-Directional Infinite Scrolling Pattern Limitations #293

Open
johncalvinyoung opened this issue May 18, 2018 · 7 comments
Open

Bi-Directional Infinite Scrolling Pattern Limitations #293

johncalvinyoung opened this issue May 18, 2018 · 7 comments

Comments

@johncalvinyoung
Copy link

So as I work on a branch for #292 (and the feature that gave rise to it) I've run into some thought problems.

I'm working with an app using non-linear paging (keyset pagination rather than limit&offset), heavily using the new subclass of InfinityModel pattern, and the next feature is likely to use bidirectional loading with two InfinityLoader instances, as outlined in the readme.

  • It would be good to let InfinityModel once instantiated have a link to the infinity service, so it can automatically request additional pages of results in certain scenarios.
  • afterInfinityModel and infinityModelLoaded always presume that loading appending to the set. It'd be good to pass direction (and track canLoadMore separately, perhaps) for 'up' as well as 'down', for the bidirectlonal-loading case. Maybe there are no more results 'after' the current point, but there might be some 'before'.

Anyone have thoughts on the above? Some of this would affect interfaces somewhat...

@snewcomer
Copy link
Collaborator

@johncalvinyoung

  1. Now that we have the infinity service, does that solve your use case? Basically can be injected anywhere now.
  2. Do you have a concrete example of this? You have to have afterInfinityModel when loading the records from the page before? I had thought _afterInfinityModel was always called, no matter which direction.

Would love to help/collaborate on any work you may have up for the problem you are solving.

Have this existing PR so far - #287

@johncalvinyoung
Copy link
Author

@snewcomer

  1. The infinity service actually cannot be injected into the extended InfinityModel. Some complaint about not having a container. I ended up dropping it at that point and just setting it on the model in the first _afterInfinityLoad hook for now. Would be better when the infinity service sets up the model.

  2. I'm using a highly-modified form of the 'cursor pagination' from the readme. This logic is used when you cannot extrapolate the availability of records from a (not present) page number or total page number. I'm in the middle of implementing bidirectional infinite scrolling, and I'll need to track 'oh you reached the end of the result set' ==> .set('canLoadMore', false) differently from 'oh, you reached the beginning of the result set' ==> set('canLoadMoreBefore', false)? In this specific scenario, we'll be sometimes launching the route with some initial parameters to start loading at a specific time or sequence number in the middle of a very long set or queue.

  afterInfinityModel(posts, infinityModel) {
    let loadedAny = posts.get('length') > 0;
    infinityModel.set('canLoadMore', loadedAny);
  }

I'll see if I can push up the hook work I was doing this afternoon. I'm pretty sure I've got the semantics of some of the promises wrong, though. And I'm struggling a bit with extending the test suite in the library--very different patterns/techniques than my projects have.

@johncalvinyoung
Copy link
Author

PS: I've been reading #287 as it goes along--it'll be good improvements when we get there, as long as those of us who don't use paged APIs can still get along!

@snewcomer
Copy link
Collaborator

@johncalvinyoung

  1. Does this commit help?

  2. The goal of 1.0 is to be extensible as possible. If it is too difficult with the existing apparatus, we should brainstorm on a new way to handle cursor pagination.

@johncalvinyoung
Copy link
Author

johncalvinyoung commented May 19, 2018

If that commit works I'll be surprised. That's basically exactly what I attempted to do in my app, in the subclass.

Edited to note: the exception was thrown once I attempted to use the service inside the class.

@johncalvinyoung
Copy link
Author

Update: I'm using 1.2.4 now as I picked back up my feature branch in progress. Lots of good changes there.

However, to combine bidirectional loading (which can even be concurrent) with cursor/keyset pagination correctly required a number of adaptations to both the infinityModel and infinityService classes (subclassed them in our app). I'll be looking at our changes to figure out if there's a good abstraction I can push back upstream. One of the key concepts is that we can't rely on a single currentPage or _increment attribute on the infinityModel, as the various hooks may update those values out of order in a race condition scenario. Passing increment around into most method calls instead of relying on state in the infinityModel helped. And the concepts of _canLoadMore and _loading and reachedInfinity are all more-or-less unidirectional in nature, and for optimum extensibility ought to be split into before/after above/below forms in the superclass.

@johncalvinyoung
Copy link
Author

Ooh. Next problem. Race condition on _increment will result in the previous page loading at the end, or vice versa. I think this would affect numbered APIs as well as keyset-paginated, if loading bidirectionally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants