sindresorhus / got

🌐 Human-friendly and powerful HTTP request library for Node.js
MIT License
14.27k stars 935 forks source link

got.paginate does not call init hooks #1574

Closed UnknownTraveller closed 3 years ago

UnknownTraveller commented 3 years ago

Describe the bug

~node version: 12.18.1~ got version: ^11.8.1

I'm using got.extend with hooks to wrap an existing API. One of the quirks of this API is to heavily use nested objects and arrays in its query parameters. The default behavior of got does not allow for nested objects as searchParams, so I tried to make it nicer by using qs~stringify with the appropriate options in an init hook.

According to my understanding of the documentation, this is one of the intended use cases for this hook. I could not find any other (more specialized) way to override query string serialization.

While this works very well the 'normal' got api (everything going through the main got function), it throws a type error instead when using got.paginate, because the options are normalized directly in this function, without calling callInitHooks first.

Actual behavior

Instead of calling init hooks before normalizing the options, the hooks are skipped in the Pagination API, causing a TypeError being thrown in validateSearchParameters:

TypeError: The `searchParams` value '[object Object]' must be a string, number, boolean or null
    at validateSearchParameters (D:\Code\got-pagination-bug\node_modules\got\dist\source\core\index.js:67:19)
    at normalizeArguments (D:\Code\got-pagination-bug\node_modules\got\dist\source\core\index.js:440:21)
    at paginateEach (D:\Code\got-pagination-bug\node_modules\got\dist\source\create.js:161:33)
    at paginateEach.next (<anonymous>)
    at AsyncGeneratorFunction.got.paginate.all (D:\Code\got-pagination-bug\node_modules\got\dist\source\create.js:213:26)
    at main (D:\Code\got-pagination-bug\index.js:28:43)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Expected behavior

Options passed to got.paginate(...) and returned from pagination.transform (after merging) should be passed to the init hooks.

got.paginate is semantically the same as calling got(...) in a loop.

Code to reproduce

const got = require('got')
const qs = require('qs')

main().then(console.log, console.error)

const client = got.extend({
  hooks: {
    init: [
      options => {
        if (!(typeof options.searchParams === 'string' || options.searchParams instanceof URLSearchParams)) {
          options.searchParams = qs.stringify(options.searchParams)
        }
      }
    ]
  }
})

async function main() {
  // this works:
  const working = await client.get('https://jsonplaceholder.typicode.com/todos', {
    searchParams: {
      a: { b: { c: 42 } }
    }
  })

  console.log('client.get is working!')

  // this throws a TypeError:
  const broken = await client.paginate.all('https://jsonplaceholder.typicode.com/todos', {
    searchParams: {
      a: { b: { c: 42 } }
    }
  })
}

Checklist

szmarczak commented 3 years ago

This will be fixed in #1521