-
Notifications
You must be signed in to change notification settings - Fork 25
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
Comments
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.
|
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) |
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. |
moved away from the project I was working on using 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); |
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:
The text was updated successfully, but these errors were encountered: