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

Jump to bottom

fix(text-validation) - pass only values that are strings #12

Merged
merged 12 commits into from
Jun 21, 2023
Merged

fix(text-validation) - pass only values that are strings #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
510b0e7
fix(text-validation) - pass only values that are strings
gabrielremote Jun 9, 2023
7353ade
Release 0.1.1-dev.20230613092121
gabrielremote Jun 13, 2023
a45443e
docs(text) - add assertion for better confidence
gabrielremote Jun 14, 2023
d8ba813
fix(text) - fix text coercion with strict
gabrielremote Jun 14, 2023
c55c71c
Release 0.1.1-dev.20230614173637
gabrielremote Jun 14, 2023
d1f3c27
fix(text) - update yup and use context.originalValue
gabrielremote Jun 15, 2023
12b420b
remove strict references
gabrielremote Jun 15, 2023
e2f7e59
Release 0.1.1-dev.20230616112349
gabrielremote Jun 16, 2023
6fbd8ce
docs(json-schemas) - fix nullables
gabrielremote Jun 19, 2023
3115f87
Release 0.1.1-dev.20230619163824
gabrielremote Jun 19, 2023
af4df92
revert package
gabrielremote Jun 19, 2023
31b468a
allow 0 values
gabrielremote Jun 19, 2023
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
49 changes: 11 additions & 38 deletions package-lock.json
View file
Open in desktop

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

3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"scripts": {
"build": "node scripts/build.js",
"test": "jest",
"test:watch": "jest --watchAll",
"lint": "eslint \"src/**/*.{js,ts}\"",
"format": "npm run format:prettier && npm run format:eslint",
"prepare": "husky install",
Expand All @@ -46,7 +47,7 @@
"dependencies": {
"lodash": "^4.17.21",
"randexp": "^0.5.3",
"yup": "^0.29.1"
"yup": "^0.30.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

todo: Just to make it clear, @gabrielseco and I had a huddle last week: Before we merge this PR, ideally we'd like our internal codebase to also use Yup 0.30 so we don't have Yup twice in our bundles.

Only then we'd merge this PR. WDYT @brennj?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have a MR#17451 in our internal codebase.

My plan is to merge this PR and create a release, what do you think @sandrina-p?

After the release I'll update the MR in our internal codebase

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gabrielseco , I'm answering your question here because Github doesn't support threads is comments detached from code.

I'll create a minor version because we're updating the yup version.
If it was only the fix probably, I'd do a patch.
What do you think @sandrina-p @brennj?

I agree!

},
"devDependencies": {
"@babel/core": "^7.21.5",
Expand Down
32 changes: 18 additions & 14 deletions src/tests/createHeadlessForm.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ describe('createHeadlessForm', () => {

describe('field support', () => {
it('support "text" field type', () => {
const { fields } = createHeadlessForm(schemaInputTypeText);
const { fields, handleValidation } = createHeadlessForm(schemaInputTypeText);

expect(fields[0]).toMatchObject({
description: 'The number of your national identification (max 10 digits)',
Expand All @@ -436,9 +436,13 @@ describe('createHeadlessForm', () => {

const fieldValidator = fields[0].schema;
expect(fieldValidator.isValidSync('CI007')).toBe(true);
expect(fieldValidator.isValidSync(true)).toBe(true); // @BUG RMT-446 - cannot be a bool
expect(fieldValidator.isValidSync(1)).toBe(true); // @BUG RMT-446 - cannot be a number
expect(fieldValidator.isValidSync(0)).toBe(true); // @BUG RMT-446 - cannot be a number
expect(fieldValidator.isValidSync(true)).toBe(false);
expect(fieldValidator.isValidSync(1)).toBe(false);
expect(fieldValidator.isValidSync(0)).toBe(false);

expect(handleValidation({ id_number: 1 }).formErrors).toEqual({
id_number: 'id_number must be a `string` type, but the final value was: `1`.',
});

expect(() => fieldValidator.validateSync('')).toThrowError('Required field');
});
Expand Down Expand Up @@ -2313,7 +2317,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_tabs: 'no',
a_fieldset: {
id_number: 123,
id_number: '123',
},
mandatory_group_array: 'no',
})
Expand All @@ -2328,7 +2332,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_tabs: 'yes',
a_fieldset: {
id_number: 123,
id_number: '123',
},
mandatory_group_array: 'no',
})
Expand All @@ -2344,7 +2348,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_tabs: 'yes',
a_fieldset: {
id_number: 123,
id_number: '123',
},
mandatory_group_array: 'yes',
a_group_array: [{ full_name: 'adfs' }],
Expand All @@ -2355,7 +2359,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_tabs: 'yes',
a_fieldset: {
id_number: 123,
id_number: '123',
tabs: 2,
},
mandatory_group_array: 'no',
Expand Down Expand Up @@ -2446,7 +2450,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_fieldset: ['id_number'],
a_fieldset: {
id_number: 123,
id_number: '123',
},
})
).toBeUndefined();
Expand All @@ -2455,7 +2459,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_fieldset: ['id_number', 'all'],
a_fieldset: {
id_number: 123,
id_number: '123',
},
})
).toEqual({
Expand All @@ -2468,7 +2472,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_fieldset: ['id_number', 'all'],
a_fieldset: {
id_number: 123,
id_number: '123',
tabs: 2,
},
})
Expand All @@ -2483,7 +2487,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_fieldset: ['id_number'],
a_fieldset: {
id_number: 123,
id_number: '123',
},
})
).toBeUndefined();
Expand All @@ -2492,7 +2496,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_fieldset: ['id_number', 'all'],
a_fieldset: {
id_number: 123,
id_number: '123',
},
})
).toEqual({
Expand All @@ -2505,7 +2509,7 @@ describe('createHeadlessForm', () => {
validateForm({
validate_fieldset: ['id_number', 'all'],
a_fieldset: {
id_number: 123,
id_number: '123',
tabs: 2,
},
})
Expand Down
16 changes: 15 additions & 1 deletion src/yupSchema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,22 @@ const todayDateHint = new Date().toISOString().substring(0, 10);
const convertBytesToKB = convertDiskSizeFromTo('Bytes', 'KB');
const convertKbBytesToMB = convertDiskSizeFromTo('KB', 'MB');

const validateOnlyStrings = string()
.trim()
.nullable()
.test(
'is-string',
'${path} must be a `string` type, but the final value was: `${value}`.',
(value, context) => {
if (context.originalValue !== null && context.originalValue !== undefined) {
return typeof context.originalValue === 'string';
}
return true;
}
);

const yupSchemas = {
text: string().trim().nullable(),
text: validateOnlyStrings,
select: string().trim().nullable(),
radio: string().trim().nullable(),
date: string()
Expand Down