contiamo / restful-react

A consistent, declarative way of interacting with RESTful backends, featuring code-generation from Swagger and OpenAPI specs 🔥
MIT License
1.87k stars 109 forks source link

Dymanic Path Parameters #260

Open elchicofrommo opened 4 years ago

elchicofrommo commented 4 years ago

Describe the bug I have a route that has path parameters. I've run the generation tool to get specific use methods for this route. In my function setup I initially don't have the value for these parameters. Ive set the use function to lazy:true so it doesn't trigger right away. I have a function that is triggered by the user after they input the value for these parameters. I find that I cannot pass in these values when I call refetch because the absolute path has already been interpreted to be /some/path/undefined. This is likely because the code expects the path parameter when first construct it (due to the `/some/path/${pathParam}' that is generated as the output from the generation step. My question is this:

How can I call refetch and replace the path parameter of generated functions dynamically. It's not clear from the documentation. I'd love to use this library cuz it's so close to what I need

fabien0102 commented 4 years ago

Hello @elchicofrommo,

If I understand well, the easiest way to do what you want should be something like this:

import React from "react";
import { useShowPetById } from "./Petstore";

const MyAwesomeComponent = () => {
  const [petId, setPetId] = React.useState<string>();
  const { data } = useShowPetById({ petId, lazy: Boolean(petId) }); // <- The trick is here!

  if (!petId) {
    return <div>choose a pet id</div>;
  } else if (data) {
    return <div>{data.name}</div>;
  } else {
    return <div>loading…</div>;
  }
};
elchicofrommo commented 4 years ago

I kind of already did something like that but I didn't like that I had to have another component hold state to manage this when the hook already has its own state that it manages. If the refetch takes arguments (which it does), it should also be able to overwrite the path parameters IMHO.

fabien0102 commented 4 years ago

I don't get it, where come from your path parameters? My example also works with a props instead of state, the idea is just to avoid a fetch using "lazy" with your parameters, after the hook already refetch everytime you change a parameters (a path parameter in this case) So no need to another state handler or refetch, otherwise you are just recoding the internals of restful-react 😉

elchicofrommo commented 4 years ago

