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.

Sign up for GitHub

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

Support multiple errors messages #32

Merged
merged 12 commits into from
Jul 23, 2020
Merged

Support multiple errors messages #32

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4e149bf
support-multiple-errors-message
Paul-Vandell Jul 19, 2020
dba4bb9
rebase rhf/resolver master
Paul-Vandell Jul 19, 2020
97d43af
take feedback comment
kotarella1110 Jul 20, 2020
9b91c0a
improve parseErrorSchema
kotarella1110 Jul 21, 2020
8367d6a
fix early issue
kotarella1110 Jul 21, 2020
1ce4bbd
refactor parseErrorSchema
kotarella1110 Jul 21, 2020
fabc1ab
remove true because message is non undefined
kotarella1110 Jul 21, 2020
1836657
improve unit test
kotarella1110 Jul 21, 2020
a43f559
reduce code size by a few bytes
kotarella1110 Jul 21, 2020
d720aa2
Merge branch 'kotarella1110-take-feedback' into feature/support-multi…
Paul-Vandell Jul 21, 2020
e33e7f2
feat(yup): add validate all field criteria logic on parsed errors
Paul-Vandell Jul 21, 2020
e998e69
feat(context): prevent user to not used the yup option context
Paul-Vandell Jul 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 83 additions & 1 deletion src/__snapshots__/yup.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,118 @@ Object {
"age": Object {
"message": "age is a required field",
"type": "required",
"types": Object {
"required": "age is a required field",
},
Copy link
Member

Choose a reason for hiding this comment

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

If validateAllFieldCriteria is false, types field shouldn't be included.

},
"name": Object {
"message": "name is a required field",
"type": "required",
"types": Object {
"min": "name is a min field",
},
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

},
},
"values": Object {},
}
`;

exports[`yupResolver errors should get errors with validate all criteria fields 1`] = `
Object {
"errors": Object {
"age": Object {
"message": "age must be a \`number\` type, but the final value was: \`NaN\` (cast from the value \`\\"test\\"\`).",
"type": "typeError",
"types": Object {
"typeError": Array [
"age must be a \`number\` type, but the final value was: \`NaN\` (cast from the value \`\\"test\\"\`).",
],
},
},
"createdOn": Object {
"message": "createdOn must be a \`date\` type, but the final value was: \`Invalid Date\`.",
"type": "typeError",
"types": Object {
"typeError": Array [
"createdOn must be a \`date\` type, but the final value was: \`Invalid Date\`.",
],
},
},
"foo": Array [
Object {
"loose": Object {
"message": "foo[0].loose must be a \`boolean\` type, but the final value was: \`null\`.
If \\"null\\" is intended as an empty value be sure to mark the schema as \`.nullable()\`",
"type": "typeError",
"types": Object {
"typeError": Array [
"foo[0].loose must be a \`boolean\` type, but the final value was: \`null\`.
If \\"null\\" is intended as an empty value be sure to mark the schema as \`.nullable()\`",
],
},
},
},
],
"password": Object {
"message": "password is a required field",
"type": "required",
"types": Object {
"matches": Array [
"Special",
"Number",
"Uppercase",
"Lowercase",
],
"min": Array [
"password must be at least 8 characters",
],
"required": Array [
"password is a required field",
],
},
},
},
"values": Object {},
}
`;

exports[`yupResolver should get errors 1`] = `
exports[`yupResolver errors should get errors without validate all criteria fields 1`] = `
Object {
"errors": Object {
"age": Object {
"message": "age must be a \`number\` type, but the final value was: \`NaN\` (cast from the value \`\\"test\\"\`).",
"type": "typeError",
"types": Object {
"typeError": "age must be a \`number\` type, but the final value was: \`NaN\` (cast from the value \`\\"test\\"\`).",
},
},
"createdOn": Object {
"message": "createdOn must be a \`date\` type, but the final value was: \`Invalid Date\`.",
"type": "typeError",
"types": Object {
"typeError": "createdOn must be a \`date\` type, but the final value was: \`Invalid Date\`.",
},
},
"foo": Array [
Object {
"loose": Object {
"message": "foo[0].loose must be a \`boolean\` type, but the final value was: \`null\`.
If \\"null\\" is intended as an empty value be sure to mark the schema as \`.nullable()\`",
"type": "typeError",
"types": Object {
"typeError": "foo[0].loose must be a \`boolean\` type, but the final value was: \`null\`.
If \\"null\\" is intended as an empty value be sure to mark the schema as \`.nullable()\`",
},
},
},
],
"password": Object {
"message": "password is a required field",
"type": "required",
"types": Object {
"required": "password is a required field",
},
},
},
"values": Object {},
}
Expand Down
109 changes: 101 additions & 8 deletions src/yup.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ const schema = yup.object().shape({
name: yup.string().required(),
age: yup.number().required().positive().integer(),
email: yup.string().email(),
password: yup
.string()
.required()
.min(8)
.matches(RegExp('(.*[a-z].*)'), 'Lowercase')
.matches(RegExp('(.*[A-Z].*)'), 'Uppercase')
.matches(RegExp('(.*\\d.*)'), 'Number')
.matches(RegExp('[!@#$%^&*(),.?":{}|<>]'), 'Special'),
website: yup.string().url(),
createdOn: yup.date().default(function () {
return new Date();
Expand All @@ -64,6 +72,7 @@ describe('yupResolver', () => {
const data = {
name: 'jimmy',
age: '24',
password: '[}tehk6Uor',
createdOn: '2014-09-23T19:25:25Z',
foo: [{ yup: true }],
};
Expand All @@ -72,20 +81,101 @@ describe('yupResolver', () => {
values: {
name: 'jimmy',
age: 24,
password: '[}tehk6Uor',
foo: [{ yup: true }],
createdOn: new Date('2014-09-23T19:25:25Z'),
},
});
});

it('should get errors', async () => {
const data = {
name: 2,
age: 'test',
createdOn: null,
foo: [{ loose: null }],
};
expect(await yupResolver(schema)(data)).toMatchSnapshot();
describe('errors', () => {
it('should get errors with validate all criteria fields', async () => {
const data = {
name: 2,
age: 'test',
password: '',
createdOn: null,
foo: [{ loose: null }],
};
const resolve = await yupResolver(schema)(data, {}, true);
expect(resolve).toMatchSnapshot();
expect(resolve.errors['age'].types).toMatchInlineSnapshot(`
Object {
"typeError": Array [
"age must be a \`number\` type, but the final value was: \`NaN\` (cast from the value \`\\"test\\"\`).",
],
}
`);
expect(resolve.errors['createdOn'].types).toMatchInlineSnapshot(`
Object {
"typeError": Array [
"createdOn must be a \`date\` type, but the final value was: \`Invalid Date\`.",
],
}
`);
expect(resolve.errors['password'].types).toMatchInlineSnapshot(`
Object {
"matches": Array [
"Special",
"Number",
"Uppercase",
"Lowercase",
],
"min": Array [
"password must be at least 8 characters",
],
"required": Array [
"password is a required field",
],
}
`);
});
Copy link
Member

Choose a reason for hiding this comment

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

The structure I had in mind was:

expect(resolve.errors['password'].types).toMatchInlineSnapshot(`
  Object {
    "matches": Array [
      "Special",
      "Number",
      "Uppercase",
      "Lowercase",
    ],
    "min": "password must be at least 8 characters",
    "required": "password is a required field",
  }
`);

We should discuss whether all should be array message when validateAllFieldCriteria is true.

Copy link
Member

@bluebill1049 bluebill1049 Jul 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, @kotarella1110. Above is a breaking change, we should stick with

expect(resolve.errors['password'].types).toMatchInlineSnapshot(`
  Object {
    "matches": Array [
      "Special",
      "Number",
      "Uppercase",
      "Lowercase",
    ],
    "min": "password must be at least 8 characters",
    "required": "password is a required field",
  }
`);

Copy link
Member

Choose a reason for hiding this comment

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

@bluebill1049 I agree with you 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, i was thinking if we keep this mix structure, i'm afraid that we have to handle the case where messages can be an array or a string in the code for every property in the schema. This is why i keep every messages in array format when validateAllCriteria is set to true. But you are in better place to take the right decision here :).

Copy link
Member

Choose a reason for hiding this comment

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

Thanks very much for understanding and putting this PR together!


it('should get errors without validate all criteria fields', async () => {
const data = {
name: 2,
age: 'test',
createdOn: null,
foo: [{ loose: null }],
};
const resolve = await yupResolver(schema)(data);
expect(await yupResolver(schema)(data)).toMatchSnapshot();
expect(resolve.errors['age'].types).toMatchInlineSnapshot(`
Object {
"typeError": "age must be a \`number\` type, but the final value was: \`NaN\` (cast from the value \`\\"test\\"\`).",
}
`);
expect(resolve.errors['createdOn'].types).toMatchInlineSnapshot(`
Object {
"typeError": "createdOn must be a \`date\` type, but the final value was: \`Invalid Date\`.",
}
`);
expect(resolve.errors['password'].types).toMatchInlineSnapshot(`
Object {
"required": "password is a required field",
}
`);
});

it('should get error if yup errors has no inner errors', async () => {
const data = {
name: 2,
age: 'test',
createdOn: null,
foo: [{ loose: null }],
};
const resolve = await yupResolver(schema, {
abortEarly: true,
})(data);
expect(resolve.errors).toMatchInlineSnapshot(`
Object {
"createdOn": Object {
"message": "createdOn must be a \`date\` type, but the final value was: \`Invalid Date\`.",
"type": "typeError",
},
}
`);
});
});

it('should pass down the yup context', async () => {
Expand Down Expand Up @@ -148,6 +238,9 @@ describe('validateWithSchema', () => {
"name": Object {
"message": "name must be at least 6 characters",
"type": "min",
"types": Object {
"min": "name must be at least 6 characters",
},
},
},
"values": Object {},
Expand Down
69 changes: 35 additions & 34 deletions src/yup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,43 +1,44 @@
import { appendErrors, transformToNestObject, Resolver } from 'react-hook-form';
import { Resolver, transformToNestObject } from 'react-hook-form';
import { FieldError, FieldErrors } from 'react-hook-form/dist/types/form';
import { DeepMap } from 'react-hook-form/dist/types/utils';
import Yup from 'yup';

const parseErrorSchema = (
const parseErrorSchema = <TFieldValues extends Record<string, any>>(
error: Yup.ValidationError,
validateAllFieldCriteria: boolean,
) =>
Array.isArray(error.inner)
? error.inner.reduce(
(previous: Record<string, any>, { path, message, type }) => ({
...previous,
...(path
? previous[path] && validateAllFieldCriteria
? {
[path]: appendErrors(
path,
validateAllFieldCriteria,
previous,
type,
message,
),
}
: {
[path]: previous[path] || {
message,
type,
...(validateAllFieldCriteria
? {
types: { [type]: message || true },
}
: {}),
},
}
: {}),
}),
): DeepMap<TFieldValues, FieldError> =>
Array.isArray(error.inner) && error.inner.length
? error.inner.reduce<FieldErrors<TFieldValues>>(
(previous, { path, message, type }) => {
const previousPath = previous[path] as FieldErrors | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use this variable inline (save few bytes)

const previousTypes = validateAllFieldCriteria
? previousPath?.types || []
: {};
return {
...previous,
[path]: {
...(previous[path] || {
message,
type,
types: previousTypes,
}),
types: {
Copy link
Member

Choose a reason for hiding this comment

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

types should be optional.

...previousTypes,
[type]: validateAllFieldCriteria
? [message, ...(previousTypes[type] || [])]
: message,
},
},
};
},
{},
)
: {
[error.path]: { message: error.message, type: error.type },
};
: ({
[error.path]: {
message: error.message,
type: error.type,
},
} as DeepMap<TFieldValues, FieldError>);

export const yupResolver = <TFieldValues extends Record<string, any>>(
schema: Yup.ObjectSchema | Yup.Lazy,
Expand Down