Skip to content

Commit

Permalink
Fix relative URLs when paginating
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak committed Aug 2, 2021
1 parent 6a44a81 commit 439fb82
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 8 deletions.
4 changes: 3 additions & 1 deletion documentation/4-pagination.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ console.log(results);
const next = parsed.find(entry => entry.parameters.rel === 'next' || entry.parameters.rel === '"next"');

if (next) {
return {url: next.reference};
return {
url: new URL(next.reference, response.requestUrl)

This comment has been minimized.

Copy link
@szmarczak

szmarczak Aug 2, 2021

Author Collaborator

@sindresorhus I think it would be best if we always required a URL instance here. When people pass an absolute URL - a string, it will work, but if they pass a relative URL (which can be according to the spec), it will break.

This comment has been minimized.

Copy link
@sindresorhus

sindresorhus Aug 4, 2021

Owner

I agree

};
}

return false;
Expand Down
4 changes: 2 additions & 2 deletions source/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1103,15 +1103,15 @@ export default class Request extends Duplex implements RequestEvents<Request> {
try {
// We can't do `await fn(...)`,
// because stream `error` event can be emitted before `Promise.resolve()`.
let requestOrResponse = fn(url, this._requestOptions);
let requestOrResponse = fn!(url, this._requestOptions);

if (is.promise(requestOrResponse)) {
requestOrResponse = await requestOrResponse;
}

// Fallback
if (is.undefined(requestOrResponse)) {
requestOrResponse = options.getFallbackRequestFunction()(url, this._requestOptions);
requestOrResponse = options.getFallbackRequestFunction()!(url, this._requestOptions);

if (is.promise(requestOrResponse)) {
requestOrResponse = await requestOrResponse;
Expand Down
12 changes: 9 additions & 3 deletions source/core/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,9 @@ const defaultInternals: Options['_internals'] = {
const next = parsed.find(entry => entry.parameters.rel === 'next' || entry.parameters.rel === '"next"');

if (next) {
return {url: next.reference};
return {
url: new URL(next.reference, response.requestUrl),
};
}

return false;
Expand Down Expand Up @@ -2224,13 +2226,17 @@ export default class Options {
return this.getFallbackRequestFunction();
}

return request!;
return request;
}

getFallbackRequestFunction() {
const url = this._internals.url as (URL | undefined);

if (url!.protocol === 'https:') {
if (!url) {
return;
}

if (url.protocol === 'https:') {
if (this._internals.http2) {
if (major < 15 || (major === 15 && minor < 10)) {
const error = new Error('To use the `http2` option, install Node.js 15.10.0 or above');
Expand Down
1 change: 1 addition & 0 deletions source/core/response.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type {URL} from 'url';
import type {IncomingMessageWithTimings, Timings} from '@szmarczak/http-timer';
import {RequestError} from './errors.js';
import type {ParseJsonFunction, ResponseType} from './options.js';
Expand Down
12 changes: 10 additions & 2 deletions test/pagination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ const resetPagination = {
shouldContinue: undefined,
};

const attachHandler = (server: ExtendedHttpTestServer, count: number): void => {
const attachHandler = (server: ExtendedHttpTestServer, count: number, {relative} = {relative: false}): void => {
server.get('/', (request, response) => {
// eslint-disable-next-line unicorn/prevent-abbreviations
const searchParams = new URLSearchParams(request.url.split('?')[1]);
const page = Number(searchParams.get('page')) || 1;

if (page < count) {
response.setHeader('link', `<${server.url}/?page=${page + 1}>; rel="next"`);
response.setHeader('link', `<${relative ? '' : server.url}/?page=${page + 1}>; rel="next"`);
}

response.end(`[${page <= count ? page : ''}]`);
Expand Down Expand Up @@ -720,6 +720,14 @@ test('calls init hooks on pagination', withServer, async (t, server) => {
]);
});

test('retrieves all elements - relative url', withServer, async (t, server, got) => {
attachHandler(server, 2, {relative: true});

const result = await got.paginate.all<number>('');

t.deepEqual(result, [1, 2]);
});

test('throws when transform does not return an array', withServer, async (t, server) => {
server.get('/', (_request, response) => {
response.end(JSON.stringify(''));
Expand Down

0 comments on commit 439fb82

Please sign in to comment.