Auto managed useFetch doesn't append URL to provider

[YassienW]

Describe the bug
When providing a URL via Provider component, useFetch URL doesn't get appended to Provider URL. This seems to only apply to auto managed useFetch

⚠️ Make a Codesandbox ⚠️
https://codesandbox.io/s/use-http-provider-bug-1n0mm?file=/src/App.js

To Reproduce
Steps to reproduce the behavior:
Check network tab in codesandbox, first request with empty string uses the Provider's URL, second and third requests don't use the Provider's URL

Expected behavior
The "/test" path should get appended to the provider's URL

[alex-cory]

You are correct! 🙂 That overwrites the url! You are looking for the path option.
if you have

<Provider url='https://example.com' />

then

useFetch('https://this-overwrites-the-url.com', [])

whereas

useFetch({ path: '/this-adds-a-path' }, [])

This is also the same functionality for managed state.

[alex-cory]

If I were to implement your desired method, it would be a huge breaking change.

[YassienW]

It is a bit odd but i understand that changing it at this point would break things. Thank you for the prompt response.

[alex-cory]

I've been deeply thinking about this and if it's worth it. I've been looking at all the patterns that have emerged over the past year. I think the biggest thing is, which feature is used the most and I think useFetch('/path') would actually be used more than useFetch('overwrite-global-url-otherwise-just-set-url'). If we wanted to actually overwrite the global url, you could just do useFetch({ url: 'my-url-overwriting-the-global-url' }). It seems most libs have the approach you desire.

[YassienW]

Well, if we consider the fact that provider is used to set a base URL to be used throughout the provider's children, it would make more sense to append to the base URL by default if it exists.

Generally speaking i think the behaviour should be determined based on the URL supplied, which is the way the native fetch method does it. Providing an absolute URL overrides the base URL while providing a relative URL adds to it:

url = http://google.com

useFetch("/search") or useFetch("search") would result in http://google.com/search

on the off chance that you want to override it you do this:

useFetch("http://bing.com/search")

Following this, i don't see the {path} option being needed anymore. In fact i don't see {url} option being needed either for unmanaged usage since you typically wouldn't be calling get, post, etc. in unmanaged usage and therefore you wouldn't need to set a base URL.

[alex-cory]

I think I will change this functionality. I think it greatly increases the understanding of the syntax and also makes it a lot cleaner. I just hate doing all these breaking changes. I also need to update videos. 😅

[alex-cory]

I still think we should have

useFetch({ url: 'overwrite-global-url' })

in the case we have a path and a url set, we can throw an invariant error saying you cannot set both a path and overwrite the global url at the same time. i.e.

useFetch('/path', { url: 'overwrite-global-url.com' })

this would error^ you would just put this in the url option like

useFetch({ url: 'overwrite-global-url.com/path' })

[YassienW]

Wouldn't automatic detection be more convenient? It's very easy to detect whether the path is absolute or relative

[alex-cory]

You're saying check if it starts with http?

[YassienW]

Yes, the way native fetch does it.

[alex-cory]

We have to basically implement our own "automatic detection" as far as I know. Let's say we're at https://example.com, the way fetch works is it says

fetch('/path') // fetches `https://example.com/path`

and if you do

fetch('https://example-2.com/path') // fetches `https://example-2.com/path`

now, if our app is rendered in https://example.com but we set https://example-2.com as the global url, we can't rely on native fetch's path detection because it only does that for https://example.com.

How do you suggest we do this? I think checking for a :// in the string. This allows for different protocols.

[YassienW]

I was thinking url.startsWith("http")

For allowing different protocols we can use url.includes("://") or url.indexOf("://") !== -1

[alex-cory]

This should be added as of v1.0.8. If there are any issues, let me know.