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.

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

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

Conversation

stefanhoelzl
Copy link

window.fetch can now handle Request and URL objects as first parameter.
See https://developer.mozilla.org/en-US/docs/Web/API/fetch#parameters

Copy link
Owner

@capricorn86 capricorn86 left a comment

Choose a reason for hiding this comment

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

Nice contribution 🌟 I am pretty sure that we will have to make relative URLs absolute, even for the URL and Request instance.

packages/happy-dom/src/fetch/FetchHandler.ts Show resolved Hide resolved
@@ -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

@capricorn86
Copy link
Owner

Hi @stefanhoelzl! 🙂

We have fixed this now as part of this PR which has been merged:
#520

@capricorn86 capricorn86 closed this Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants