Feat: Add minDigitChars - #1111
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thanks for the PR! Great idea! Since this is a very specific action, I may wait for more developers to express interest in such a feature before reviewing and merging it. |
fabian-hiller
added
enhancement
commit: |
|
Thanks @fabian-hiller 🙂 If this was accepted, I was going to propose:
I'd also like |
|
If anyone stumbles across this and is looking for a way to implement this now, you can use this: const minDigits = (min: number) =>
v.pipe(
v.string(),
v.check(
(input) => (input.match(/\d/gu) || []).length >= min,
`Must contain at least ${min} digit${min === 1 ? "" : "s"}.`,
),
)
v.safeParse(minDigits(2), 'I have no digits') |
|
In general, I am very interested in providing actions to validate the number of digits, letters and special characters, but we need to find perfect names for them. Valibot usually provides actions like |
|
Ah, of course; I saw v.digits('The string contains something other than digits.')
v.digits(8, 'Exactly 8 digits are required.') |
|
I don't recommend this as it would make our API inconsistent. What about |
|
Okay let's go with that 👍 🙂 I'll update this PR when I can a moment. Thanks. |
89108b1 to
5582469
Compare
5582469 to
d6c5269
Compare
|
Okay, that should be good to go; changed name and rebased onto |
|
Thank you so much! It looks good at first glance. I will try to review and merge it soon. Are you also interested in adding |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new feature to validate the minimum count of digit characters in a string, improving password validation. Key changes include:
- Adding a new utility function (_getDigitCount) to count digits in a string.
- Creating a new minDigitChars action with complete type definitions and overloads.
- Extending API properties and test coverage to support this new feature.
Reviewed Changes
Copilot reviewed 24 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| website/src/routes/api/(actions)/minDigitChars/properties.ts | Adds API property definitions for the minDigitChars action. |
| library/src/utils/_getDigitCount/* | Implements the _getDigitCount utility along with corresponding tests. |
| library/src/actions/minDigitChars/minDigitChars.ts | Implements the minDigitChars action with overloads and type annotations. |
| library/src/actions/minDigitChars/.test | Provides comprehensive tests and type assertions for the new action. |
| library/src/actions/index.ts | Updates the module exports to include minDigitChars. |
Files not reviewed (10)
- website/src/routes/api/(actions)/minDigitChars/index.mdx: Language not supported
- website/src/routes/api/(async)/customAsync/index.mdx: Language not supported
- website/src/routes/api/(async)/fallbackAsync/index.mdx: Language not supported
- website/src/routes/api/(async)/intersectAsync/index.mdx: Language not supported
- website/src/routes/api/(async)/lazyAsync/index.mdx: Language not supported
- website/src/routes/api/(async)/pipeAsync/index.mdx: Language not supported
- website/src/routes/api/(async)/unionAsync/index.mdx: Language not supported
- website/src/routes/api/(methods)/config/index.mdx: Language not supported
- website/src/routes/api/(methods)/fallback/index.mdx: Language not supported
- website/src/routes/api/(methods)/pipe/index.mdx: Language not supported
|
I certainly am interested in adding those. Are there any plans to automate the docs creation? That’s probably the fiddliest part about contributing to this project 😅 I’ve dabbled with typedoc on my personal utility library, Tings, and it works quite nicely. Your setup is more sophisticated as you’re linking pages, but maybe that could go into a JSDoc directive. It would reduce contributor effort, and your review effort massively. |
|
Unfortunately, our docs creation is currently very manual. But in general it should be possible to automate this process by writing a script to extract all necessary information and automatically generate the required |
|
Automation sounds nice, probably would surely make llms.txt easier too. I just wonder if it would be possible to have those links retained, which links to related schemas/actions/methods. |
|
I'm not looking for a perfect solution. There will still be a manual part. I am more looking for a CLI script that speeds up the process by generating an initial template with all the info we already know from the source code. |
|
Thanks @fabian-hiller I'm not sure if I'm going to get the time unfortunately; my time is quite limited these days with family commitments. But if someone else wants to implement this I highly suggest tying it into some custom JSDoc, e.g. /**
* Creates a min digits validation action.
*
* @param requirement The minimum digits.
*
* @returns A min digits action.
*
* @schemas any, array, custom, instance, string, tuple, unknown
* @methods pipe
* @utils isOfKind, isOfType
*/
export function minDigitChars<It should get to the point where contributors do not need to touch the docs at all 👍🤓 |
|
That is ok. I am sure someone else will look at it in the long run. For now, I prefer to keep our JSDoc comments simple and don't add extra stuff there. We need a separate JSDoc comment for each TypeScript overload function signature, and adding all that info would force us to duplicate it, which is annoying to maintain. |
|
@will-stone is attempting to deploy a commit to the Open Circle Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
Bot
added
the
size:XL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
🚧 Files skipped from review as they are similar to previous changes (15)
WalkthroughAdds public export wiring and documentation index entries for the Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
library/src/actions/minDigitChars/minDigitChars.ts (1)
22-22: Align thetypediscriminator with the action name for API consistency.
minDigitCharscurrently declares/emitstype: 'min_digits'. Consider usingmin_digit_charsso the discriminator follows the same naming pattern as the exported action and future related actions.♻️ Suggested change
export interface MinDigitCharsIssue< @@ - readonly type: 'min_digits'; + readonly type: 'min_digit_chars'; @@ export interface MinDigitCharsAction< @@ - readonly type: 'min_digits'; + readonly type: 'min_digit_chars'; @@ return { kind: 'validation', - type: 'min_digits', + type: 'min_digit_chars',Also applies to: 54-54, 117-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@library/src/actions/minDigitChars/minDigitChars.ts` at line 22, The action's type discriminator is misnamed as 'min_digits' — update all occurrences of the readonly property "type" in the minDigitChars action definitions to 'min_digit_chars' so the discriminator matches the exported action name minDigitChars and the project naming pattern; change each instance (the readonly type: 'min_digits' declarations at the three spots) and any code that constructs or matches on that string to use 'min_digit_chars' to keep interface/type checks and pattern matching consistent.library/src/actions/minDigitChars/minDigitChars.test.ts (1)
89-95: Decouple expected values from production utility in invalid-case test.The assertion currently uses
_getDigitCountto compute expectedreceived, which can mask defects if the utility regresses.Suggested test-only adjustment
- expectActionIssue( - action, - baseIssue, - ['', 'foo', 'abc1234'], - (value) => `${_getDigitCount(value)}` - ); + const expectedReceived: Record<string, string> = { + '': '0', + foo: '0', + abc1234: '4', + }; + expectActionIssue(action, baseIssue, ['', 'foo', 'abc1234'], (value) => { + return expectedReceived[value]; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@library/src/actions/minDigitChars/minDigitChars.test.ts` around lines 89 - 95, The test "for invalid strings" relies on the production helper _getDigitCount to compute expected `received` values, which can hide regressions; update the test that calls expectActionIssue with action and baseIssue for ['', 'foo', 'abc1234'] to use hard-coded expected strings (e.g. '0', '0', '4' or whatever the correct expected outputs are) instead of calling _getDigitCount, so the test asserts fixed expected results and is decoupled from the production utility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/routes/api/`(actions)/minDigitChars/index.mdx:
- Line 53: The example error message for v.minDigitChars is awkward; update the
example call to use clearer, user-friendly wording—e.g., change
v.minDigitChars(2, 'This string must include not 1, but at least 2 digits.') to
a concise message like 'Must contain at least 2 digits' (or similar clear
phrasing) so the v.minDigitChars example reads naturally for end users.
In `@website/src/routes/api/`(types)/MinDigitCharsAction/properties.ts:
- Around line 22-36: Update the href on the MinDigitCharsIssue type reference:
locate the object with name 'MinDigitCharsIssue' (in the properties definition)
and change its href from '../MinDigitCharsAction/' to '../MinDigitCharsIssue/'
so the link points to the correct documentation page for MinDigitCharsIssue.
- Around line 58-72: The BaseValidation generics entry for the custom type
MinDigitCharsIssue currently has an incorrect href pointing to
../MinDigitCharsAction/; update the href for the MinDigitCharsIssue generics
object to ../MinDigitCharsIssue/ so the custom type correctly references the
issue definition (modify the object whose name is 'MinDigitCharsIssue' inside
the generics array to use href: '../MinDigitCharsIssue/').
---
Nitpick comments:
In `@library/src/actions/minDigitChars/minDigitChars.test.ts`:
- Around line 89-95: The test "for invalid strings" relies on the production
helper _getDigitCount to compute expected `received` values, which can hide
regressions; update the test that calls expectActionIssue with action and
baseIssue for ['', 'foo', 'abc1234'] to use hard-coded expected strings (e.g.
'0', '0', '4' or whatever the correct expected outputs are) instead of calling
_getDigitCount, so the test asserts fixed expected results and is decoupled from
the production utility.
In `@library/src/actions/minDigitChars/minDigitChars.ts`:
- Line 22: The action's type discriminator is misnamed as 'min_digits' — update
all occurrences of the readonly property "type" in the minDigitChars action
definitions to 'min_digit_chars' so the discriminator matches the exported
action name minDigitChars and the project naming pattern; change each instance
(the readonly type: 'min_digits' declarations at the three spots) and any code
that constructs or matches on that string to use 'min_digit_chars' to keep
interface/type checks and pattern matching consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d03e0459-1543-4efa-90d6-82ea7a3d965f
📒 Files selected for processing (34)
.gitignorelibrary/src/actions/index.tslibrary/src/actions/minDigitChars/index.tslibrary/src/actions/minDigitChars/minDigitChars.test-d.tslibrary/src/actions/minDigitChars/minDigitChars.test.tslibrary/src/actions/minDigitChars/minDigitChars.tslibrary/src/utils/_getDigitCount/_getDigitCount.test.tslibrary/src/utils/_getDigitCount/_getDigitCount.tslibrary/src/utils/_getDigitCount/index.tslibrary/src/utils/index.tswebsite/src/routes/api/(actions)/minDigitChars/index.mdxwebsite/src/routes/api/(actions)/minDigitChars/properties.tswebsite/src/routes/api/(async)/customAsync/index.mdxwebsite/src/routes/api/(async)/fallbackAsync/index.mdxwebsite/src/routes/api/(async)/intersectAsync/index.mdxwebsite/src/routes/api/(async)/lazyAsync/index.mdxwebsite/src/routes/api/(async)/pipeAsync/index.mdxwebsite/src/routes/api/(async)/unionAsync/index.mdxwebsite/src/routes/api/(methods)/config/index.mdxwebsite/src/routes/api/(methods)/fallback/index.mdxwebsite/src/routes/api/(methods)/pipe/index.mdxwebsite/src/routes/api/(schemas)/any/index.mdxwebsite/src/routes/api/(schemas)/custom/index.mdxwebsite/src/routes/api/(schemas)/intersect/index.mdxwebsite/src/routes/api/(schemas)/lazy/index.mdxwebsite/src/routes/api/(schemas)/string/index.mdxwebsite/src/routes/api/(schemas)/union/index.mdxwebsite/src/routes/api/(schemas)/unknown/index.mdxwebsite/src/routes/api/(types)/MinDigitCharsAction/index.mdxwebsite/src/routes/api/(types)/MinDigitCharsAction/properties.tswebsite/src/routes/api/(types)/MinDigitCharsIssue/index.mdxwebsite/src/routes/api/(types)/MinDigitCharsIssue/properties.tswebsite/src/routes/api/menu.mdwebsite/src/routes/guides/(main-concepts)/pipelines/index.mdx
| { | ||
| type: 'custom', | ||
| name: 'MinDigitCharsIssue', | ||
| href: '../MinDigitCharsAction/', | ||
| generics: [ | ||
| { | ||
| type: 'custom', | ||
| name: 'TInput', | ||
| }, | ||
| { | ||
| type: 'custom', | ||
| name: 'TRequirement', | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Incorrect href for MinDigitCharsIssue reference.
The MinDigitCharsIssue type should link to ../MinDigitCharsIssue/ rather than ../MinDigitCharsAction/. This will result in incorrect navigation when users click on the MinDigitCharsIssue link in the documentation.
🔧 Proposed fix
{
type: 'custom',
name: 'MinDigitCharsIssue',
- href: '../MinDigitCharsAction/',
+ href: '../MinDigitCharsIssue/',
generics: [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| type: 'custom', | |
| name: 'MinDigitCharsIssue', | |
| href: '../MinDigitCharsAction/', | |
| generics: [ | |
| { | |
| type: 'custom', | |
| name: 'TInput', | |
| }, | |
| { | |
| type: 'custom', | |
| name: 'TRequirement', | |
| }, | |
| ], | |
| }, | |
| { | |
| type: 'custom', | |
| name: 'MinDigitCharsIssue', | |
| href: '../MinDigitCharsIssue/', | |
| generics: [ | |
| { | |
| type: 'custom', | |
| name: 'TInput', | |
| }, | |
| { | |
| type: 'custom', | |
| name: 'TRequirement', | |
| }, | |
| ], | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/routes/api/`(types)/MinDigitCharsAction/properties.ts around
lines 22 - 36, Update the href on the MinDigitCharsIssue type reference: locate
the object with name 'MinDigitCharsIssue' (in the properties definition) and
change its href from '../MinDigitCharsAction/' to '../MinDigitCharsIssue/' so
the link points to the correct documentation page for MinDigitCharsIssue.
| { | ||
| type: 'custom', | ||
| name: 'MinDigitCharsIssue', | ||
| href: '../MinDigitCharsAction/', | ||
| generics: [ | ||
| { | ||
| type: 'custom', | ||
| name: 'TInput', | ||
| }, | ||
| { | ||
| type: 'custom', | ||
| name: 'TRequirement', | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Same href issue for MinDigitCharsIssue in BaseValidation generics.
The MinDigitCharsIssue reference in the BaseValidation generics also incorrectly links to ../MinDigitCharsAction/ instead of ../MinDigitCharsIssue/.
🔧 Proposed fix
{
type: 'custom',
name: 'MinDigitCharsIssue',
- href: '../MinDigitCharsAction/',
+ href: '../MinDigitCharsIssue/',
generics: [📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| type: 'custom', | |
| name: 'MinDigitCharsIssue', | |
| href: '../MinDigitCharsAction/', | |
| generics: [ | |
| { | |
| type: 'custom', | |
| name: 'TInput', | |
| }, | |
| { | |
| type: 'custom', | |
| name: 'TRequirement', | |
| }, | |
| ], | |
| }, | |
| { | |
| type: 'custom', | |
| name: 'MinDigitCharsIssue', | |
| href: '../MinDigitCharsIssue/', | |
| generics: [ | |
| { | |
| type: 'custom', | |
| name: 'TInput', | |
| }, | |
| { | |
| type: 'custom', | |
| name: 'TRequirement', | |
| }, | |
| ], | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/src/routes/api/`(types)/MinDigitCharsAction/properties.ts around
lines 58 - 72, The BaseValidation generics entry for the custom type
MinDigitCharsIssue currently has an incorrect href pointing to
../MinDigitCharsAction/; update the href for the MinDigitCharsIssue generics
object to ../MinDigitCharsIssue/ so the custom type correctly references the
issue definition (modify the object whose name is 'MinDigitCharsIssue' inside
the generics array to use href: '../MinDigitCharsIssue/').
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Sorry for the delay. I try to catch up in the next few weeks. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| /** | ||
| * Creates a min digits validation action. | ||
| * | ||
| * @param requirement The minimum digits. | ||
| * | ||
| * @returns A min digits action. | ||
| */ | ||
| export function minDigitChars< | ||
| TInput extends string, | ||
| const TRequirement extends number, | ||
| >( | ||
| requirement: TRequirement | ||
| ): MinDigitCharsAction<TInput, TRequirement, undefined>; |
| */ | ||
| // @__NO_SIDE_EFFECTS__ | ||
| export function _getDigitCount(input: string): number { | ||
| return (input.match(/\d/gu) || []).length; |
| type: 'custom', | ||
| name: 'ErrorMessage', | ||
| href: '../ErrorMessage/', | ||
| generics: [ | ||
| { | ||
| type: 'custom', | ||
| name: 'MinDigitCharsIssue', | ||
| href: '../MinDigitCharsAction/', | ||
| generics: [ | ||
| { | ||
| type: 'custom', | ||
| name: 'TInput', | ||
| }, | ||
| { | ||
| type: 'custom', | ||
| name: 'TRequirement', | ||
| }, | ||
| ], | ||
| }, |
| BaseValidation: { | ||
| modifier: 'extends', | ||
| type: { | ||
| type: 'custom', | ||
| name: 'BaseValidation', | ||
| href: '../BaseValidation/', | ||
| generics: [ | ||
| { | ||
| type: 'custom', | ||
| name: 'TInput', | ||
| }, | ||
| { | ||
| type: 'custom', | ||
| name: 'TInput', | ||
| }, | ||
| { | ||
| type: 'custom', | ||
| name: 'MinDigitCharsIssue', | ||
| href: '../MinDigitCharsAction/', | ||
| generics: [ | ||
| { | ||
| type: 'custom', | ||
| name: 'TInput', | ||
| }, | ||
| { | ||
| type: 'custom', | ||
| name: 'TRequirement', | ||
| }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, |
| export * from './minEntries/index.ts'; | ||
| export * from './minDigitChars/index.ts'; |
|
@will-stone Hi. Are you interested in further developing this pull request to bring it to a merge? If so, I'll review it. Unfortunately, I don't think we can release just minDigitChars; the API needs to be symmetrical. |
|
Hi @yslpn Ah, sorry! I've not had much time to review this. I'm more than happy if someone else would like to take the reins or close this off for a future time. Apologies I was unable to finish it. |
Hi Fabian 👋 🙂
Long time fan of this project. Thought I'd make a contribution...
Added a check for minimum amount of digits in a string. Useful for password validation. Only thing I haven't done is update the i18n package, are we expected to do that?
Also, I noticed that when running
pnpm -F valibot testit creates atsconfig.vitest-temp.jsonfile so have added a commit to ignore that.Thanks,
Will.
Summary by CodeRabbit
minDigitCharsvalidation action for strings, plus a digit-count utility used by the action.minDigitCharsconstruction, and validation/inference behavior.minDigitCharsand its related types, and updated API listings/menus to include it.