Trigger new request on url change about use-http HOT 27 CLOSED

So I think eventually I'm going to be deprecating the onMount feature. The best way to accomplish this is

import useFetch from 'use-http'

function Todos({ someID }) {
  const { get, data, loading, error } = useFetch('https://example.com/todos')

  useEffect(() => { // this should fire onMount and when someID changes
    get(`/${someID}`)
  }, [someID, get])

  return (
    <>
      {error && 'Error!'}
      {loading && 'Loading...'}
      {(data || []).length > 0 && data.map(todo => (
        <div key={todo.id}>{todo.title}</div>
      )}
    </>
  )
}

Alternatively this might work if someID isn't coming from a url change

import useFetch from 'use-http'
import useRouter from 'use-react-router'

function Todos({ someID }) {
  const { get, data, loading, error } = useFetch('https://example.com/todos')
  const { location } = useRouter()

  useEffect(() => { // this should fire onMount and when location.pathname changes
    get(`/${someID}`)
  }, [location.pathname, get])

  return (
    <>
      {error && 'Error!'}
      {loading && 'Loading...'}
      {(data || []).length > 0 && data.map(todo => (
        <div key={todo.id}>{todo.title}</div>
      )}
    </>
  )
}

from use-http.

For this, I think there are times that you don't want loading to be set to true by default. If you want to submit a form, you don't want loading true by default, only when you click submit. But, if this is the desired thing, then we could add something like:

<Provider url='' options={{ initialLoading: true }}><App /></Provider>

Isn't it enough to check if onMount === true and/or it's a GET request?

from use-http.

I should be able to hop back on this, this week.

from use-http.

@Markus-ipse this feature for the onMount loading === true by default is implemented in this branch and all the tests are working. Will merge today

from use-http.

Okay, I added onUpdate. It's available in v0.1.83. It's possible in the future we change this to a better name, but for now this works and should do exactly what you're looking for. 😊Going to close this. Feel free to reopen if something doesn't work or if you have some more syntax ideas!

from use-http.

#145

from use-http.

Sorry, I've been quite swamped at work so I haven't had the time to look at the example you posted and really think about it, but I looked at the PR now and I'd just like to say that I agree with your final decision :)

from use-http.

Your first example is the way I'm currently solving it now, but it feels a bit superfluous to me to have to use a useEffect to call a function I got from my fetch-hook, when all I want is for it to fetch anytime the url changes.

It's also a minor annoyance that the loading flag will be false before it turns true, which means I cannot just do an early return with a <Spinner /> if loading === true, but it's probably not a good practice on my end to assume all data exists just because ´loading === false` 😅

Do you have any thoughts on future changes to simplify this particular kind of flow or is it perhaps something you've already considered and found some downsides to that makes it not worth it (and if not, are you accepting PRs 😄)?

Thanks!

from use-http.

I am accepting PRs! :) What did you have in mind for this?

I've also been thinking about the loading state. Are there any cases you can think of where having loading === true as the default where it might be a bad thing? If not, I'm happy to make it true by default and once the fetch is called, set it to false

from use-http.

Before you get too far on a PR though, let's discuss some possible solutions :)

from use-http.

I can't really think of a case where setting loading === true as the default would be problematic ( provided that request will happen onMount), that's how I've always had it when doing my fetching manually.

I haven't spent a lot of time thinking about it yet, but perhaps adding the option to pass in a dependency array, like useEffect and adding another option (or changing onMount) that will re-trigger the request anytime something in the dependency array changes?

import {useGet} from 'use-http'

function Todos(props) {
  const options = {
    onMount: true 
  }

  const todos = useGet('https://example.com/todos/' + props.someID, options, [props.someId, options]) // 1

  // 2
  if (todos.loading) { return <MySpinner /> }
  
  if (todos.error) { return <h2>Oopsie</h2> }

  return (
    <>
      {(todos.data.map(todo => ( // 3
        <div key={todo.id}>{todo.title}</div>
      )}
    </>
  )
}

1 -> I'm not sure if the dependency array makes sense when onMount == false but my guess is that it wouldn't hurt.

2 -> if loading defaults to true I can just do an early return with a loading indicator and not have to think about handling the existence or non-existence of the response data yet.

3 -> (todos.data || []).length > 0 && todos.data.map(...) can be reduced to just todos.data.map(...) provided that I trust that my backend will always deliver an array.

But the last two are mostly selling points for me only because I'm doing a lot of prototyping at the moment..

from use-http.

If I uncomment the content of the dependency array for makeFetch and for the useEffect that handles the onMount logic, and then add url as a dependecy arg for get, i.e. const get = useCallback(makeFetch(HTTPMethod.GET), [url]), then I get the desired behaviour for GET-requests, anytime the url changes another request is triggered.

Is there a specific reason for the dependency arrays being commented out for makeFetch and useEffect?

from use-http.

They were causing an infinite loop in some of the tests. I just haven't had time to come back and fix everything. Feel free to make a PR. :) Just make sure all the tests are working please!

from use-http.

I can't really think of a case where setting loading === true as the default would be problematic ( provided that request will happen onMount), that's how I've always had it when doing my fetching manually.

I haven't spent a lot of time thinking about it yet, but perhaps adding the option to pass in a dependency array, like useEffect and adding another option (or changing onMount) that will re-trigger the request anytime something in the dependency array changes?

import {useGet} from 'use-http'

function Todos(props) {
  const options = {
    onMount: true 
  }

  const todos = useGet('https://example.com/todos/' + props.someID, options, [props.someId, options]) // 1

  // 2
  if (todos.loading) { return <MySpinner /> }
  
  if (todos.error) { return <h2>Oopsie</h2> }

  return (
    <>
      {(todos.data.map(todo => ( // 3
        <div key={todo.id}>{todo.title}</div>
      )}
    </>
  )
}

1 -> I'm not sure if the dependency array makes sense when onMount == false but my guess is that it wouldn't hurt.

2 -> if loading defaults to true I can just do an early return with a loading indicator and not have to think about handling the existence or non-existence of the response data yet.

3 -> (todos.data || []).length > 0 && todos.data.map(...) can be reduced to just todos.data.map(...) provided that I trust that my backend will always deliver an array.

But the last two are mostly selling points for me only because I'm doing a lot of prototyping at the moment..

For this, I think there are times that you don't want loading to be set to true by default. If you want to submit a form, you don't want loading true by default, only when you click submit. But, if this is the desired thing, then we could add something like:

<Provider url='' options={{ initialLoading: true }}><App /></Provider>

from use-http.

I was literally just about to post that lol. I think that would work pretty well

from use-http.

I'm so busy currently and want to add this in, but just strapped for time. I really want this. Feel free to submit a PR ❤️

from use-http.

I'm down to pair with you on it :)

from use-http.

I started looking into it a few weeks ago, but I got stuck on the infinite loop in the tests, so I rage-quit after two days. Then I started looking into it again yesterday and tried just inputing the URL and nothing else to the various dependency arrays and I no longer get the infinite loop, but I get 3 failed tests, but I've unable to setup VS Code to debug Jest tests written typescript so I'm a bit stuck again :'(

from use-http.

Hop on spectrum and we can chat. https://spectrum.chat/use-http?tab=posts

from use-http.

Also, I think the community seems to want onMount so maybe I'll leave it for now. I like the loading === true by default when onMount is true. That makes sense. For your question about trigger new request on url change, I was also thinking about something like:

const App = () => {
  const { location } = useReactRouter()
  const request = useFetch('url', {
    onUpdate: [location.pathname]
  })
}

from use-http.

For my particular case it's about triggering a new request when the url parameter changes, not the page you're on. More concretely, say I have a page where i show the details for a product and there is a dropdown where I can select a different product.

const App = () => {
  const [selectedProductId, setSelectedProductId] = useState();
  const request = useFetch('someurl/product/' + selectedProductId)
 // ...etc.
}

But maybe that was your point as well and I just misunderstood?

from use-http.

const App = () => {
  // when productID changes, rerun the fetch
  //  for route: https://url.com/product/:productID
  const { match: { params } } = useReactRouter()
  const request = useFetch(`https://url.com/product/${params.productID}`, {
    onUpdate: [params.productID]
  })
}

