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

feat: hash validation #307

Merged
merged 5 commits into from
Dec 23, 2023

Conversation

ariskemper
Copy link
Contributor

No description provided.

Copy link

netlify bot commented Dec 18, 2023

‼️ Deploy request for valibot rejected.

Name Link
🔨 Latest commit 259030e

@fabian-hiller fabian-hiller self-assigned this Dec 19, 2023
@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority labels Dec 19, 2023
@fabian-hiller
Copy link
Owner

I think for Valibot it might be better to change the argument to an array to be able to allow different hash types. What do you think?

// Just one hash type
hash(['md5']);

// Multiple hash types
hash(['md5', 'sha1', 'crc32']);

@ariskemper
Copy link
Contributor Author

ariskemper commented Dec 22, 2023

Do you have use case for multiple hash types?

@fabian-hiller
Copy link
Owner

I don't have this use case in my own projects, but I believe there could be, for example, APIs that accept more than two hash types as a parameter. We could also accept both, a string or an array as the first argument.

// With string arg (slower and more complicated)
const Schema1 = union([string([hash('md5')]), string([hash('sha1')])]);

// With array arg (faster and easier to write)
const Schema2 = string([hash(['md5', 'sha1'])]);

@ariskemper
Copy link
Contributor Author

ariskemper commented Dec 23, 2023

Good example, will add support for array of hash types.

@fabian-hiller
Copy link
Owner

Thanks! I will review it later.

@fabian-hiller
Copy link
Owner

I can't thank you enough for all the work you put into Valibot! Soon you will be the second top contributor. Feel free to use the project as a reference.

@fabian-hiller fabian-hiller merged commit e779e31 into fabian-hiller:main Dec 23, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority This has priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants