Inconsistent camel casing of path params between schema/types and runtime code

[TheTedAdams]

The typescript plugins paramsToSchema function formats path param keys to camel case.

The context template does not do this, which for my path params containing a . caused the query to not update when that path param changed (since the param value was never getting inserted into the key, so the key did not change).

The fetcher template also does not do this, which for my path params caused them to not be inserted.

Post-generation, I was able to fix both of these issues:
Context:

const resolvePathParam = (key: string, pathParams: Record<string, string>) => {
  if (key.startsWith('{') && key.endsWith('}')) {
    return pathParams[camel(key.slice(1, -1))];
  }
  return key;
};

Fetcher:

return (
  url.replace(/\{[\w|.]*\}/g, (key) =>
    encodeURIComponent(pathParams[camel(key.slice(1, -1))])
  ) + query
);

(For fetcher I also found I needed to add encodeURIComponent as I had some path params with / characters in them)

I know in #17 there is discussion that all params should match the OpenAPI spec, in which case the paramsToSchema function should not be camel casing. That said, I do prefer the developer experience of having camel cased params.... but either way the schema and runtime code templates need to match to avoid errors.

[needim]

Hmm, maybe issue #17's reason was this.

I didn't encounter this in my use cases yet. Can you also provide some part of an OpenApi spec to test this? Looks like you are using dot notation in your path params if I understand correctly.

[fabien0102]

Hi @TheTedAdams,

Do you have sample OpenAPI specs to illustrate your use case? 🤔 Normally, paramsToSchema only format the params in path, and those params are never named (the order in the path is the only critical thing to check) so this should work! But I might miss something 😅

Regarding the #17, this is for the other params (where the key also matters), so more an optional nice to have if you don't want to have snake case in your javascript project 😁

[TheTedAdams]

Sorry for the slow response, missed the github notification!

It's entirely possible what my backend guys have given me isn't very standard, but it seems to be valid OAS2. Our API is a passthrough to a gRPC API that takes in objects, and they've mapped some chunks of those objects into the path so we end up with paths like this:

