Skip to content

Fix case conversion for all fields #1

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

Conversation

OskarAsplin
Copy link

I tested your solution on some swagger files and noticed that it was not converting names of linked/nested fields. For example, the name of Enums were only converted in the main type linking to an Enum in a namespace, but not in the namespace itself. Fields in a nested object were also not converted. See example below.

export type MyTools = {
  toolResources?: { // Case transformed correctly
    tool_for_you?: string; // Not case transformed in nested objects
  };
  tool: Tools.handyTool; // Enum is case transformed here
};

export namespace Tools {
  export enum handy_tool { // But not case transformed here - leads to TS error
    HAMMER_TIME = 'HAMMER_TIME',
    SAW = 'SAW',
  }
}

This PR fixes this by converting the case of all fields containing a name.

And thanks a lot for starting the PR. This is a great addition to the library :)

@OskarAsplin
Copy link
Author

OskarAsplin commented May 30, 2023

Turned out there was an additional problem as well. The response bodies in service operations were not case converted. I added a fix for that too. Also I added the transform flag to Readme and renamed it to transformCase

Copy link
Owner

@robdodson robdodson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Just one comment regarding the helper function names.

name: transforms[type](model.name),
};
}
export const convertModelNames = <T extends Model | OperationResponse>(model: T, type: Exclude<Case, Case.NONE>): T => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OskarAsplin What do you think about renaming all of these functions to:

  • convertModelCase
  • convertEnumCase
  • convertServiceCase

so they are consistent?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to go ahead and merge and update them in my PR

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, thanks!

@robdodson robdodson merged commit 0971e23 into robdodson:add-transformModelCase Jul 29, 2023
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

Successfully merging this pull request may close these issues.

2 participants