-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(rules): add required_if rule for conditional field validation #5138
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@vee-validate/rules": patch | ||
| --- | ||
|
|
||
| Fix required_if rule not re-evaluating on subsequent submissions (#4868) | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||||||||||||||||||
| import { isEmpty } from './utils'; | ||||||||||||||||||||||
| import { isEmptyArray, isNullOrUndefined } from '../../shared'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const requiredIfValidator = ( | ||||||||||||||||||||||
| value: unknown, | ||||||||||||||||||||||
| params: [unknown, ...unknown[]] | { target: unknown; values?: unknown[] }, | ||||||||||||||||||||||
| ) => { | ||||||||||||||||||||||
| let target: unknown; | ||||||||||||||||||||||
| let values: unknown[] | undefined; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (Array.isArray(params)) { | ||||||||||||||||||||||
| target = params[0]; | ||||||||||||||||||||||
| values = params.slice(1); | ||||||||||||||||||||||
| } else { | ||||||||||||||||||||||
| target = params.target; | ||||||||||||||||||||||
| values = params.values; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Determine if the field should be required | ||||||||||||||||||||||
| let isRequired: boolean; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (values && values.length) { | ||||||||||||||||||||||
| // eslint-disable-next-line eqeqeq | ||||||||||||||||||||||
| isRequired = values.some(val => val == target); | ||||||||||||||||||||||
| } else { | ||||||||||||||||||||||
| isRequired = !isNullOrUndefined(target) && !isEmptyArray(target) && !!String(target).trim().length; | ||||||||||||||||||||||
|
Comment on lines
+25
to
+26
|
||||||||||||||||||||||
| } else { | |
| isRequired = !isNullOrUndefined(target) && !isEmptyArray(target) && !!String(target).trim().length; | |
| } else if (typeof target === 'boolean') { | |
| // For boolean targets, only `true` should make the field required. | |
| isRequired = target; | |
| } else { | |
| isRequired = | |
| !isNullOrUndefined(target) && | |
| !isEmptyArray(target) && | |
| !!String(target).trim().length; |
Copilot
AI
Mar 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the condition is met, this returns !isEmpty(value), which doesn't match the semantics of the existing required rule (e.g. ' ' and false will incorrectly be considered valid). To keep behavior consistent across rules, delegate to the required validator (or mirror its trim/boolean handling) when isRequired is true.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import validate from '../src/required_if'; | ||
|
|
||
| test('validates required_if with array params', () => { | ||
| // target has a matching value, field is empty -> invalid | ||
| expect(validate('', ['Online Banking', 'Online Banking'])).toBe(false); | ||
| expect(validate(null, ['Online Banking', 'Online Banking'])).toBe(false); | ||
| expect(validate(undefined, ['Online Banking', 'Online Banking'])).toBe(false); | ||
| expect(validate([], ['Online Banking', 'Online Banking'])).toBe(false); | ||
|
|
||
| // target has a matching value, field is filled -> valid | ||
| expect(validate('Nabil', ['Online Banking', 'Online Banking'])).toBe(true); | ||
|
|
||
| // target does not match any value, field is empty -> valid (not required) | ||
| expect(validate('', ['Cash', 'Online Banking'])).toBe(true); | ||
| expect(validate(null, ['Cash', 'Online Banking'])).toBe(true); | ||
| expect(validate(undefined, ['Cash', 'Online Banking'])).toBe(true); | ||
| }); | ||
|
Comment on lines
+3
to
+17
|
||
|
|
||
| test('validates required_if with object params', () => { | ||
| // target matches, field is empty -> invalid | ||
| expect(validate('', { target: 'Online Banking', values: ['Online Banking'] })).toBe(false); | ||
|
|
||
| // target matches, field is filled -> valid | ||
| expect(validate('Nabil', { target: 'Online Banking', values: ['Online Banking'] })).toBe(true); | ||
|
|
||
| // target does not match, field is empty -> valid | ||
| expect(validate('', { target: 'Cash', values: ['Online Banking'] })).toBe(true); | ||
| }); | ||
|
|
||
| test('validates required_if with multiple comparison values', () => { | ||
| // target matches one of the values | ||
| expect(validate('', ['Online Banking', 'Online Banking', 'Wire Transfer'])).toBe(false); | ||
| expect(validate('', ['Wire Transfer', 'Online Banking', 'Wire Transfer'])).toBe(false); | ||
| expect(validate('Nabil', ['Online Banking', 'Online Banking', 'Wire Transfer'])).toBe(true); | ||
|
|
||
| // target matches none | ||
| expect(validate('', ['Cash', 'Online Banking', 'Wire Transfer'])).toBe(true); | ||
| }); | ||
|
|
||
| test('validates required_if without comparison values (truthy target)', () => { | ||
| // target is truthy, no comparison values -> field is required | ||
| expect(validate('', ['some value'])).toBe(false); | ||
| expect(validate('filled', ['some value'])).toBe(true); | ||
|
|
||
| // target is empty/falsy, no comparison values -> field is not required | ||
| expect(validate('', [''])).toBe(true); | ||
| expect(validate('', [null])).toBe(true); | ||
| expect(validate('', [undefined])).toBe(true); | ||
| }); | ||
|
|
||
| test('validates required_if with object params and no values array', () => { | ||
| // target is truthy -> required | ||
| expect(validate('', { target: 'truthy' })).toBe(false); | ||
| expect(validate('filled', { target: 'truthy' })).toBe(true); | ||
|
|
||
| // target is falsy -> not required | ||
| expect(validate('', { target: '' })).toBe(true); | ||
| expect(validate('', { target: null })).toBe(true); | ||
| expect(validate('', { target: undefined })).toBe(true); | ||
| }); | ||
|
|
||
| test('uses loose equality for value comparison', () => { | ||
| // '1' == 1 should be true with loose equality | ||
| expect(validate('', ['1', 1])).toBe(false); | ||
| expect(validate('', [1, '1'])).toBe(false); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changeset summary says this fixes
required_ifnot re-evaluating on subsequent submissions, but the PR primarily re-introduces therequired_ifrule into@vee-validate/rules. Please align the changeset text with the actual change so the published changelog is accurate (e.g. mention re-adding/restoring the rule).