"/realms/{address.realmId}/accounts/{address.accountId}/definitions/{address.path}": {
  "get": {
    "summary": "Get a definition details with the provided UUID",
    "operationId": "ProvisionService_ProvisionDefinitionGet",
    "responses": {
      "200": {
        "description": "A successful response.",
        "schema": {
          "$ref": "#/definitions/ProvisionDefinitionGetResponse"
        }
      },
      "default": {
        "description": "An unexpected error response.",
        "schema": {
          "$ref": "#/definitions/Status"
        }
      }
    },
    "parameters": [
      {
        "name": "address.realmId",
        "description": "Realm name; required.",
        "in": "path",
        "required": true,
        "type": "string"
      },
      {
        "name": "address.accountId",
        "description": "Account ID derived from the account service.",
        "in": "path",
        "required": true,
        "type": "string"
      },
      {
        "name": "address.path",
        "description": "This is the specific name/path of the object.",
        "in": "path",
        "required": true,
        "type": "string"
      },
    ],
  },

Because the types generation code calls camel that is outputting the following path params object:

 export type ProvisionServiceProvisionDefinitionListPathParams = {
  /*
  * Realm name; required.
  */
  addressRealmId: string;
  /*
  * Account ID derived from the account service.
  */
  addressAccountId: string;
};

This is why when I filled in the pathParams object based on the TypeScript types the original resolveUrl method from the fetcher template was not finding the params in my pathParams object, because it was looking for address.realmId in an object that had addressRealmId on it.

I'm a little confused about this statement:

and those params are never named (the order in the path is the only critical thing to check) so this should work!

What I've seen in context and fetcher is that it find the path params by looking for {key} and then takes the key between the { } chars to look up the value from the pathParams object. It doesn't appear to use order at all, and only uses the param names?

[needim]

Hi!

Can you please share the full generated fetch function, hook and the types for :
get "/realms/{address.realmId}/accounts/{address.accountId}/definitions/{address.path}"

Looks like you are expecting something like this as I understood:

 export type ProvisionServiceProvisionDefinitionListPathParams = {
  /*
  * Realm name; required.
  */
  ["address.realmId"]: string;
  /*
  * Account ID derived from the account service.
  */
  ["address.accountId"]: string;
};

But let's see the generated functions and types, please.

[TheTedAdams]

I will include the generated types/fetch/hook, however I want to clarify because I think I'm being misunderstood.

My point isn't that ...PathParams should have ["address.realmId"]. I don't mind that things have been camel cased for path params (honestly prefer it). My point is that while the type generation gets converted from address.realmId to addressRealmId, the generated FooContext.ts and FooFetcher.ts files are not doing that same conversion when replacing those params in the URL string (/realms/{address.realmId}/accounts/{address.accountId}/definitions/{address.path}).

In my app what I've done is made modifications to the Context and Fetcher files so that path parms are run through the same camel function (from the case npm package) that the type generation function is currently using.

So my repo (which is working!) in the Context.ts file I've replaced the original resolvePathParam:

const resolvePathParam = (key: string, pathParams: Record<string, string>) => {
  if (key.startsWith('{') && key.endsWith('}')) {
    return pathParams[key.slice(1, -1)];
  }
  return key;
};

with a modified one (that mirrors the type generator by calling camel(key):

const resolvePathParam = (key: string, pathParams: Record<string, string>) => {
  if (key.startsWith('{') && key.endsWith('}')) {
    return pathParams[camel(key.slice(1, -1))];
  }
  return key;
};

And in the Fetcher.ts file I've replaced the original resolveUrl:

const resolveUrl = (
  url: string,
  queryParams: Record<string, string> = {},
  pathParams: Record<string, string> = {}
) => {
  let query = new URLSearchParams(queryParams).toString();
  if (query) query = `?${query}`;
  return url.replace(/\{\w*\}/g, (key) => pathParams[key.slice(1, -1)]) + query;
};

With a modified one (that mirrors the type generator by calling camel(key)):

const resolveUrl = (
  url: string,
  queryParams: Record<string, string> = {},
  pathParams: Record<string, string> = {}
) => {
  let query = new URLSearchParams(queryParams).toString();
  if (query) query = `?${query}`;
  return (
    url.replace(/\{[\w|.]*\}/g, (key) =>
      encodeURIComponent(pathParams[camel(key.slice(1, -1))])
    ) + query
};

Like I mentioned in the first post, this is necessary because path params are getting run through camel in paramsToSchema.ts here:

openapi-codegen/plugins/typescript/src/core/paramsToSchema.ts

Line 15 in 282eef3

const formatKey = params[0].in === "path" ? camel : (key: string) => key;

So one solution would be to stop calling camel in paramsToSchema.ts (not my preferred). The other solution would be to call camel in resolvePathParam in the context template and resolveUrl in the fetcher template (what I've done w/ post-generator modification).

For completeness, generated stuff from the Components file as requested:

Components:

export type ProvisionServiceProvisionDefinitionGetPathParams = {
  /*
   * Realm name; required.
   */
  addressRealmId: string;
  /*
   * Account ID derived from the account service.
   */
  addressAccountId: string;
  /*
   * This is the specific name/path of the object.
   */
  addressPath: string;
};

export type ProvisionServiceProvisionDefinitionGetQueryParams = {
  /*
   * This is the specific API call.
   */
  ['address.service']?: string;
  /*
   * This is the kind of object
   */
  ['address.resource']?: string;
  /*
   * Optional
   */
  ['address.details']?: string[];
};

export type ProvisionServiceProvisionDefinitionGetError = Fetcher.ErrorWrapper<{
  status: Exclude<ClientErrorStatus | ServerErrorStatus, 200>;
  payload: Schemas.Status;
}>;

export type ProvisionServiceProvisionDefinitionGetVariables = {
  pathParams: ProvisionServiceProvisionDefinitionGetPathParams;
  queryParams?: ProvisionServiceProvisionDefinitionGetQueryParams;
} & FuzzballContext['fetcherOptions'];

export const fetchProvisionServiceProvisionDefinitionGet = (
  variables: ProvisionServiceProvisionDefinitionGetVariables
) =>
  fuzzballFetch<
    Schemas.ProvisionDefinitionGetResponse,
    ProvisionServiceProvisionDefinitionGetError,
    undefined,
    {},
    ProvisionServiceProvisionDefinitionGetQueryParams,
    ProvisionServiceProvisionDefinitionGetPathParams
  >({
    url: '/realms/{address.realmId}/accounts/{address.accountId}/definitions/{address.path}',
    method: 'get',
    ...variables,
  });

export const useProvisionServiceProvisionDefinitionGet = <
  TData = Schemas.ProvisionDefinitionGetResponse
>(
  variables: ProvisionServiceProvisionDefinitionGetVariables,
  options?: Omit<
    reactQuery.UseQueryOptions<
      Schemas.ProvisionDefinitionGetResponse,
      ProvisionServiceProvisionDefinitionGetError,
      TData
    >,
    'queryKey' | 'queryFn'
  >
) => {
  const { fetcherOptions, queryOptions, queryKeyFn } =
    useFuzzballContext(options);
  return reactQuery.useQuery<
    Schemas.ProvisionDefinitionGetResponse,
    ProvisionServiceProvisionDefinitionGetError,
    TData
  >(
    queryKeyFn({
      path: '/realms/{address.realmId}/accounts/{address.accountId}/definitions/{address.path}',
      operationId: 'provisionServiceProvisionDefinitionGet',
      variables,
    }),
    () =>
      fetchProvisionServiceProvisionDefinitionGet({
        ...fetcherOptions,
        ...variables,
      }),
    {
      ...options,
      ...queryOptions,
    }
  );
};

[needim]

oh, thanks for the clarification! So the issue is an inconsistency between generated components and context&fetcher resolvers.

Probably idea here was to keep generated files as minimal as possible and don't add any extra dependency to the userland like the case package.

My ideas:

1. camelCase transform can be optional, and we can ask the user on the init function if they want camelCase transform or not. If they want, we can transform params and include the case package in generated fetcher&context with modified resolvers for camelCase.
2. fetcher and context belong to userland and users need to change them for their needs. But this should be mentioned in the documentation.
3. ??

I like option 1.

But also want to hear from you @fabien0102, @TheTedAdams

[TheTedAdams]

I think 1 is a valid option.

I think if I understand what you're saying for 2, the suggestion is to leave things as they are but add documentation saying "Hey, if you have any path params that aren't in camel case, you have to make these changes or nothing will work". I think since OpenAPI doesn't seem to limit what those path param keys can be this isn't a great solution.

Option 3 idea that could fix this and also maybe allow delivery of #17 without adding an npm package dep at the cost of some file size bloat would be to generate map objects that maps formattedKey to original.key/original_key...

Could be something like

export const useProvisionServiceProvisionDefinitionGet = <
  TData = Schemas.ProvisionDefinitionGetResponse
>(
  variables: ProvisionServiceProvisionDefinitionGetVariables,
  options?: Omit<
    reactQuery.UseQueryOptions<
      Schemas.ProvisionDefinitionGetResponse,
      ProvisionServiceProvisionDefinitionGetError,
      TData
    >,
    'queryKey' | 'queryFn'
  >
) => {
  const { fetcherOptions, queryOptions, queryKeyFn } =
    useFuzzballContext(options);

  // original -> formatted since we use keys in path to look up formatted keys in variables.pathParams
  const pathKeyMap = {
    ["address.realmId"]: "addressRealmId",
    ["address.accountId"]: "addressAccountId",
    ["address.path"]: "addressPath"
  };

  // formatted -> original since we use keys in variables.queryParams to add to URLSearchParams
  const queryKeyMap = {
      ['addressService']: "address.service",
      ['addressResource']: "address.resource",
      ['addressDetails']: 'address.details',
  };

  return reactQuery.useQuery<
    Schemas.ProvisionDefinitionGetResponse,
    ProvisionServiceProvisionDefinitionGetError,
    TData
  >(
    queryKeyFn({
      path: '/realms/{address.realmId}/accounts/{address.accountId}/definitions/{address.path}',
      operationId: 'provisionServiceProvisionDefinitionGet',
      variables,
      pathKeyMap,
    }),
    () =>
      fetchProvisionServiceProvisionDefinitionGet({
        ...fetcherOptions,
        ...variables,
        pathKeyMap,
        queryKeyMap
      }),
    {
      ...options,
      ...queryOptions,
    }
  );
};

This might be overkill and more bloat than wanted, but would provide a way to open up various key transformation options...

[Matteomt]

Hi, I have encountered a similar (the same, but in a simpler context) issue with path parameter job_id being inconsistently transformed to jobId from the OpenAPI schema to Typescript, but not in resolvePathParam.

In fact, everything in my setup was silently working with an hard-to-find caching issue: the generated query keys are silently missing the value for the path parameters, and undefined is being used in the query key. Then the data for the wrong job is loaded and displayed without triggering any kind of cache miss or refetch as expected for TanStack Query given the query key is not changing (job_id results to be always undefined in the query key due to this issue).

From the point of view of queryKeyFn, resolvePathParam looks for job_id, while jobId is provided and this is the issue.
In fact, the functions use... generated by this tool from my OpenAPI schema contain a call to queryKeyFn with the untrasformed path (so they contain the untrasformed parameter names like job_id).

Then it all continues as nothing happened because, from the point of view of backendFetch, which calls resolveUrl, the url it receives does already contain the transformed path parameter names.
This is because the functions fetch... generated by this tool from my OpenAPI schema do contain the trasformed path parameter names (like jobId) in the url, so it fetches correctly when it fetches.

Please note that the snake_case style for parameter names is the usual one if your backend is implemented in python. I apreciate the transformation to camelCase for typescript even if not required, but it must be consistent across code paths.

So the following is the patch that I apply to my ~Context.ts as an initial, partial, and very unhappy solution:

diff --git a/backendContext.ts b/backendContext.ts
--- a/backendContext.ts buggy
+++ b/backendContext.ts patched
@@ -63,9 +63,11 @@
   return queryKey;
 };
 // Helpers
+const ___camelize: (str: string) => string = str =>
+    str.toLowerCase().replace(/[^a-zA-Z0-9]+(.)/g, (m, chr) => chr.toUpperCase())
 const resolvePathParam = (key: string, pathParams: Record<string, string>) => {
   if (key.startsWith("{") && key.endsWith("}")) {
-    return pathParams[key.slice(1, -1)];
+    return pathParams[___camelize(key.slice(1, -1))];
   }
   return key;
 };

[daviseford]

oh, thanks for the clarification! So the issue is an inconsistency between generated components and context&fetcher resolvers.

Probably idea here was to keep generated files as minimal as possible and don't add any extra dependency to the userland like the case package.

My ideas:

1. camelCase transform can be optional, and we can ask the user on the init function if they want camelCase transform or not. If they want, we can transform params and include the case package in generated fetcher&context with modified resolvers for camelCase.
2. fetcher and context belong to userland and users need to change them for their needs. But this should be mentioned in the documentation.
3. ??

I like option 1.

But also want to hear from you @fabien0102, @TheTedAdams

My team is still suffering from this issue. I would love a resolution. Personally, I don't see the need to force camelcasing things, I think the code written should match the spec.

@Matteomt 's fix above works for me, but it would obviously be preferable to not have to update every single *Context.ts file in my project.

[fabien0102]

Hey! Sorry I'm quite busy thoses days and I have no time to take care of my open-source projects…

The issue seams to be on the way we are camelized the path indeed (due to the . in your case) this should be quite straightforward to fix. You can update *Context.ts and *Fetcher.ts as much as you want, since this is in userland (I need to do something to make it more obvious 🤔) (for the record, if you don't have Generated by @openapi-codegen header, this is userland ;))

Another way to fix it is to apply camel in your specs like this:

// openapi-codegen.config.ts
// ...
   to: async (context) => {
      const filenamePrefix = "github";
      camelizePathParams(context.openAPIDocument); // <= Mutate the specs
      const { schemasFiles } = await generateSchemaTypes(context, {
        filenamePrefix,
      });
      await generateReactQueryComponents(context, {
        filenamePrefix,
        schemasFiles,
      });
    },
// ...

[fabien0102]

This should be fixed in the 7.0.1, thx for the nice reporting, sorry for the delay and please tell me if this is not working for you :)