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

fix: Corrected ObjectToCamel, ObjectToPascal, ObjectToSnake types #81

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

ryanhaugh
Copy link

There is an issue with the ObjectToCamel, ObjectToPascal, ObjectToSnake where they don't handle an array of string number unions correctly.

For example, the following:

ObjectToCamel<{
  prop_name: ("a" | "b" | "c")[];
}>

incorrectly returns:

{
    propName: "a"[] | "b"[] | "c"[];
}

when it should return:

{
    propName: ("a" | "b" | "c")[];
}

This PR fixes ObjectToCamel, ObjectToPascal, ObjectToSnake to return the correct string/number union array type.

@ryanhaugh ryanhaugh changed the title Fixed ObjectToCamel, ObjectToPascal, ObjectToSnake types fix: Corrected ObjectToCamel, ObjectToPascal, ObjectToSnake types Aug 1, 2024
@ryanhaugh ryanhaugh marked this pull request as ready for review August 1, 2024 21:50
@BigBallard
Copy link

Frankly, seeing fixes like this hang for months is discouraging when the library is as popular as it is.

@RossWilliams
Copy link
Owner

Frankly, seeing fixes like this hang for months is discouraging when the library is as popular as it is.

@BigBallard If you want to re-write the PR so it doesn't reformat all the lines then I will look at it.

@RossWilliams
Copy link
Owner

@ryanhaugh If you can preserve existing formatting then I can look at these changes.

@ryanhaugh
Copy link
Author

@RossWilliams apologies for all the formatting changes - I've fixed them.

@ryanhaugh
Copy link
Author

This PR also fixes this issue:

ObjectToCamel<{
  property_name: undefined;
}>;

incorrectly returns:

{
  propertyName: unknown[];
}

when it should return:

{
  propertyName: undefined;
}

@RossWilliams let me know if you need anything else from me so we can get this across the line - thanks!

@RossWilliams
Copy link
Owner

@ryanhaugh Build fails, you need to adjust the tests.

@ryanhaugh
Copy link
Author

@RossWilliams I've simplified ObjectToCamel, ObjectToPascal, ObjectToSnake. I've also ensured all existing tests pass and have added more tests to ensure the bugs I've found are fixed with my PR.

@ryanhaugh
Copy link
Author

@RossWilliams any chance you could take another look at this PR?
Thanks!
Ryan

@ryanhaugh
Copy link
Author

@RossWilliams anything I can do to help move this along?

@ryanhaugh
Copy link
Author

Starting to think this won't be reviewed - please prove me wrong.

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.

3 participants