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

Expose openAPI nameFormatter function. #3939

Closed
MarkLyck opened this issue May 17, 2022 · 4 comments
Closed

Expose openAPI nameFormatter function. #3939

MarkLyck opened this issue May 17, 2022 · 4 comments

Comments

@MarkLyck
Copy link
Contributor

MarkLyck commented May 17, 2022

Is your feature request related to a problem? Please describe.

I'm having a lot of pain setting up our OpenAPI handlers in graphQL mesh. We have quite a lot of datasources.

At first, I was using the "stable" openapi handler, but I quickly found that many of the types, and even fieldNames it devises from the openAPI spec were not great, and resulted in a lot of numbers like

mac
mac3
request
requests

This is very close to the interface I want it to be, except mac should be the plural macs and mac3 should just be mac (or maybe macById)

It also creates a lot of types with the same name + a number like Mac5Input which is not as bad as the fieldName but it is getting a bit confusing.

I understand why it does that, but it's not pretty or easy to solve.

Then I remembered there was a new openAPI handler The guild is working on, and I tried switching to that.

The field names from the newOpenapi handler, is 10x worse. like:

cloudnac_DOT_mac_DOT_get
cloudnac_DOT_mac_DOT_list_macs
cloudnac_DOT_requests_DOT_get
cloudnac_DOT_requests_DOT_list_requests

and so on... (just terrible query names!)

BUT they are unique and undeniably functionally better and easier to reason with than the incrementing numbers. So my only problem now is how terrible those names are to use by default after it goes through the sanitizeNameForGraphQL function.

https://github.com/Urigo/graphql-mesh/blob/8ab2702d182b9d824d40ff3f133840a2c6d89ce3/packages/utils/src/sanitize-name-for-graphql.ts#L40

Describe the solution you'd like
I would like to be able to use my own formatter, to use the operationId from openAPI and choose how i Format it, instead of being forced to turn it into a sea of DOT's and have unnecessary prefixes.

Describe alternatives you've considered
My first thought was to solve this with the rename transform and regexes like this:

       - rename:
           mode: bare
           renames:
             - from:
                 type: Query
                 field: (.*)_DOT_(.*)
               to:
                 type: Query
                 field: $1_$2
               useRegExpForFields: true

To at least make it reasonable. But I realized I could only capture one set of DOT's and I needed to capture everything but the DOT's as well as other key things I don't want.

I also tried to at least get it somewhat closer with:

      - rename:
          mode: wrap
          renames:
            - from:
                type: Query
                field: _DOT_list_(.*)
              to:
                type: Query
                field: $1
              useRegExpForFields: true

in hopes to convert e.g. cloudnac_DOT_requests_DOT_list_requests to just requests. But instead that results in: cloudnac_DOT_requestsrequests 🤔 (I'm not quite sure how it comes up with that result?)

I also tried:

            - from:
                type: Query
                field: (_DOT_list_)(.*)
              to:
                type: Query
                field: $2
              useRegExpForFields: true

with no luck either, and I can't seem to find any broader explanation of the regex features in the docs.

E.g. I want to make these transformations in a function:

cloudnac_DOT_mac_DOT_get 👉 mac
cloudnac_DOT_mac_DOT_list_macs 👉 macs
cloudnac_DOT_requests_DOT_get 👉 request
cloudnac_DOT_requests_DOT_list_requests 👉 requests

In my case that means, getting rid of the prefix, and turning cases with "get" into singular form, and cases with "list" into plural form.

We have a nice standard for operationId in our many APIs that we cannot just change for the sake of matching graphQL-mesh's default formatting rules.

Additional context

Currently, to get around this mess. I'm having to use the rename transformation on every single root level field on every single API, in the above example where we have just 4 fields and 2 mutation fields this adds 38 manual lines of code renaming:

      - rename:
          mode: wrap
          renames:
            - from:
                type: Query
                field: cloudnac_DOT_mac_DOT_get
              to:
                type: Query
                field: mac
            - from:
                type: Query
                field: cloudnac_DOT_mac_DOT_list_macs
              to:
                type: Query
                field: macs
            - from:
                type: Query
                field: cloudnac_DOT_requests_DOT_get
              to:
                type: Query
                field: request
            - from:
                type: Query
                field: cloudnac_DOT_requests_DOT_list_requests
              to:
                type: Query
                field: requests
            - from:
                type: Mutation
                field: cloudnac_DOT_mac_DOT_patch
              to:
                type: Mutation
                field: patchMac
            - from:
                type: Mutation
                field: cloudnac_DOT_requests_DOT_patch
              to:
                type: Mutation
                field: patchRequest

Which is not terrible for 1 or 2 APIs, but starts getting cumbersome with 10+ APIs... Not to mention having to maintain this, everytime an API changes, it's not 1 more place that we have to remember to go in and update or add rename transformations for every field.

as far as I can tell, there's no way to achieve this with a simple regex.

All of this could be solved with an optional formatter function that takes operationId as a parameter.

@ardatan
Copy link
Owner

ardatan commented May 17, 2022

It is not easy for us to give names for the operation names that are invalid for GraphQL spec. You can always use naming convention and rename transform with regular expressions to have your own naming format.

@ardatan ardatan closed this as completed May 17, 2022
@MarkLyck
Copy link
Contributor Author

MarkLyck commented May 18, 2022

@ardatan of course I'm not asking for to do names that are invalid for graphQL spec.

As mentioned in the ticket, I have not found any way to accomplish this with the rename transform (aside renaming every single field and type manually, which for us would end up being +1,000 renames in total which would just be insane to manage).

It is not easy for us to give names for the operation names that are invalid for GraphQL spec. You can always use naming convention and rename transform with regular expressions to have your own naming format.

Can you please explain how we could accomplish this simple scenario with the rename transformation and regex?

cloudnac_DOT_mac_DOT_get 👉 mac
cloudnac_DOT_mac_DOT_list_macs 👉 macs
cloudnac_DOT_requests_DOT_get 👉 request
cloudnac_DOT_requests_DOT_list_requests 👉 requests

I posted what I have tried in the ticket above, but I could not get it working with the existing functionality which is why I created this feature request to expand needed functionality. Maybe there's another way than exposing a function?

But it would be great to be able to do this without having to manually look up and write 4-6 lines of yml config for every single field and type which for last graphql gateways is in the tens of thousands of config lines.

@ardatan ardatan reopened this May 18, 2022
@Urigo Urigo mentioned this issue Aug 11, 2022
46 tasks
@gilgardosh
Copy link
Collaborator

gilgardosh commented Sep 18, 2022

Recently we published a complete rewrite of the openApi handler, including a big change in the naming convention.
Migration guide here.

@MarkLyck The new convention is much closer to the logic you presented, could you give it a go and update?

@theguild-bot theguild-bot mentioned this issue Sep 28, 2022
@gilgardosh
Copy link
Collaborator

Closed due to this and no further interest

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

3 participants