Skip to content

feat(string): add snakeCase, startCase, lowerCase, kebabCase in compat layer#517

Merged
raon0211 merged 4 commits into
toss:mainfrom
dayongkr:compat/case
Sep 13, 2024
Merged

feat(string): add snakeCase, startCase, lowerCase, kebabCase in compat layer#517
raon0211 merged 4 commits into
toss:mainfrom
dayongkr:compat/case

Conversation

@dayongkr
Copy link
Copy Markdown
Collaborator

Description

I added these functions in compat layer

Benchmarks

snakeCase

Screenshot 2024-09-12 at 10 46 31 PM

startCase

Screenshot 2024-09-12 at 10 44 22 PM

lowerCase

Screenshot 2024-09-12 at 10 45 56 PM

kebabCase

Screenshot 2024-09-12 at 10 46 55 PM

close #508

Additional

I found a bug in startCase that is also in lodash.(you can see a related comment in here.

Bug: startCase has to convert FOO BAR to Foo Bar, but it converts to FOO BAR.

I think that we have to fix this bug in es-toolkit

@dayongkr dayongkr requested a review from raon0211 as a code owner September 12, 2024 13:52
@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
es-toolkit ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 13, 2024 2:34am

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.59%. Comparing base (3409d25) to head (cb16f44).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #517   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files         195      199    +4     
  Lines        1484     1492    +8     
  Branches      389      388    -1     
=======================================
+ Hits         1478     1486    +8     
  Misses          5        5           
  Partials        1        1           

export function normalizeForCase(str: string | object): string {
if (typeof str === 'object') {
str = str.toString();
str = String(str);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this improve performance?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in my computer it becomes slower

Copy link
Copy Markdown
Collaborator Author

@dayongkr dayongkr Sep 13, 2024

Choose a reason for hiding this comment

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

Does this improve performance?

I changed this code for the case that str is null or undefined. Before codes throws error in this case.

But we already guided that shouldn't use with null and undefiend by typescript and I also found this changes makes 1.75x slower.

So I reverted! Thanks

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

But Lodash don't throw error in that case. So I think String(str) is better way, because this function is for compatibility.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you tried template strings? Is it faster?

Copy link
Copy Markdown
Collaborator Author

@dayongkr dayongkr Sep 13, 2024

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not we just add a null and undefined check at the top?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes we can do that, but I will compare with Lodash source codes for more compatible!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think that we have to implement estoolkit/compat/toString before merging this. So i'm going to implement it at first!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Okay!

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.

Support for camelCase, kebabCase, lowerCase, snakeCase, startCase in compat layer

4 participants