Cannot properly replace URL while paginating

[fiznool]

Describe the bug

• Node.js version: 12.16.1
• OS & version: macOS 10.15.4

When using the pagination API, it is desirable to remove the original searchParams for subsequent pages if the full URL for the next page is returned from the API. This is so that the search params on the query string of the returned URL can be used instead of the original searchParams.

As per this comment, this should be possible by returning searchParams: undefined from the paginate function, but this actually causes the original searchParams to take precedence, and any params on the query string in the returned URL are removed. What's more, the original searchParams are duplicated in the resulting URL.

Actual behavior

Original searchParams should not be added to URL.

Expected behavior

Original searchParams are duplicated in resulting URL, and search params inside the returned URL are overwritten/removed.

Code to reproduce

Using the Facebook Graph API, which returns the 'next page' URL as described above.

const accounts = await got.paginate.all('https://graph.facebook.com/v6.0/me/accounts', {
  searchParams: {
    access_token: '<access_token>',
    fields: 'id,name',
    limit: 1
  },
  responseType: 'json',
  pagination: {
    transform: (response) => response.body.data,
    paginate: (response) => {
      const { paging } = response.body;
      if(!paging.next) {
        // As per the Facebook docs, if there is no `next`, there is no more data.
        return false;
      }
      // Otherwise, we should return the whole URL.
      return {
        url: new URL(paging.next),
        searchParams: undefined // <-- This does not work
      };
    }
  }
});

As an aside, setting searchParams: '' in the return object from paginate is a workaround to this issue, and results in the desired behaviour.

Checklist

• I have read the documentation.
• I have tried my code with the latest version of Node.js and Got.

[crcastle]

Moreover, returning a non-undefined searchParams seems to append to the previous request's searchParams, not replace it.

I think @sinedied ran into this issue and described it here, but I'm not sure if it was clear or captured as a bug.

Here is the sequence of URLs the Pagination API is creating for @sinedied. I'm seeing the same pattern -- i.e. appending instead of replacing.

https://dev.to/api/articles/me/all?per_page=2&page=1
https://dev.to/api/articles/me/all?per_page=2&page=2&per_page=2&page=1
https://dev.to/api/articles/me/all?per_page=2&page=3&per_page=2&page=2&per_page=2&page=1
https://dev.to/api/articles/me/all?per_page=2&page=4&per_page=2&page=3&per_page=2&page=2&per_page=2&page=1
...

The docs say

It [the pagination.paginate function] should return an object representing Got options pointing to the next page.

To me, that means the returned Got options will completely replace the previous request's Got options, not merge or append to the previous request's Got options.

However, I could see the benefit of a merge so that things like e.g. responseType: 'json' don't have to be repeated.

[szmarczak]

Actually if it's undefined it keeps the old one. From the docs:

If the new property is set to undefined, it keeps the old one.

[szmarczak]

I think the solution is to make searchParams non enumerable. It should be behaving the same as body, json, and form. It would work for this use case, but it would break another use cases.

[szmarczak]

Actually I think it's a regression

[szmarczak]

But if undefined is explicit, then it should be undefined I think... Gonna test that.

[szmarczak]

const got = require('got');

const options = got.mergeOptions({
    url: new URL('http://localhost:41285'),
    searchParams: new URLSearchParams('page=0')
}, {
    url: 'http://localhost:41285/?page=1',
    searchParams: undefined
});

options.url.href;
// 10.7.0 => http://localhost:41285/?page=1
// latest => http://localhost:41285/?page=0&page=0

[szmarczak]

If it's undefined, then it should keep the old value only if it's a plain object.

[szmarczak]

In Got 12 setting serachParams to undefined will clear them. See https://github.com/sindresorhus/got/blob/main/documentation/2-options.md#resetting-options