Skip to content

Commit

Permalink
fix(valibotResolver): infinite loop (#655)
Browse files Browse the repository at this point in the history
* update valibot version

New version includes new methods. In particular for this branch `variant()` which can cause an infinite loop

* this is a not a zod test

* add new fixtures and tests to prove the issue

* ensure exit condition is always met
  • Loading branch information
richardvanbergen authored Dec 26, 2023
1 parent 3805d3e commit 6f1b139
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 13 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@
"superstruct": "^1.0.3",
"typanion": "^3.14.0",
"typescript": "^5.1.6",
"valibot": "^0.12.0",
"valibot": "^0.24.1",
"vest": "^4.6.11",
"vite": "^4.4.9",
"vite-tsconfig-paths": "^4.2.0",
Expand Down
8 changes: 4 additions & 4 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion valibot/src/__tests__/Form-native-validation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function TestComponent({ onSubmit }: Props) {
);
}

test("form's native validation with Zod", async () => {
test("form's native validation with Valibot", async () => {
const handleSubmit = vi.fn();
render(<TestComponent onSubmit={handleSubmit} />);

Expand Down
20 changes: 18 additions & 2 deletions valibot/src/__tests__/__fixtures__/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import {
array,
boolean,
required,
union
union,
variant,
literal,
} from 'valibot';

export const schema = required(
Expand All @@ -29,7 +31,13 @@ export const schema = required(
minLength(8, 'Must be at least 8 characters in length'),
]),
repeatPassword: string('Repeat Password is required'),
accessToken: union([string('Access token should be a string'), number('Access token should be a number')], "access token is required"),
accessToken: union(
[
string('Access token should be a string'),
number('Access token should be a number'),
],
'access token is required',
),
birthYear: number('Please enter your birth year', [
minValue(1900),
maxValue(2013),
Expand All @@ -46,6 +54,14 @@ export const schema = required(
}),
);

export const schemaError = variant('type', [
object({ type: literal('a') }),
object({ type: literal('b') }),
]);

export const validSchemaErrorData = { type: 'a' };
export const invalidSchemaErrorData = { type: 'c' };

export const validData = {
username: 'Doe',
password: 'Password123_',
Expand Down
40 changes: 39 additions & 1 deletion valibot/src/__tests__/valibot.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
/* eslint-disable no-console, @typescript-eslint/ban-ts-comment */
import { valibotResolver } from '..';
import { schema, validData, fields, invalidData } from './__fixtures__/data';
import {
schema,
validData,
fields,
invalidData,
schemaError,
validSchemaErrorData,
invalidSchemaErrorData,
} from './__fixtures__/data';
import * as valibot from 'valibot';

const shouldUseNativeValidation = false;
Expand Down Expand Up @@ -98,4 +106,34 @@ describe('valibotResolver', () => {

expect(result).toMatchSnapshot();
});

it('should be able to validate variants', async () => {
const result = await valibotResolver(schemaError, undefined, {
mode: 'sync',
})(validSchemaErrorData, undefined, {
fields,
shouldUseNativeValidation,
});

expect(result).toEqual({
errors: {},
values: {
type: 'a',
},
});
});

it('should exit issue resolution if no path is set', async () => {
const result = await valibotResolver(schemaError, undefined, {
mode: 'sync',
})(invalidSchemaErrorData, undefined, {
fields,
shouldUseNativeValidation,
});

expect(result).toEqual({
errors: {},
values: {},
});
});
});
6 changes: 2 additions & 4 deletions valibot/src/valibot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const parseErrors = (
validateAllFieldCriteria: boolean,
): FieldErrors => {
const errors: Record<string, FieldError> = {};
for (; valiErrors.issues.length; ) {
const error = valiErrors.issues[0];

for (const error of valiErrors.issues) {
if (!error.path) {
continue;
}
Expand All @@ -38,8 +38,6 @@ const parseErrors = (
: error.message,
) as FieldError;
}

valiErrors.issues.shift();
}

return errors;
Expand Down

0 comments on commit 6f1b139

Please sign in to comment.