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: apply stricter typing to the v* signatures #831

Merged
merged 4 commits into from
Nov 4, 2024

Conversation

robbtraister
Copy link
Contributor

@robbtraister robbtraister commented Nov 1, 2024

The v4 function is guaranteed to return a string unless the buf argument is provided. By modifying the signature to denote this parameter as required for the Uint8Array case, we get the benefit of a stricter return type of string when simply calling v4().

Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the contribution.

Can you do me a favor and update the other methods that take an optional buf argument as well? (v1.ts, v3.ts, v5.ts, and v7.ts)

Edit: ... and v6.ts, too. So basically "all the thingz!"

@broofa broofa enabled auto-merge (squash) November 4, 2024 15:35
@broofa broofa disabled auto-merge November 4, 2024 15:35
@broofa broofa changed the title fix: apply stricter typing to the v4 signature fix: apply stricter typing to the v* signatures Nov 4, 2024
@broofa
Copy link
Member

broofa commented Nov 4, 2024

@robbtraister Thanks for the update. This looks good.

FYI, I added this commit after realizing the overload types for v3 and v5 aren't really necessary. (Right? If I got that wrong, please let me know.)

@broofa broofa enabled auto-merge (squash) November 4, 2024 17:22
@broofa broofa disabled auto-merge November 4, 2024 17:22
@broofa broofa enabled auto-merge (squash) November 4, 2024 17:22
@broofa broofa disabled auto-merge November 4, 2024 17:23
@broofa broofa merged commit c2d3fed into uuidjs:main Nov 4, 2024
5 checks passed
@robbtraister
Copy link
Contributor Author

@robbtraister Thanks for the update. This looks good.

FYI, I added this commit after realizing the overload types for v3 and v5 aren't really necessary. (Right? If I got that wrong, please let me know.)

Calling v3("value", "namespace") should always return string, but removing the overloads types it as string | Uint8Array.

Having said that, I only ever use v4, so 🤷

@broofa
Copy link
Member

broofa commented Nov 4, 2024

Calling v3("value", "namespace") should always return string, but removing the overloads types it as string | Uint8Array.

Ah, right... thanks for catching that!

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