Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#582@patch: Window.fetch handles URL and Request objects. #624

Closed
wants to merge 2 commits into from
Closed

#582@patch: Window.fetch handles URL and Request objects. #624

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
350a25a
#582@patch: Window.fetch handles URL and Request objects.
stefanhoelzl Oct 16, 2022
0025f8e
Merge branch 'master' into task/582-fix-fetch-url-parameter
capricorn86 Oct 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 21 additions & 2 deletions packages/happy-dom/src/fetch/FetchHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import URL from '../location/URL';
import RelativeURL from '../location/RelativeURL';
import { RequestInfo } from './IRequest';
import IRequestInit from './IRequestInit';
import IDocument from '../nodes/document/IDocument';
import IResponse from './IResponse';
import Response from './Response';
import NodeFetch from 'node-fetch';
import { Request } from 'node-fetch';

/**
* Helper class for performing fetch.
Expand All @@ -17,14 +20,30 @@ export default class FetchHandler {
* @param [init] Init.
* @returns Response.
*/
public static fetch(document: IDocument, url: string, init?: IRequestInit): Promise<IResponse> {
public static fetch(
document: IDocument,
url: RequestInfo,
init?: IRequestInit
): Promise<IResponse> {
// We want to only load NodeFetch when it is needed to improve performance and not have direct dependencies to server side packages.
const taskManager = document.defaultView.happyDOM.asyncTaskManager;

return new Promise((resolve, reject) => {
const taskID = taskManager.startTask();

NodeFetch(RelativeURL.getAbsoluteURL(document.defaultView.location, url), init)
let request;
if (typeof url === 'string') {
request = new Request(RelativeURL.getAbsoluteURL(document.defaultView.location, url));
} else if (url instanceof URL) {
// URLs are always absolute, no need for getAbsoluteURL.
request = new Request(url);
capricorn86 marked this conversation as resolved.
Show resolved Hide resolved
} else {
request = new Request(RelativeURL.getAbsoluteURL(document.defaultView.location, url.url), {
...url
});
}

NodeFetch(request, init)
.then((response) => {
if (taskManager.getTaskCount() === 0) {
reject(new Error('Failed to complete fetch request. Task was canceled.'));
Expand Down
3 changes: 3 additions & 0 deletions packages/happy-dom/src/fetch/IRequest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import IHeaders from './IHeaders';
import IBody from './IBody';
import URL from './../location/URL';

export type RequestInfo = IRequest | string | URL;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to put the type as an interface with "I" as prefix in its own file, so that it is easy to find it (e.g. by doing ctrl + p and search for it).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, just that I understand you right, I will create a new file IRequestInfo.ts with only these two lines:

import URL from './../location/URL';
export type IRequestInfo = IRequest | string | URL;

But RequestInfo is not really a Interface isn't it? It can be a string or URL which are no Interfaces


/**
* Fetch request.
Expand Down
5 changes: 3 additions & 2 deletions packages/happy-dom/src/window/IWindow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import Plugin from '../navigator/Plugin';
import PluginArray from '../navigator/PluginArray';
import IResponseInit from '../fetch/IResponseInit';
import IRequest from '../fetch/IRequest';
import { RequestInfo } from '../fetch/IRequest';
import IHeaders from '../fetch/IHeaders';
import IRequestInit from '../fetch/IRequestInit';
import IResponse from '../fetch/IResponse';
Expand Down Expand Up @@ -328,11 +329,11 @@ export default interface IWindow extends IEventTarget, NodeJS.Global {
/**
* This method provides an easy, logical way to fetch resources asynchronously across the network.
*
* @param url URL.
* @param url RequestInfo.
* @param [init] Init.
* @returns Promise.
*/
fetch(url: string, init?: IRequestInit): Promise<IResponse>;
fetch(url: RequestInfo, init?: IRequestInit): Promise<IResponse>;

/**
* Creates a Base64-encoded ASCII string from a binary string (i.e., a string in which each character in the string is treated as a byte of binary data).
Expand Down
5 changes: 3 additions & 2 deletions packages/happy-dom/src/window/Window.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import AsyncTaskManager from '../async-task-manager/AsyncTaskManager';
import IResponse from '../fetch/IResponse';
import IResponseInit from '../fetch/IResponseInit';
import IRequest from '../fetch/IRequest';
import { RequestInfo } from '../fetch/IRequest';
import IRequestInit from '../fetch/IRequestInit';
import IHeaders from '../fetch/IHeaders';
import IHeadersInit from '../fetch/IHeadersInit';
Expand Down Expand Up @@ -615,11 +616,11 @@ export default class Window extends EventTarget implements IWindow {
* This method provides an easy, logical way to fetch resources asynchronously across the network.
*
* @override
* @param url URL.
* @param url RequestInfo.
* @param [init] Init.
* @returns Promise.
*/
public async fetch(url: string, init?: IRequestInit): Promise<IResponse> {
public async fetch(url: RequestInfo, init?: IRequestInit): Promise<IResponse> {
return await FetchHandler.fetch(this.document, url, init);
}

Expand Down
11 changes: 9 additions & 2 deletions packages/happy-dom/test/setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ global.mockedModules = {
options: null
},
'node-fetch': {
url: null,
url: {
url: Symbol('url')
},
init: null,
error: null,
response: {
Expand Down Expand Up @@ -53,7 +55,12 @@ class NodeFetchResponse {
}
}

class NodeFetchRequest extends NodeFetchResponse {}
class NodeFetchRequest extends NodeFetchResponse {
constructor(url) {
super();
this.url = url;
}
}
class NodeFetchHeaders {}

jest.mock('node-fetch', () => {
Expand Down
44 changes: 42 additions & 2 deletions packages/happy-dom/test/window/Window.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ describe('Window', () => {
const response = await window.fetch(expectedUrl, expectedOptions);
const result = await response[method]();

expect(MOCKED_NODE_FETCH.url).toBe(expectedUrl);
expect(MOCKED_NODE_FETCH.url.url).toBe(expectedUrl);
expect(MOCKED_NODE_FETCH.init).toBe(expectedOptions);
expect(result).toEqual(MOCKED_NODE_FETCH.response[method]);
});
Expand All @@ -470,7 +470,47 @@ describe('Window', () => {
const response = await window.fetch(expectedPath, expectedOptions);
const textResponse = await response.text();

expect(MOCKED_NODE_FETCH.url).toBe('https://localhost:8080' + expectedPath);
expect(MOCKED_NODE_FETCH.url.url).toBe('https://localhost:8080' + expectedPath);
expect(MOCKED_NODE_FETCH.init).toBe(expectedOptions);
expect(textResponse).toEqual(MOCKED_NODE_FETCH.response.text);
});

it('Handles URL object.', async () => {
const expectedUrl = new window.URL('https://localhost:8080/path');
const expectedOptions = {};

const response = await window.fetch(expectedUrl, expectedOptions);
const textResponse = await response.text();

expect(MOCKED_NODE_FETCH.url.url).toBe(expectedUrl);
expect(MOCKED_NODE_FETCH.init).toBe(expectedOptions);
expect(textResponse).toEqual(MOCKED_NODE_FETCH.response.text);
});

it('Handles Request object.', async () => {
const expectedRequest = new window.Request('https://localhost:8080/path', { method: 'GET' });
const expectedOptions = {};

const response = await window.fetch(expectedRequest, expectedOptions);
const textResponse = await response.text();

expect(MOCKED_NODE_FETCH.url.url).toBe(expectedRequest.url);
expect(MOCKED_NODE_FETCH.url.method).toBe(expectedRequest.method);
expect(MOCKED_NODE_FETCH.init).toBe(expectedOptions);
expect(textResponse).toEqual(MOCKED_NODE_FETCH.response.text);
});

it('Handles Request object with relative url.', async () => {
const expectedPath = '/path';
const expectedRequest = new window.Request(expectedPath);
const expectedOptions = {};

window.location.href = 'https://localhost:8080';

const response = await window.fetch(expectedRequest, expectedOptions);
const textResponse = await response.text();

expect(MOCKED_NODE_FETCH.url.url).toBe('https://localhost:8080' + expectedPath);
expect(MOCKED_NODE_FETCH.init).toBe(expectedOptions);
expect(textResponse).toEqual(MOCKED_NODE_FETCH.response.text);
});
Expand Down