Fix and add tests for duplicate hook calls on paginated requests

[delilahw]

Resolves #1367 ✨

Modified the options normaliser to check for duplicate hooks before concatenating arrays.
Following up from my comment on the issue, I decided to just filter the array instead of setting some sort of flag to indicate duplicated hooks.

I also added two tests for this specific scenario 😇 Wasn't really sure where to put it (hooks or pagination?) so I made a new file.

Checklist

• I have read the documentation.
• I have included a pull request description of my changes.
• I have included some tests.
• If it's a new feature (not applicable), I have included documentation updates.

[delilahw]

All fixed now. Your thoughts?

[szmarczak]

I'll review this on Sunday. I'm away from home now.

[delilahw]

All good! Enjoy your time out 😊

[szmarczak]

I figured out why this happens. It's because you call .paginate() on the instance which has already defined hooks. The function normalizes the options here

got/source/create.ts

Line 206 in a3e171c

let normalizedOptions = normalizeArguments(url, options, defaults.options);

And calls instance(normalizedOptions), which results in duplicate properties because normalizedOptions can be the same as defaults.

got/source/create.ts

Line 227 in a3e171c

const result = (await got(normalizedOptions)) as Response;

Currently I'm on my way back, I'll be home in 7h. This fix is not correct, it should be in create.ts. I'll try to fix this by tomorrow evening, but feel free to propose a solution :)

[delilahw]

Thanks for your help. From what I see, the main purpose of this line is to make sure the pagination options are normalised for use in paginateEach, and not to merge hooks or any other properties (even though it does).

got/source/create.ts

Line 206 in a3e171c

let normalizedOptions = normalizeArguments(url, options, defaults.options);

So we can just use instance(normalizedOptions) to handle all the merging of options, by passing in options instead of normalizedOptions. However, for all the pages afterwards, we'll need to access the normalized options, merge them, and then pass them back in. To prevent duplicates, we need to reset the hooks because the options have been normalized when we access them here:

got/source/create.ts

Lines 259 to 263 in a3e171c

	if (optionsToMerge === result.request.options) {
	normalizedOptions = result.request.options;
	} else if (optionsToMerge !== undefined) {
	normalizedOptions = normalizeArguments(undefined, optionsToMerge, normalizedOptions);
	}

Thats my proposed solution. What do you think?

Also, I modified the tests here to match the function signature paginate.all(url, options). Some of the tests look like this

got/test/pagination.ts

Line 553 in a3e171c

const all = await got.paginate.all('', {

But some look like this

got/test/pagination.ts

Line 358 in a3e171c

const result = await got.paginate.all<number>({

I don't know why the ci is failing, cause it passed on my own repo.

[szmarczak]

Thanks for the tests ❤️ Fixed in e02845f