from use-http.

Right, I see.. Btw, why not use the standard dependecy-array-as-last-argument syntax that all(?) base React hooks follow?

const App = () => {
  // when productID changes, rerun the fetch
  //  for route: https://url.com/product/:productID
  const { match: { params } } = useReactRouter()
  const request = useFetch(`https://url.com/product/${params.productID}`, [params.productID])
}

The only downside I can see is having to accept between 1 and 3 argument and then using typeof checks to figure out if it was called with either of the following:

useFetch(url, options, deps)
useFetch(url, options)
useFetch(url, deps)
usefetch(url)

from use-http.

I'm not 100% opposed, it just adds a lot of complexity behind the scenes because of all the type checking.

useFetch(url, options, deps)
useFetch(url, options)
useFetch(url, deps)
useFetch(url)
useFetch(options, deps) // what if we already have the url set in the <Provider url='base url' />
useFetch(deps) // what if we already have the url set in the <Provider url='base url' />

I just feel like it's simpler to just do the onUpdate. Also, it's not confusing right?

from use-http.

@alex-cory it's not that confusing, and confusion and be solved by documentation, so considering the complexity you mentioned it's probably a worthy trade-off.

Edit:
Hmm.. maybe the name "onChange" is a bit confusing as it sounds like you should pass in a change handler function, but I don't have a better suggestion.. and also that applies to "onMount" as well.. so just ignore me 😅

from use-http.

@Markus-ipse so it looks like the overwhelming majority of people like your suggestion for the dependency array as a final argument to trigger it. I will be implementing this soon ^_^ and removing onUpdate.

from use-http.

I do have a question for you though on syntax for pagination. With this new dependency array, what would you name the paginate part?

const App = () => {
  const [page, setPage] = useState(1)
  const { data, loading } = useFetch({
    onMount: true,
    path: `/todos?page=${page}&pageSize=15`,
    paginate: (currData, newData) => [...currData, ...neweData],
    data: []
    // OR
    data: (currData, newData) => [...currData, ...neweData],

    // OR
    onUpdate: (currData, newData) => [...currData, ...neweData],

    // OR
    onNewData: (currData, newData) => [...currData, ...neweData],

    // OR
    merge: (currData, newData) => [...currData, ...neweData],

    // OR
    // might make it too confusing?
    paginate: 'prepend', // or 'append' (for []) and 'merge' for objects. Could also accept a callback

    // OR
    mergeNewData: (currData, newData) => [...currData, ...neweData],

     // OR
    updateNewData: (currData, newData) => [...currData, ...neweData],
  }, [page])

  return (
    <>
      {data.map(item => <div key={item.id}>{item.name}</div>}
      {loading ? 'Loading...' : <button onClick={() => setPage(page + 1)}>Load More</button>}
    </>
  )
}

Other naming conventions could be updateOnLoad, reducerOnUpdate (I don't really like this one), accumulate, aggregate, onLoad

from use-http.