Critique my baby react-query useQuery impl

, ,
1

Hey all.

Still new to rescript. Been struggling through Promise interop and how to model my I/O for a react app. I like the react-query approach for which there are rescript bindings, but those bindings don’t use the Result error handling strategy. Further, I realized that the react-query bindings return an object modelling query state that really doesn’t map into the ML spirit, so I did a dummy useQuery impl that models state using variant tuples.

/**
 * CQuery is a simple version of ReactQuery::useQuery that _cares_ about
 * Result<'t, 'e>, vs using untyped Promise rejections.
 */

type data_state<'a, 'e> =
  | Empty
  | Data('a)
  | Error('e)

type fetch_state =
  | Idle
  | Fetching(/* retry */ int)

type exn_interop<'t> =
  | Pok('t)
  | Pexn


let useQuery = (~queryFn, ~fatalExn) => {
  let (state, setState) = React.useState(_ => (Empty, Fetching(0)))
  React.useEffect0(() => {
    let isCancelled = ref(false)
    queryFn()
    ->Promise.map(v => Pok(v))
    ->Promise.Js.catch(_ => Promise.Js.resolved(Pexn))
    ->Promise.get(res =>
      if isCancelled.contents {
        ()
      } else {
        // maybe add on retries later
        switch res {
        | Pok(res') =>
          switch res' {
          | Ok(v) => setState(_ => (Data(v), Idle))
          | Error(e) => setState(_ => (Error(e), Idle))
          }
        | Pexn => setState(_ => (Error(fatalExn), Idle))
        }
      }
    )
    Some(() => isCancelled := true)
  })
  state
}

You can see it used in the following:

@react.component
let make = (~className: option<string>=?, ~title: string) => {
  let recentPosts = CQuery.useQuery(
    ~queryFn=WitClient.Posts.getRecent(~limit=10, ~offset=0),
    ~fatalErr=WitErr.Failed_request,
  )
  <div className={Belt_Option.getWithDefault(className, "")}>
    <Html.H2> {string(title)} </Html.H2>
    {switch recentPosts {
    | (CQuery.Empty, _)
    | (CQuery.Error(_), CQuery.Fetching(_)) =>
      <SkeletonList />
    | (CQuery.Error(err), CQuery.Idle) => err->WitErr.toString->string
    | (CQuery.Data(data), _) =>
      <ol className="list-decimal list-inside">
        {Belt.Array.map(data.values, it =>
          <li>
            <a href={Link.slugToPath(it.slug)} onClick={onNavLinkClick}> {it.title->string} </a>
          </li>
        )->React.array}
      </ol>
    }}
  </div>
}
2

It’s better to write a wrapper for react-query bibdings that return a variant type instead. The problem with your solution that there’s no cache, so if there are multiple components requesting the same data, then you’ll send a request for each of them. Also it’s not really good to make requests directly from useEffect, you can read more about it in react18 release notes.

1 Like
3

Hey @DZakh, thanks for the feedback! I appreciate it.

It’s better to write a wrapper for react-query bibdings that return a variant type instead

This was my original play. However, with my level of rescript-fu, I failed to get the .error field to be of type option<Js.Exn.t | 'e> or just option<'e>. The browser throws Js.Exn.t's on connection errors, and 'e should be derived from my Promise.t<result<'a, 'e>>. There’s also the matter of rescript exceptions, which I’m not sure how to funnel all three failure cases in gracefully in the wrapper. In my solution, I was able to coerce all failure modelling down into a single WitErr.t type. I may take another go at it, shimming error handling impl on their bindings. I posted an issue in the binding repo too.

not really good to make requests from useEffect

Can you back that claim? I checked the following:

references

AFAIK, useEffect is the idiom that React offers for …doing effects, whether I use it directly or transitively thru a 3p dep.

4

Here’s my wrapper attempt, first pass.

type rec queryResultR<'queryError, 'queryData> = {
  status: ReactQuery_Types.queryStatus,
  // snip...
  // giant copy and paste from queryResult, dropping error,
  // swapping in my error field
  error: option<'queryError>,
}

// useQuery wrapper, that accepts Result aware promises, mapping
// .data => Ok value & .error => Error value
let useQueryR = (~queryFn) => {
  let (err, setErr) = React.useState(_ => None)
  let queryFnWithResultHandling = React.useCallback1(
    opts =>
      queryFn(opts)
      ->Promise.tapError(e => setErr(_ => Some(e)))
      ->Promise.Js.fromResult
      ->Promise.Js.toBsPromise,
      // @TODO how to propagate Js exceptions from .catch?
    [queryFn],
  )
  let res = ReactQuery_Query.useQuery(
    ReactQuery_Query.queryOptions(~queryFn=queryFnWithResultHandling, ()),
  )
  {
    status: res.status,
    // ...snip... giant record copying common fields.
    // @TODO, is there a graceful way to copy the intersecting structural
    // fields,
    error: err,
  }
}


// DEMO

type foo_err = Zed

let foo = () => {
  if true {
    Ok(1)
  } else {
    Error(Zed)
  }
}

let v = useQueryR(~queryFn=_ => Promise.resolved(foo()))
v.error // option<foo_err>
v.data //  option<int>
5

Sorry for the misleading answer. It was in an issue discussion https://github.com/facebook/react/issues/24455?ysclid=l66icckw3e297849912#issuecomment-1111545042

Also, the reason why it’s better to use react-query internally is that your solution doesn’t solve the problems react-query does. It’s more like a helper to make requests on a component mount. I don’t say that it’s bad if it works for you.

3 Likes