openapi-ts icon indicating copy to clipboard operation
openapi-ts copied to clipboard

Transform case for generated types

Open robdodson opened this issue 1 year ago • 16 comments

Description

On the old openapi-typescript-codegen repo, I opened a PR to add a --transformCase flag which seemed pretty popular. Would y'all be willing to accept a similar PR?

Much like the original requester in https://github.com/ferdikoomen/openapi-typescript-codegen/issues/1252, our company has a large API that returns keys in snake case, but we convert them to camel case on the frontend using a bit of middleware in our fetch functions. The types generated from our OpenAPI specs are in snake case, which means we can't use them.

For example, if the original file was in snake case:

type MyModel {
  foo_bar: string;  
}

Using --transformCase camel would convert it to:

type MyModel {
  fooBar: string;  
}

robdodson avatar May 12 '24 23:05 robdodson

Hey @robdodson, how do you currently deal with this since the package doesn't support it?

mrlubos avatar May 13 '24 03:05 mrlubos

@mrlubos

@OskarAsplin published the package to npm in this comment. I've been using that version to generate our types.

I looked into using other libraries like Zodios and ts-rest but haven't really committed to either of them. Since our app is quite large and several developers work on it, just getting folks to incrementally adopt generated types from OpenAPI has been as far as I've gotten.

robdodson avatar May 13 '24 03:05 robdodson

I'm going to file this under the clients label as there's more work needed to complete this feature. Do you have an example of the middleware you use to actually transform the data?

mrlubos avatar May 13 '24 03:05 mrlubos

yep it's pretty simple. It uses the second argument to JSON.parse, which is a reviver function

import { camelCase } from 'lodash'

export default function camelCaseJsonBody(body: string): any {
  if (!body) {
    return {}
  }
  return JSON.parse(body, function (k, v) {
    if (('' + k).includes('_')) {
      this[camelCase(k)] = v
      return
    }

    return v
  })
}

Our network util essentially does this:

const res = fetch(...)
const data = await res.json()
return camelCaseJsonBody(data)

robdodson avatar May 13 '24 04:05 robdodson

Thanks for all the context @robdodson! No concern about overhead added by this transform?

I'll say that this feature isn't a priority at the moment, but it's good to keep in mind when doing architectural decisions. As you already found out, transforming nested values isn't intuitive as an example..

mrlubos avatar May 13 '24 05:05 mrlubos

No concern about overhead added by this transform?

I certainly don't love the transform, but it was put in during the early days of the company (3-4+ years ago) before I joined and now all of the backend services are built expecting snake case, and all of the frontend SPAs expect camel case. So it's one of those things that's difficult to change at this point.

I'll say that this feature isn't a priority at the moment, but it's good to keep in mind when doing architectural decisions.

That's fair, though the work for the PR is already done (for openapi-typescript-codegen at least) and I was curious how hard it might be for me to port it over? I can at least attest to the fact that we've been using it for quite a while on pretty large OpenAPI specs and haven't run into any issues and there were a decent number of upvotes on the original PR indicating that other folks have run into this same problem.

Is your main concern the complexity it might add to the codebase or is it more of an issue around not having time to work on something like this?

robdodson avatar May 13 '24 06:05 robdodson

@robdodson mainly a time/focus issue, it's just not the biggest lever to pull right now imo. Most of the touched files in the original pull request are quite different now, so I'm not sure we can even call it porting anymore, feels like it would have to be written almost from scratch.

A few things to think about:

  • configuration. Can it be config file only? Is CLI support required? Most of the current configuration is in nested option objects and CLI doesn't support those yet.
  • middleware. Both request and response middleware should be provided so people don't have to write it themselves. There's a similar flag for transforming dates into Date in types, and the feedback has been that without middleware, that functionality is useless. I want to avoid similar fate for this feature.
  • implementation. I'm not loving that you have to map through models before they're printed. Wonder if you ran any benchmark tests or know the impact this has had on performance? I've recently started adding mappings which are used to deduplicate types, would love to use something like that if possible.