This is the simplest example. There are no props for the path parameters and all activity is contained within the functional component. I've tried passing in a prop (as seen below) but subsequent call always ends up showing me the result for the original prop. I've followed the events in the debugger and when refetch is called the path param (which was originally defined as somepath/{myParam} exists in the hook as somepath/actualValue. The original string template isn't in the object when refetch is called so it couldn't replace the path param even if it wanted to.

You say it should be easy. Can you give me a clear example? As it stands i wrote my own lib so that I could pass in a param (like below) and refetch with the correct path, and the params substituted.

function MyComponent(props){
  let ref = React.createRef();
  let {data: result, loading, refetch} = useGeneratedRestfulRoute({myParam: props.myRef}) // this props path does n
  function handleClick(event){
    refetch({myParam: ref.current.value});
    event.preventDefault();
  }

  return(
      <form onSubmit={handleClick}>
        <input name="myParam" ref={ref}></input>
        <button type="submit">submit</button>
      </form>
  )
}
zeorin commented 4 years ago

I have a scenario in where I need to make many submissions in a component, each for a different id, which is a path parameter. It'd be so nice to be able to do this imperatively in the mutate call.

pierre-H commented 4 years ago

Any news ? Also needed 😃

fabien0102 commented 4 years ago

@elangobharathi Sorry for the very late response… (for my defense, I was at the hospital due to a 16m fall accident 🙄). To take back your example, the problem is how the components are splitted (or I'm missing something in the example), I will give a try: https://codesandbox.io/s/pensive-violet-uph9j?file=/src/App.tsx

So you don't need to use refetch at all, as soon as you update the path (or a path parameter since this is just a wrapper as we can see bellow) restful-react will refetch itself.

image

You just need to add lazy: !myParam to avoid a fetch before the myParam is set and we are good.

If I'm still aside of your question, please do a little code sandbox because I really don't get it ^^

pierre-H commented 4 years ago

Thank you @fabien0102 ! I wish you a good recovery !

zeorin commented 4 years ago

Sorry to hear about your accident!

In my case this is not an ergonomic solution. The issue I have is how do I know when to change the path parameter.

It's not just a case of not having one at the component mount and then getting it during the lifetime of the component. Using the lazy trick works well enough for that.

But when I have a list of things that I need to update, I need to loop through them. If I need to change the path parameter, currently, I don't know when I can do this. Even if I manage to work around it somehow, that's really not readable code, what I'm actually intending to achieve is not apparent.

A path parameter is conceptually not any different from a query parameter. If we allow for query parameters to be set when making the mutate call, why not a path parameter? Seems like a fairly arbitrary distinction.

fabien0102 commented 4 years ago

I have a scenario in where I need to make many submissions in a component, each for a different id, which is a path parameter. It'd be so nice to be able to do this imperatively in the mutate call.

If it's for DELETE, this should works, otherwise I don't think so (but I need to try)

fabien0102 commented 4 years ago

Sorry to hear about your accident!

In my case this is not an ergonomic solution. The issue I have is how do I know when to change the path parameter.

It's not just a case of not having one at the component mount and then getting it during the lifetime of the component. Using the lazy trick works well enough for that.

But when I have a list of things that I need to update, I need to loop through them. If I need to change the path parameter, currently, I don't know when I can do this. Even if I manage to work around it somehow, that's really not readable code, what I'm actually intending to achieve is not apparent.

A path parameter is conceptually not any different from a query parameter. If we allow for query parameters to be set when making the mutate call, why not a path parameter? Seems like a fairly arbitrary distinction.

We can definitely extends the mutate for this, the initial design was assuming that the id was on DELETE operation otherwise the body is the way to go (if I remember well). Of course this is true for our API usage, but maybe not for yours 🙄

The thing is that pathParameter is just a path, and restful-react don't know about the param (it's on the generator level).

In contiamo, sometime, we are just using custom fetch generator (https://github.com/contiamo/restful-react/blob/12d905cf848bdcee6362b6aaaf496dd3d1f90d63/examples/restful-react.config.js#L20-L39) when restful-react doesn't fit the usecase :wink:

And yes, sometime restful-react make your life easier, sometime not yet ^^ But I will glad to improve this!

ApacheEx commented 4 years ago

I have a scenario in where I need to make many submissions in a component, each for a different id, which is a path parameter. It'd be so nice to be able to do this imperatively in the mutate call.

if I understood your question properly it should be possible with current approach:

const { mutate } = useMutate({
  verb: 'POST',
  path: ({ id }) => `/api/v1/${id}/projects`,
});

// and then you just call.
mutate(body, { 
  pathParams: { id: 'whatever1' },
});

mutate(body, { 
  pathParams: { id: 'whatever2' },
});

mutate(body, { 
  pathParams: { id: 'whatever3' },
});

and also it should work for DELETE request since v14.4.0:

const { mutate } = useMutate({
  verb: 'DELETE',
  path: ({ id }) => `/api/v1/projects/${id}`,
});

// and then you just call.
mutate({}, { 
  pathParams: { id: 'whatever1' },
});

mutate({}, { 
  pathParams: { id: 'whatever2' },
});

mutate({}, { 
  pathParams: { id: 'whatever3' },
});
johncblandii commented 3 years ago

I think one of the uses here is missed so I'll try to help clarify a bit. I was hit with this tonight.

Basic example (code below):

The path param is ignored 100% when passed directly as you would with the initial call to the hook. Of course, you can pass the whole path, but why generate hooks just to copy out a path across your app.

The fix for this is to take the refetch and call refetch({ pathParams: { id: 'my id' } }) so it isn't a blocker. The problem is it isn't clear in the docs (had to read the code to see it was even possible) and id is still required on the initial function so I have to set it to '', which will confuse others without this long comment I'm about to type.

So, I believe this could use some improvements to help make this a bit easier; whether it is improved docs or support for "not required"/dynamic path params on initial hook calls. I also noticed path could be a function, which wasn't clear either and yet still wouldn't work in this example since the data needed is in a different component than the hook.

Basic example code:

const useCache = (...) => {
  const id = useSelector(...redux stuff);
  const cachedData = useSelector(...redux stuff);

  useEffect(() => {
    // Without pathParams here, it will use an empty string for the id
    if (!cachedData && id) refetch({ pathParams: { id } });
  }, [cachedData, id]);

  return { data: cachedData };
}

const useMenuData = () => {
  const { refetch, ...result} = useGetMyData({ id: '', lazy: true });

  return {
    ...useCatch({refetch}),
    ...result
  }
}

(hand-typed from memory so ignore any glaring code issues here; lol)

Note: yes, the hook could get passed into the useCache hook as well so it is all managed in there as well with the lazy: !id hack mentioned earlier. There are other issues with data massaging, etc in there as well.