Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Path parameters not automatically replaced? #19

Open
trevorpfiz opened this issue Dec 2, 2023 · 4 comments
Open

Path parameters not automatically replaced? #19

trevorpfiz opened this issue Dec 2, 2023 · 4 comments

Comments

@trevorpfiz
Copy link

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 trevorpfiz changed the title Path parameters not being replaced Path parameters not automatically replaced? Dec 2, 2023
@trevorpfiz
Copy link
Author

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
Copy link
Owner

astahmer commented Dec 5, 2023

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 #18 (comment)

@trevorpfiz
Copy link
Author

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
Copy link
Author

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants