Feature Requests and Syntax Ideas about use-http HOT 28 CLOSED

I like the idea passing an abort controller to the library, gives the user the ability to override functionality if they want to notify user of an aborted request use a mock controller in testing, for limiting requests i would go with limit or rateLimit

from use-http.

@alex-cory I think you would want a delay between each retry so you could have retryDelay?: number with a default.

from use-http.

I like the concept of initial data, in my half arsed and half baked first attempt at useFetch I had initial data and a transform function that was run to transform the data after it was returned from the apiL

export const useFetchData = <TData, TReturnData = TData>(
  fetchUrl: string,
  defaultValue: TData,
  transformFunc: (o: TData) => TReturnData = identity,
): UseFetchData<TReturnData> => {

from use-http.

@alex-cory i did not notice the await. thats a nice trick, and it ensures setTodos([...todos, newTodo]) happens only once for each addTodo. I noticed the stale newTodo. would this be necessary

const latestNewTodo = useRef(newTodo);
latestNewTodo.current = newTodo

and later after the await, using the updater form of the set function

setTodos(prevTodos => {
  return [...prevTodos, latestNewTodo.current]
})

More importantly, to prevent errors and re-render problems, I did have to alter the if statement.

if (todos.length === 0 && initialTodos && initialTodos.length && !loadingInitialTodos) {

I say more importantly because my whole reason for asking for an options.reducer and to return the dispatch was so that I could interact with the data. Instead of that, could the setData function from useFetch.ts be returned, like this.

const [todos, loading, error, request, setTodos] = useGet(url, { 
  initialData: { items: [], hasMore: true }
})

Doesn't that remove the need for the if.

@dagda1 It's probably not a top priority, but +1 for "transform function". i had the same idea, but I didn't have the courage to request.

from use-http.

What do we think about this syntax for useQuery and useMutation?

import React, { Fragment } from 'react'
import { Provider, useQuery, useMutation } from 'use-http'

function QueryComponent() {
  const request = useQuery`
    query Todos($userID string!) {
      todos(userID: $userID) {
        id
        title
      }
    }
  `

  const getTodosForUser = id => request.query({ userID: id })
  
  return (
    <Fragment>
      <button onClick={() => getTodosForUser('theUsersID')}>Get User's Todos</button>
      {request.loading ? 'Loading...' : <pre>{request.data}</pre>}
    </Fragment>
  )
}

const App = () => (
  <Provider url='https://example.com'>
    <QueryComponent />
  </Provider>
)

The main difference here is the string template literal here. aka the

const request = useQuery`my query here`

instead of

const request = useQuery(`my query here`)

from use-http.

I guess it wouldn't bee too difficult to support both

from use-http.

@dagda1 what do you think of this syntax?

from use-http.

@alex-cory I'm not too up to speed with graphql. I do remember that literal syntax from a contract that used styled-components ages ago.

I'd like to be taking advantage of typescript for these queries, string queries give me the heebies.

I did a spike with this ages ago but not sure I can give much of an opinion here apart from that.

from use-http.

Interesting. I will check it out :) Thanks for the feedback!

from use-http.

@alex-cory the reason I stumbled across your library is that I need something that does retries. Would you be open to me having a stab at a PR for retries?

from use-http.

@dagda1 Yes! There's actually a TODO at the bottom of the readme for this. I was thinking there could be an option that would look like

const request = useFetch(url, {
  retries: 3
  // OR
  retry: 3
})

and after the 3rd retry it will throw an error into request.error. I think while it's retrying just keep the loading state to true

from use-http.

Has a useReducer implementation been considered? I ask this partly because I haven't figure out how to use use-http in a scenario where the data was an array that would need to be added to later. onClick to get more items, for example.

I think the existing functionality could could be replicated with a default reducer and useReducer instead of the three useState's in useFetch.ts. From the user's perspective, there would no change unless they supplied a reducer, and possibly an initial state.

I have my own naive implementation of this, reusing use-http's FetchContext, some copy/paste from useFetch.ts, and dispatch instead of setData, setError, and setLoading. It seems to work. Any thoughts? Thanks for the lib.

from use-http.

@violinchris I have thought about this.

const Todos = () => {
  const [todos, setTodos] = useState([])
  const [initialTodos, loadingInitialTodos, _, getInitialTodos] = useGet(url)
  const [newTodo, loadingNewTodo, _, postNewTodo] = usePost(url)

  useEffect(() => {
    getInitialTodos()
  }, [])

  // set initial todos
  if (todos.length === 0 && !loadingInitialTodos) {
    setTodos(initialTodos)
  }

  const addTodo = useCallback(async () => {
    await postNewTodo({ title: 'new todo' })
    setTodos([...todos, newTodo])
  }, [postNewTodo, newTodo])

  return <>
    {todos.map(todo => <div key={todo.id}>{todo.title}</div>}
    <button onClick={addTodo}>Add Todo</button>
  </>
}

(there might be some holes in this^ I did it really fast)
Could you please post a sample of how you would like the end syntax with the reducer to look like?

from use-http.

@alex-cory

I'm even less certain about this than I was when I posted my reducer comment. I came across a few things as I tried to get this your example working.

I had to change if (todos.length === 0 && !loadingInitialTodos) { to if (todos.length === 0 && initialTodos && !loadingInitialTodos) {. This made wonder why there is no option for providing initial data to useFetch or useGet, but then I realied that would have been a problem in its own way.

I never got the if (!loadingNewTodo && newTodo) { to work correctly. Once you have a new newTodo you always have one, so I got the re-renders. I noticed that before you edited, you had a line with a usePrevious hook. I was initially excited, but I couldn't get to to function correctly. The previous value didn't stay one behind the current value. Very embarrassing for me, but even if it worked right I think the newTodo render happens before the render where loadingNewTodo becomes false.

Anyway, I mention all of this not to nitpick with your code. I could still be doing multiple things wrong. It's that the style of coding necessary seems error prone with the multiple conditions in the if statements, and not being able to easily figure out why a render is happening, yet having to rely on a specific re-render in the if statement.

Specifically regarding the syntax, here is what I was thinking about.

import reducer from 'highlyReusableReducer.js'

const Todos = () => {

  const [todos, loading, error, request, dispatch] = useGet(url, { 
    reducer, initialData: { items: [], hasMore: true }
  })

  let handleToggleDone = id => {
    dispatch({ type: 'TOGGLE_TODO', payload: id })
  }
  return <>
    {todos.items.map(todo => {
      return ( <div onClick={e => { handleToggleDone(todo.id) } 
                    key={todo.id}>
                      {todo.title}
               </div>
              )
    }
  </>
}

I'm less convinced than earlier that this is worth it, but I don't see a downside. It would function as it currently does if someone chose to not use the dispatch/reducer. And its the ultimate opt-out for someone that decides they like this library, but not for everything.

Thanks again. Its useful to me regardless, and I'll keep watching the repo.

from use-http.

@violinchris did you notice that I changed my example above to include an await? Please try again and let me know if that works for you. I will definitely be posting an example of this, I'm just kind of swamped trying to ensure everything is working by getting tests out.

from use-http.

Interesting. I kind of like that idea. I need to think about that a little bit. @violinchris

from use-http.

I think you are right with the initialData. I'm going to most likely add this feature, but still trying to work it out.

from use-http.

🤔What about something like this?

const TestApp = () => {
  const [todos, setTodos] = useState([])
  // const [data, loading, error, request, setData] = useFetch()
  const request = useFetch('http:example.com', {
    // initial data, probably just an empty default as this will get overwritten
    // each time `request.method()` is called
    data: []
  })

  useEffect(() => {
    initializeTodos()
  }, [])

  async function initializeTodos() {
    // const { data, endLoading } = await request.get('/todos')
    const [ initialTodos, stopLoading ] = await request.get('/todos')
    setTodos(initialTodos)
    stopLoading()
  }

  async function addTodo() {
    const [ todo, stopLoading ] = await request.post('/todos', {
      title: 'No way...'
    })
    setTodos(oldTodos => [...oldTodos, newTodo])
    stopLoading()
  }
}

I like this better but it'll require using js Proxy and we would probably have to remove array destructuring syntax/might have to remove destructuring syntax completely (which might not be a bad thing). I've only built 1 thing with Proxy so

const TestApp2 = () => {
  const [todos, setTodos] = useState([])
  // const [data, loading, error, request, setData] = useFetch()
  const request = useFetch('http:example.com', {
    // initial data, probably just an empty default as this will get overwritten
    // each time `request.method()` is called
    data: []
  })

  useEffect(() => {
    initializeTodos()
  }, [])

  async function initializeTodos() {
    const initialTodos = await request.get('/todos')
    setTodos(initialTodos)
    request.loading = false // -> request.setLoading(false)
  }

  async function addTodo() {
    const newTodo = await request.post('/todos', {
      title: 'No way...'
    })
    setTodos(oldTodos => [...oldTodos, newTodo])
    request.loading = false
  }

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

from use-http.

At some point in the past, I think data wasn't being returned from what is now doFetch in useFetch.ts, but now it is. That was my assumption anyway, esp. when looking at your first todo example that added the newTodo from the const [newTodo, loadingNewTodo, _, postNewTodo] = usePost(url) line after the await postNewTodo({ title: 'new todo' }).

Anyway, with these examples, this all seems more conventional in terms of how i think about async stuff, but also it seems more likely that a certain boilerplate will be the usual procedure.

const [todos, setTodos] = useState([])

const request = useFetch('http:example.com', { ...

useEffect(() => { initialize() }, []) ...

async function initialize() { ...

async function addTodo() {

This is why I'm still interested in setData being returned, and I'm wondering what the issue with that is. I definitely understand being hesitant, because I am as well, but I'm not imagining a problem.

Here is a different possible version of the syntax that could allow for less boilerplate. Notice the onMount, and the new opts argument. Maybe the opts is clumsy, and another method that only returned the data without setting it would be nicer, but I think it gets the point across.

  const [todos, loading, error, request, setTodos] = useFetch('http:example.com', {
    data: [], 
    onMount: true // yes, I saw the comment regarding onMount somewhere, maybe opt-in?
  })

  async function addTodo() {
    let opts = { shouldSetData: false }
    const newTodo = await request.post('/todos', {
      title: 'No way...'
    }, opts)
    setTodos(oldTodos => [...oldTodos, newTodo])
    request.loading = false
  }

With that said, I am understanding your recent todo examples and I am satisfied without the setData being returned. Even if returning setData is a good idea, there is no rush to implement this as far as I'm concerned.

Btw, I like the proxy syntax, but not enough to give up destructuring syntax. But, what would ever be the use of request.loading = false. Isn't that already handled automatically.

from use-http.

So, the problem is that you need to setTodos before loading === false. And the only way for loading to be handled properly is for 1. loading === true 2. setData(data) 3. loading === false. With the current syntax it's like 1. loading === true, 2. almost like a race condition we don't know loading will be set to false before or after setTodos is called. Unless I'm overthinking things (which could very well be the case). I'm wrong sometimes 😛

from use-http.

Well, you are correct, but I'm not going to let that deter me.

Regarding the 1,2,3 (try-catch-finally), you could easily cut that to 1,2 with two useReducer dispatches instead of 3 useStates sets, even without considering my previous ill-conceived suggestion of returning a dispatch, and it could appear to the user with the same syntax as now. I think it would cut down on a render, unless I misunderstand how it works. I think you could also return a setDataOnly method (that takes a function) that works exactly like setData from useState, but it could interact with the reducer. Or not. Either way, one less render.

There is still the uncertainty about whether loading gets set to false before or after, but this is how I consider both scenarios with regards to putting request.loading = false in the todos example.

1. loading set to false before setTodos is called.

Arguably, an app should not rely on the 1,2,3 order, but if I understand, this is potentially the situation to be avoided, and in fact the situation I would expect, since you are returning data after finally { ... }. I guess this could be bad if a render occurs before todos are ready, but why set loading = false. It is already false. What good would that do?

2. loading set to false after setTodos is called.

Somehow setTodos gets called while loading is still true. Perhaps, briefly, a spinner or loading indicator instead of a glorious list of todos. Todos are still there in the data, and will get rendered because when loading is eventually set to false.

If I am not understanding things correctly, this will at least demonstrate my misunderstanding, and perhaps misunderstandings others may have.

Anyway, I'll back off with this now. Everything seems to be moving very fast and I'm not solid with typescript. It's easy enough to read, but I always get some error that slows me down. No need to respond if I'm wrong. I'll just keep watching the issues and check out the documentation. Thanks.

from use-http.

from use-http.

@dagda1 Yep, returning dispatch was "ill-conceived." Does setDataOnly have the same problems?

from use-http.

in my opinion yes, you might as well create the hook yourself. i think i like the reducer idea. i would allow a transform function to be passed with the initial options that could be applied to any dataset once the remote resolves.

from use-http.

Hi @alex-cory - Is there any way to "reset" the request? IE: Clear the 'error' in order to fire it again? I've a situation where I may want to fire a request a number of times (incorrect data on first try, then rectify and try again), and I'd like to be able to clear the "error" object somehow. I can get around it in other javascripty ways but was wondering if it's possible with the library.

Similar to abort(), I guess, but more of a reset() function?

from use-http.

Whenever you fire the request again, it automatically resets the error to undefined. Could you post a code snippet of exactly what you're doing and how you would like it to look? i.e.

const App = () => {
  // assuming we already have a Provider with the `url` set in it
  const { error, get, reset } = useFetch({ onMount: true, data: [] })

  const doSomething = async () => {
    await get() // <- this resets the error everytime it's called
    reset()     // <- just trying to see the scenario in which we need it! 😊

    // is this kind of what you're talking about?
    const data = await get()
    if (!data) { // data is not what you think or want
      reset()
      await doSomething()
    }
  }
  
  return (
    <>
      {error && error.message}
      <button onClick={doSomething}>Do Something</button>
    </>
  )
}

from use-http.

I have a feature request idea, it's something I am using in my own useFetch implementation, but with time my use cases expanded, and I need to switch to something like use-http. But I don't see that this lib has what I had, and that is a skip function/option, that just ignores the GET requests.

The way I work is I have my state defined in the beginning as null or undefined (something falsey) and in the beginning, I don't want to make that requests because my data is not defined, as soon as an user interacts with a field (ex: search box) and starts typing, we start executing those GET requests.

NOTE: the search box value is a part of the url, or queryParams, and based on that the hooks knows to refetch by itself

Another one is useMock, which I use for when my endpoint/DB is slow, and I know the result I would get, it is always the same, so I start directly loading my "data", and avoid any requests. So when the backend guys fix that long query or that 500 error response, then we just switch off useMock prop to false and the fetch works normally.

const [searchBox, useSearchBox] = useState('')

const { data, error, loading, refetch } = useState('endpoint' + searchBox, { requestParams }, {executionLogicParams : {
   skipIf: !searchBox,
   useMock: true,
   mockData: mockedResponseVariable -- (some JSON object) --
}}) 

Are there any similar replacements for these features, does it make sense to have them as part of useFetch at all?

from use-http.

1. Skip. This is an interesting idea. Can you find any examples of other fetch hooks that use this same methodology? Currently you could do something with managed-state.

const { data, get } = useFetch('endpoint' + searchBox)

useEffect(() => {
  if (searchBox) get()
}, [searchBox])

2. Mocking

const { data, error, loading } = useFetch('endpoint', {
  data: mockData // set's the default data for this as the mock data
})

from use-http.