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

URL vs URL string #462

Closed
joeshub opened this issue Jan 10, 2017 · 7 comments · Fixed by #465
Closed

URL vs URL string #462

joeshub opened this issue Jan 10, 2017 · 7 comments · Fixed by #465

Comments

@joeshub
Copy link

joeshub commented Jan 10, 2017

I could be wrong, but it appears the Fetch specification and most native implementations accept a URL object as the first parameter to fetch() while it appears most of the examples here require a URL string, and passing a URL object breaks non-native implementations, for example Safari.

Is this polyfill built to support an URL object or URL string?

Thank you

@mislav
Copy link
Contributor

mislav commented Jan 11, 2017

Per spec, the fetch() method accepts a "RequestInfo" as 1st argument.

RequestInfo is a Request or string URL.

The new Request() constructor accepts an "input" as 1st argument.

"Input" is a string URL or Request.

Therefore, in both cases, the spec says that URLs should be passed as strings, not URL instances.

@mislav mislav closed this as completed Jan 11, 2017
@dgraham
Copy link
Contributor

dgraham commented Jan 11, 2017

"Input" is a string URL or Request.

This is how I understand the specification as well. It's interesting, though, that Chrome, Edge, Firefox, and Safari Technology Preview all accept a URL object as the input parameter.

Intuitively, we expect passing a URL object to work, so maybe we're misreading the spec?

@mislav
Copy link
Contributor

mislav commented Jan 11, 2017

I don't think we're misreading the spec. In case of new Request(input):

5. If input is a string, then run these substeps: …
6. Otherwise (input is a Request object), run these substeps: …

@joeshub
Copy link
Author

joeshub commented Jan 13, 2017

Thanks for clarifying. FWIW I think what is confusing is the spec actually shows a URL being used in an example not to mention as @dgraham mentioned, most browsers supporting it now too.

var url = new URL("https://geo.example.org/api"),
    params = {lat:35.696233, long:139.570431}
Object.keys(params).forEach(key => url.searchParams.append(key, params[key]))
fetch(url).then(/* … */)

@mislav
Copy link
Contributor

mislav commented Jan 13, 2017

@joeshub Ah, nice find. I agree that it would be intuitive to support URLs. If you have the time, you could try adding the tests that verify that we accept URL objects properly. Meanwhile, I'm going to open an issue in the spec repo asking for details.

@mislav mislav reopened this Jan 13, 2017
@mislav
Copy link
Contributor

mislav commented Jan 13, 2017

Here's the official reply:

the binding layer (the bit IDL defines) will first check if the passed argument is a Request object and otherwise invoke ToString() on it

This is interesting and probably not what we do in the polyfill. We will have to change our behavior for spec compliance.

@Mouvedia
Copy link

The toString invocation needs to be documented prominently.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants