Replace client.scroll with a method on applicable response objects

[dsibilly]

Using client.scroll is kind of awkward because it's effectively a method that allows for retrieving arbitrary JSON responses from any URL. It requires a tiny bit of boilerplate to use, as illustrated in its unit test:

import igdb from 'igdb-api-node';

const client = igdb('API_KEY');

client.games({
    order: 'rating',
    scroll: 1
}).then(response => {
   /* do stuff with the initial response... */

    /* scroll to the next page */
    return client.scroll(response.scrollUrl);
}).then(scrollResponse => {
    /* do stuff with the scroll response */
});

While perform-request is building the response object, it can easily add a scroll method directly to that object that has all the data it needs already baked into it, such that the following becomes possible:

import igdb from 'igdb-api-node';

const client = igdb('API_KEY');

client.games({
    order: 'rating',
    scroll: 1
}).then(response => {
   /* do stuff with the initial response... */

    /*
    Scroll to the next page.
    The response already knows where to go,
    so we don't need to provide any URL at all.
    */
    return response.scroll();
}).then(scrollResponse => {
    /* do stuff with the scroll response */
});

It's a very small change, but it allows the user to completely disregard the API implementation details of scroll URLs, and moves the responsibility for scroll logic from the client to the response itself.

Responses without scrollUrl set will end up having no scroll method, so inspecting for its existence before calling it may be necessary. It's no different from inspecting to make sure that response.scrollUrl is set, so it may not be a big deal. If that proves awkward, though, the method can default to a stub that returns either:

• a rejected Promise with an Error stating that there is no more content to scroll to, or...
• a resolved Promise with an object which has a body property set to false.

There may be better ideas on how to resolve that bit. However, moving scrolling from a generalized client method to a response method seems like a worthwhile change. Keeping the client method around as a deprecated part of the API until v4.0.0 seems prudent to avoid introducing a breaking change and violating semver.

[krazyjakee]

This is a beautiful solution.

I'm trying to think how every page could be obtained in one go (with defined limits). Either an implementation like response.scrollAll({ resultLimit: 300, pageLimit: 5 }).then((results) => {}) or a more advanced example using your implementation. That way the wrapper (or example) handles any special responses in the resolved promise and the developer doesn't have to care.

I'm not sure that a series of .then functions is a realistic use case for the scroll api.

[dsibilly]

That's a good point; if the series of resulting Promises can be abstracted away by the wrapper and yield a more intuitive client API, that's a goal worth pursuing. I'll think on this.

[krazyjakee]

I pushed a crude solution & test to the develop branch.

Sadly you have to provide the endpoint & query as a string to use it. It would be nice to pass an endpoint and the query as a separate object but I'm going to wait for suggestions on this.

Any feedback or comments on this would be useful.

[krazyjakee]

It's been in master subtly for a little while and I've been using it. Seems ok.