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: Screaming snake case #5

Merged
merged 7 commits into from
Jul 22, 2023
Merged

fix: Screaming snake case #5

merged 7 commits into from
Jul 22, 2023

Conversation

jly36963
Copy link
Contributor

@jly36963 jly36963 commented Jul 18, 2023

The toCamelCase and deepCamelKeys functions have unexpected behavior when the argument involves SCREAMING_SNAKE_CASE.

Related to: #6

@jly36963
Copy link
Contributor Author

jly36963 commented Jul 18, 2023

Quero iniciar a conversação, mas eu tentarei resolver o problema.
Thanks for making such a cool library 🙌

@jly36963 jly36963 changed the title fix: screaming snake case fix: Screaming snake case Jul 18, 2023
@gustavoguichard
Copy link
Owner

@jly36963 thanks for finding this out.. does the error only happens at runtime or type-level is consistent?

Would you mind filing an issue meanwhile so other folks can relate to if they find such bug?

@jly36963
Copy link
Contributor Author

Yup, moved the code example to an issue

@jly36963 jly36963 marked this pull request as ready for review July 20, 2023 12:37
@jly36963
Copy link
Contributor Author

@gustavoguichard Se eu precisar mudar alguma coisa, deixe-me saber 👍

@gustavoguichard
Copy link
Owner

@jly36963 thank you very much for catching the bug and fixing it! Amazing contribution!!
I'll make a lil suggestion in loco if you don't mind.

src/casing.ts Outdated
Comment on lines 57 to 64
type CamelCase<T extends string> = Words<T> extends [infer first, ...infer rest]
? Join<[Lowercase<Is<first, string>>, ...CapitalizeAll<Is<rest, string[]>>]>
? Join<
[
Lowercase<Is<first, string>>,
...CapitalizeAll<LowercaseAll<Is<rest, string[]>>>,
]
>
: T
Copy link
Owner

Choose a reason for hiding this comment

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

All the type functions in this file are mapping to the same logic as the runtime logic.
So to keep that pattern this type should probably be defined as follows:

type CamelCase<T extends string> = PascalCase<T> extends `${infer first}${infer rest}`
  ? `${Lowercase<first>}${rest}`
  : T

Comment on lines 86 to 90
function toPascalCase<T extends string>(str: T): PascalCase<T> {
return capitalize(toCamelCase(str))
return words(str)
.map((v) => capitalize(toLowerCase(v)))
.join('') as PascalCase<T>
}
Copy link
Owner

Choose a reason for hiding this comment

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

Whereas this type should probably be defined as:

type PascalCase<T extends string> = Join<CapitalizeAll<LowercaseAll<Words<T>>>, ''>

@gustavoguichard gustavoguichard merged commit 8e03f24 into gustavoguichard:main Jul 22, 2023
@jly36963 jly36963 deleted the screaming-snake-case branch October 3, 2023 23:06
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.

None yet

2 participants