mrlubos avatar May 13 '24 07:05 mrlubos

Hey @robdodson, still looking to migrate to Hey API? I might prioritise this issue soon

mrlubos avatar Aug 12 '24 03:08 mrlubos

@mrlubos I am still waiting too

denatys avatar Aug 12 '24 07:08 denatys

Thanks for following up @mrlubos. We've started using Orval and taking advantage of its input transformer feature to camel case the spec before it uses it to generate code. We had to write our own script to transform the spec, but it's been working fine so far.

robdodson avatar Aug 12 '24 16:08 robdodson

@robdodson thanks for providing additional context! Can you share the extent of your own script and why was it needed? Are you using any additional plugins Orval provides?

mrlubos avatar Aug 13 '24 04:08 mrlubos

Our OpenAPI specs have all operation query params and request bodies in snake case, so I just make a copy of the spec and iterate over all query params and requestBodies and camel case all of the properties. Then when the spec gets sent to Orval, the generated types and API functions all use camel case for everything.

I'm using their custom httpClient feature and I wrote my own client that accepts an options object to indicate if request bodies and query params should be converted from camel case back to snake case. By default, I convert request bodies back to snake case because all of our endpoints expect snake case bodies. And by default I do not convert query params to snake case, because some of our endpoints expect snake case and some expect camel case.

I'm also using the ability to split the generated client files by OpenAPI tags. We have a few small services and one big monolith service, so it's helpful to be able to break up that monolith into smaller clients. But all of the schema types from the monolith end up in a single file. Orval doesn't have an option to break up the schemas, and honestly I don't know a good way to do it because schemas doesn't support tags. But I think it's fine to have all of the models in one big file since multiple clients might return the same response body types.

I'm also using the custom headers feature to inject pragmas to /* eslint disable */ the generated files. And I use the afterAllFilesWrite hook to run prettier against the files. Although it looks like there's also a config option specifically for prettier so maybe I should use that.

robdodson avatar Aug 13 '24 20:08 robdodson

This will be super helpful when I get to this feature, thanks a lot @robdodson! For your header pragmas, why not throw those files into ignore files? That's what Hey API recommends

mrlubos avatar Aug 13 '24 20:08 mrlubos

oh that's a good idea, I guess we could do that as well.

robdodson avatar Aug 13 '24 20:08 robdodson

@mrlubos another feature that we're using is the ability to set a baseUrl for clients. We have some services that mount to specific paths in our monolith. So I might have a pets service that mounts to /api/pets. That service generates its own OpenAPI spec, but I need to give all of its operations an /api/pets baseUrl

robdodson avatar Aug 13 '24 21:08 robdodson

@robdodson I believe we've got that use case covered with clients. You can instantiate as many as you want with different URLs, and then pass them to the service function to override the default. Same logic with the upcoming TanStack Query plugin

mrlubos avatar Aug 14 '24 03:08 mrlubos

This is an extremely common use case - several projects at various companies used boxcar casing for APIs, and a tool like humps to transform to and from camel case on the client side. On my current project, we're doing it now - for one service, we're having to override the entire request function, a non-trivial amount of code, to support this transformation; for another service, we're using responseTransformer which is great for response bodies, but doesn't cover request bodies or params. Built-in support for this transformation is available in other popular generation tools like openapi-generator-cli, which is why we'll probably be migrating to that tool in the coming weeks. Making case conversion a priority is going to be important for the long-term success of this project IMO.

davidgoli avatar Oct 18 '24 17:10 davidgoli

Thank you for the thoughtful feedback @davidgoli. This issue is definitely getting some eyeballs judging by the reactions, and will be implemented at some point. Right now there are other issues needing to be resolved, which is why it's not getting love. Hopefully you'll be able (and willing) to give this package a try when it's in a better shape!

mrlubos avatar Oct 18 '24 18:10 mrlubos

