astahmer / typed-openapi

Generate a headless Typescript API client from an OpenAPI spec
https://typed-openapi-web.vercel.app/
200 stars 23 forks source link

Path parameters not automatically replaced? #19

Open trevorpfiz opened 11 months ago

trevorpfiz commented 11 months ago

Should we have to manually replace the path params? If I have an endpoint, /Patient/{patient_id}, the patient_id is not getting replaced by the parameter.

Here is how I am doing it right now:

const patientData = await api.get("/Patient/{patient_id}", {
          path: { patient_id: path.patient_id },
});
export const api = createApiClient(async (method, url, params) => {
  const canvasToken = await ensureValidToken();
  const headers = {
    Authorization: `Bearer ${canvasToken}`,
    Accept: "application/json",
  };
  const options: RequestInit = { method, headers };

  if (params) {
    if (method === "post" || method === "put") {
      options.body = JSON.stringify(params);
    } else if (method === "get") {
      // console.log(method, url, params, "parameters");
    }
  }

  if (params?.path) {
    Object.entries(params.path).forEach(([key, value]) => {
      if (typeof value === "string") {
        url = url.replace(`{${key}}`, value);
      }
    });
  }

  return fetch(url, options).then((res) => res.json());
}, env.FUMAGE_BASE_URL);
trevorpfiz commented 11 months ago

I noticed the same thing for query parameters. I guess this is just the headless nature of bringing our own fetcher? I wonder if the library should have a default fetcher that handles the basics if I am not missing something.

if (params?.query) {
    const queryParams = Object.entries(params.query)
      .map(([key, value]) => {
        // Convert value to string if it is not already a string
        const stringValue = typeof value === 'string' ? value : String(value);
        return `${encodeURIComponent(key)}=${encodeURIComponent(stringValue)}`;
      })
      .join("&");
    if (queryParams) {
      url += `?${queryParams}`;
    }
  }
astahmer commented 11 months ago

hey, regarding the original issue, where would you think it should be replaced ? in a custom fetcher ?

and yes, providing a default fetcher makes sense and I'm definitely open to this idea, a bit more details here https://github.com/astahmer/typed-openapi/issues/18#issuecomment-1833322629

trevorpfiz commented 11 months ago

I will give these things some thought as I play around more and can submit a PR in the coming weeks.

Initial thoughts are that I like the control of being able to pass a custom fetcher to handle the params how I want and to validate or not, but the out-of-box experience has felt a little low level. Adding a default fetcher will help, but I could see where handling basic body, query, and path params in the get, post, and put methods might be the right place. Validation probably in the custom fetcher, and left optional?

For my use case, where I am converting a slightly outdated postman collection to an openapi spec, the generated code is not 100% right, so having full control of the validation has been nice.

trevorpfiz commented 9 months ago

moved away from the project I was working on using typed-openapi, but I wanted to at least share what I was doing as a basic example. lots of different things people will want to do, so the easiest solution is probably adding to the example usage comment in the generated file or the README with an example similar to this.

export const api = createApiClient(async (method, url, params) => {
  const token = await ensureValidToken();
  const headers = {
    Authorization: `Bearer ${token}`,
    Accept: "application/json",
    "Content-Type": "application/json",
  };
  const options: RequestInit = { method, headers };

  // Add body to request
  if (params) {
    if (method === "post" || method === "put") {
      options.body = JSON.stringify(params.body);
      // console.log(options.body, "body");
    } else if (method === "get") {
      // console.log(method, url, params, "parameters");
    }
  }

  // Replace path parameters in url
  if (params?.path) {
    Object.entries(params.path).forEach(([key, value]) => {
      if (typeof value === "string") {
        url = url.replace(`{${key}}`, value);
      }
    });
  }

  // Add query parameters to url
  if (params?.query) {
    const queryParams = Object.entries(params.query)
      .map(([key, value]) => {
        // Convert value to string if it is not already a string
        const stringValue = typeof value === "string" ? value : String(value);
        return `${encodeURIComponent(key)}=${encodeURIComponent(stringValue)}`;
      })
      .join("&");
    if (queryParams) {
      url += `?${queryParams}`;
    }
  }

  // console.log(method, url, params, "parameters");

  // TODO - can return headers, status, and ok if needed
  return fetch(url, options).then((res) => res.json());
}, env.BASE_URL);