I'm using this script in my package.json at the moment

"generate:api-spec": "curl http://localhost:8000/openapi.json -o openapi.json && npx @hey-api/openapi-ts && node scripts/convertToCamelCase.js"

where scripts/convertToCamelCase.js is

const replaceInFiles = require("replace-in-files");

async function convertToCamelCase() {
  try {
    await replaceInFiles({
      files: "./generated/**/*.ts",
      from: /_([a-z])/g,
      to: (_, p1) => p1.toUpperCase(),
    });
  } catch (error) {
    console.error("Error converting to camelCase:", error);
  }
}

convertToCamelCase();

Basically just finding all snake_case instances and replacing them with camelCase. It involves using the replace-in-files library

Jonathanvwersch avatar Oct 31 '24 23:10 Jonathanvwersch

We have something similar, but it's more complex than that because, for example, you might have string literals or enums or path strings, so a global find-and-replace isn't sufficient. It's easiest to perform this transformation when you already have an AST and can selectively transform certain categories of tokens.

davidgoli avatar Nov 01 '24 00:11 davidgoli

Thanks for following up @mrlubos. We've started using Orval and taking advantage of its input transformer feature to camel case the spec before it uses it to generate code. We had to write our own script to transform the spec, but it's been working fine so far.

Hi @robdodson, I'm re-reading this thread and I realised I never asked. Why didn't you also transform your OpenAPI specification before passing it to Hey API? I know you mentioned other things later, but it sounds like the crux of this issue for you was modifying the input.

mrlubos avatar Nov 27 '24 07:11 mrlubos

@davidgoli any chance you can point me to how openapi-generator-cli handles this or any other tool you might've come across since?

What really trips me up is handling scenarios when not every API parameter follows the same naming pattern (mentioned in this thread too), or "inconsistent" names that can't be converted wholesale by a tool like humps.

For example, if the source parameter was myHTMLtag, we could convert it to camelCase in types which is myHtmlTag, but then sending it back to the API would not be straightforward. Generating a precise transformer feels like an overkill for most cases too, not to mention we have a lot of unsupported cases.

At this point, I feel like committing to the wholesale approach might work the best for most people. There would still need to be an escape hatch to handle edge cases like the one I mention above. What do you think?

mrlubos avatar Nov 27 '24 08:11 mrlubos

We had to write our own converter, both to convert the generated types after generation, and also at runtime, to transform response bodies and also transform any post bodies/query parameters passed to our API back to snake case. In the process, it's important not to transform string literal values, and we also ignore tokens that have no lowercase letters and tokens that start with an underscore. (If I had to do over again, I probably wouldn't bother with converting the case, but the codebase is too large to easily change at this point, so we're committed.)

What's helpful here is the fact that our API consistently uses snake case and not a mix - which would otherwise be a nightmare. You'd probably need a "special tokens" list of custom mappings like myHTMLtag that are transformed according to your use case. We don't have any need for anything like that; I feel like it's probably a v2 kind of feature here - a more basic, general approach should get the job done. It's also useful to integrate with a runtime parser like Zod as the last step, to ensure that the transformation was successful. In general, though, it's easier to smooth out edge cases as needed in userland than to build special casing into the framework.

davidgoli avatar Nov 27 '24 16:11 davidgoli

@mrlubos

Hi @robdodson, I'm re-reading this thread and I realised I never asked. Why didn't you also transform your OpenAPI specification before passing it to Hey API? I know you mentioned other things later, but it sounds like the crux of this issue for you was modifying the input.

It was mainly because I noticed Orval provided an API where I could pass it a transformer function and they would pass in an already parsed version of the spec. Like, I could have written my own build step to first transform the spec, put it in an intermediate location, and then call Orval and pass it that version, but that felt really annoying to do. I liked that there was an easy lifecycle hook so I didn't have to store intermediate specs and possibly end up with stale ones.

robdodson avatar Dec 10 '24 19:12 robdodson