From 120726175aa97a9066fb765155ae4fef15b1e0ad Mon Sep 17 00:00:00 2001 From: Jason Quense Date: Wed, 9 Mar 2022 10:43:24 -0500 Subject: [PATCH] fix: partial() BREAKING CHANGE: 'required' no longer adds a test for most schema, to determine if a schema is required, check it's `spec.optional` and `spec.nullable` values, also accessible via `describe()` --- src/array.ts | 10 +++++--- src/date.ts | 7 +++--- src/number.ts | 15 ++++++++---- src/schema.ts | 26 +++----------------- src/string.ts | 36 ++++++++++++++++++++------- src/util/createValidation.ts | 11 ++++++--- test/mixed.ts | 21 +++++----------- test/object.ts | 47 +++++++++++++++++++++++++++++++++++- 8 files changed, 110 insertions(+), 63 deletions(-) diff --git a/src/array.ts b/src/array.ts index dd9a0867b..e1eb99d67 100644 --- a/src/array.ts +++ b/src/array.ts @@ -1,4 +1,3 @@ -import isAbsent from './util/isAbsent'; import isSchema from './util/isSchema'; import printValue from './util/printValue'; import parseJson from './util/parseJson'; @@ -192,8 +191,9 @@ export default class ArraySchema< name: 'length', exclusive: true, params: { length }, + skipAbsent: true, test(value) { - return isAbsent(value) || value.length === this.resolve(length); + return value!.length === this.resolve(length); }, }); } @@ -206,9 +206,10 @@ export default class ArraySchema< name: 'min', exclusive: true, params: { min }, + skipAbsent: true, // FIXME(ts): Array test(value) { - return isAbsent(value) || value.length >= this.resolve(min); + return value!.length >= this.resolve(min); }, }); } @@ -220,8 +221,9 @@ export default class ArraySchema< name: 'max', exclusive: true, params: { max }, + skipAbsent: true, test(value) { - return isAbsent(value) || value.length <= this.resolve(max); + return value!.length <= this.resolve(max); }, }); } diff --git a/src/date.ts b/src/date.ts index 597cbb446..adc171e00 100644 --- a/src/date.ts +++ b/src/date.ts @@ -1,7 +1,6 @@ // @ts-ignore import isoParse from './util/isodate'; import { date as locale } from './locale'; -import isAbsent from './util/isAbsent'; import Ref from './Reference'; import type { AnyObject, Message } from './types'; import type { @@ -85,8 +84,9 @@ export default class DateSchema< name: 'min', exclusive: true, params: { min }, + skipAbsent: true, test(value) { - return isAbsent(value) || value >= this.resolve(limit); + return value! >= this.resolve(limit); }, }); } @@ -99,8 +99,9 @@ export default class DateSchema< name: 'max', exclusive: true, params: { max }, + skipAbsent: true, test(value) { - return isAbsent(value) || value <= this.resolve(limit); + return value! <= this.resolve(limit); }, }); } diff --git a/src/number.ts b/src/number.ts index d2561a8a0..fbd300c3c 100644 --- a/src/number.ts +++ b/src/number.ts @@ -67,8 +67,9 @@ export default class NumberSchema< name: 'min', exclusive: true, params: { min }, + skipAbsent: true, test(value: Maybe) { - return isAbsent(value) || value >= this.resolve(min); + return value! >= this.resolve(min); }, }); } @@ -79,8 +80,9 @@ export default class NumberSchema< name: 'max', exclusive: true, params: { max }, + skipAbsent: true, test(value: Maybe) { - return isAbsent(value) || value <= this.resolve(max); + return value! <= this.resolve(max); }, }); } @@ -91,8 +93,9 @@ export default class NumberSchema< name: 'max', exclusive: true, params: { less }, + skipAbsent: true, test(value: Maybe) { - return isAbsent(value) || value < this.resolve(less); + return value! < this.resolve(less); }, }); } @@ -103,8 +106,9 @@ export default class NumberSchema< name: 'min', exclusive: true, params: { more }, + skipAbsent: true, test(value: Maybe) { - return isAbsent(value) || value > this.resolve(more); + return value! > this.resolve(more); }, }); } @@ -121,7 +125,8 @@ export default class NumberSchema< return this.test({ name: 'integer', message, - test: (val) => isAbsent(val) || Number.isInteger(val), + skipAbsent: true, + test: (val) => Number.isInteger(val), }); } diff --git a/src/schema.ts b/src/schema.ts index b4ccef785..36fd84f5c 100644 --- a/src/schema.ts +++ b/src/schema.ts @@ -627,10 +627,6 @@ export default abstract class Schema< return this.clone({ strict: isStrict }); } - protected _isPresent(value: any) { - return value != null; - } - protected nullability(nullable: boolean, message?: Message) { const next = this.clone({ nullable }); next.internalTests.nullable = createValidation({ @@ -671,27 +667,11 @@ export default abstract class Schema< required(message: Message = locale.required): any { return this.clone().withMutation((next) => - next - .nonNullable(message) - .defined(message) - .test({ - message, - name: 'required', - exclusive: true, - test(value: any) { - return this.schema._isPresent(value); - }, - }), - ) as any; + next.nonNullable(message).defined(message), + ); } - notRequired(): any { - return this.clone().withMutation((next) => { - next.tests = next.tests.filter( - (test) => test.OPTIONS!.name !== 'required', - ); - return next.nullable().optional(); - }); + return this.clone().withMutation((next) => next.nullable().optional()); } transform(fn: TransformFunction) { diff --git a/src/string.ts b/src/string.ts index 6e364b384..7404fba99 100644 --- a/src/string.ts +++ b/src/string.ts @@ -1,4 +1,4 @@ -import { MixedLocale, string as locale } from './locale'; +import { MixedLocale, mixed as mixedLocale, string as locale } from './locale'; import isAbsent from './util/isAbsent'; import type Reference from './Reference'; import type { Message, AnyObject } from './types'; @@ -83,8 +83,22 @@ export default class StringSchema< }); } - protected _isPresent(value: any) { - return super._isPresent(value) && !!value.length; + required(message?: Message) { + return super.required(message).withMutation((schema: this) => + schema.test({ + message: message || mixedLocale.required, + name: 'required', + skipAbsent: true, + test: (value) => !!value!.length, + }), + ); + } + + notRequired() { + return super.notRequired().withMutation((schema: this) => { + schema.tests.filter((t) => t.OPTIONS!.name !== 'required'); + return schema; + }); } length( @@ -96,8 +110,9 @@ export default class StringSchema< name: 'length', exclusive: true, params: { length }, + skipAbsent: true, test(value: Maybe) { - return isAbsent(value) || value.length === this.resolve(length); + return value!.length === this.resolve(length); }, }); } @@ -111,8 +126,9 @@ export default class StringSchema< name: 'min', exclusive: true, params: { min }, + skipAbsent: true, test(value: Maybe) { - return isAbsent(value) || value.length >= this.resolve(min); + return value!.length >= this.resolve(min); }, }); } @@ -126,8 +142,9 @@ export default class StringSchema< exclusive: true, message, params: { max }, + skipAbsent: true, test(value: Maybe) { - return isAbsent(value) || value.length <= this.resolve(max); + return value!.length <= this.resolve(max); }, }); } @@ -153,10 +170,9 @@ export default class StringSchema< name: name || 'matches', message: message || locale.matches, params: { regex }, + skipAbsent: true, test: (value: Maybe) => - isAbsent(value) || - (value === '' && excludeEmptyString) || - value.search(regex) !== -1, + (value === '' && excludeEmptyString) || value!.search(regex) !== -1, }); } @@ -206,6 +222,7 @@ export default class StringSchema< message, name: 'string_case', exclusive: true, + skipAbsent: true, test: (value: Maybe) => isAbsent(value) || value === value.toLowerCase(), }); @@ -218,6 +235,7 @@ export default class StringSchema< message, name: 'string_case', exclusive: true, + skipAbsent: true, test: (value: Maybe) => isAbsent(value) || value === value.toUpperCase(), }); diff --git a/src/util/createValidation.ts b/src/util/createValidation.ts index 7ea771078..9ff261f0a 100644 --- a/src/util/createValidation.ts +++ b/src/util/createValidation.ts @@ -10,6 +10,7 @@ import { } from '../types'; import Reference from '../Reference'; import type { AnySchema } from '../schema'; +import isAbsent from './isAbsent'; export type PanicCallback = (err: Error) => void; @@ -58,6 +59,7 @@ export type TestConfig = { test: TestFunction; params?: ExtraParams; exclusive?: boolean; + skipAbsent?: boolean; }; export type Test = (( @@ -73,6 +75,7 @@ export default function createValidation(config: { test: TestFunction; params?: ExtraParams; message?: Message; + skipAbsent?: boolean; }) { function validate( { @@ -88,7 +91,7 @@ export default function createValidation(config: { panic: PanicCallback, next: NextCallback, ) { - const { name, test, params, message } = config; + const { name, test, params, message, skipAbsent } = config; let { parent, context, abortEarly = rest.schema.spec.abortEarly } = options; function resolve(item: T | Reference) { @@ -144,9 +147,11 @@ export default function createValidation(config: { else panic(err); }; + const shouldSkip = skipAbsent && isAbsent(value); + if (!sync) { try { - Promise.resolve(test.call(ctx, value, ctx)).then( + Promise.resolve(!shouldSkip ? test.call(ctx, value, ctx) : true).then( handleResult, handleError, ); @@ -159,7 +164,7 @@ export default function createValidation(config: { let result: ReturnType; try { - result = test.call(ctx, value, ctx); + result = !shouldSkip ? test.call(ctx, value, ctx) : true; if (typeof (result as any)?.then === 'function') { throw new Error( diff --git a/test/mixed.ts b/test/mixed.ts index 7bc0d2092..d4e4789c2 100644 --- a/test/mixed.ts +++ b/test/mixed.ts @@ -711,10 +711,11 @@ describe('Mixed Types ', () => { await expect(inst.isValid('a')).resolves.toBe(true); }); - it('concat should maintain explicit presence', async function () { - let inst = string().required().concat(string()); + it('concat should override presence', async function () { + let inst = string().required().concat(string().nullable()); - await expect(inst.isValid(undefined)).resolves.toBe(false); + await expect(inst.isValid(undefined)).resolves.toBe(true); + await expect(inst.isValid(null)).resolves.toBe(true); }); it('gives whitelist precedence to second in concat', async function () { @@ -968,12 +969,7 @@ describe('Mixed Types ', () => { label: undefined, nullable: false, optional: false, - tests: [ - { - name: 'required', - params: undefined, - }, - ], + tests: [], oneOf: [], notOneOf: [], innerType: { @@ -1040,12 +1036,7 @@ describe('Mixed Types ', () => { label: undefined, nullable: false, optional: false, - tests: [ - { - name: 'required', - params: undefined, - }, - ], + tests: [], oneOf: [], notOneOf: [], innerType: { diff --git a/test/object.ts b/test/object.ts index c16726c45..9526be90b 100644 --- a/test/object.ts +++ b/test/object.ts @@ -567,7 +567,7 @@ describe('Object types', () => { }); }); - it('should respect abortEarly', () => { + it('should respect abortEarly', async () => { let inst = object({ nest: object({ str: string().required(), @@ -672,6 +672,51 @@ describe('Object types', () => { ]); }); + it('partial() should work', async () => { + let inst = object({ + age: number().required(), + name: string().required(), + }); + + await expect(inst.isValid({ age: null, name: '' })).resolves.toEqual(false); + + await expect(inst.partial().isValid({})).resolves.toEqual(true); + + await expect(inst.partial().isValid({ age: null })).resolves.toEqual(false); + await expect(inst.partial().isValid({ name: '' })).resolves.toEqual(false); + }); + + xit('deepPartial() should work', async () => { + let inst = object({ + age: number().required(), + name: string().required(), + contacts: array( + object({ + name: string().required(), + age: number().required(), + }), + ).defined(), + }); + + await expect(inst.isValid({ age: 2, name: 'fs' })).resolves.toEqual(false); + await expect( + inst.isValid({ age: 2, name: 'fs', contacts: [{}] }), + ).resolves.toEqual(false); + + await expect(inst.deepPartial().isValid({})).resolves.toEqual(true); + await expect( + inst.deepPartial().validate({ contacts: [{}] }), + ).resolves.toEqual(true); + + await expect( + inst.deepPartial().isValid({ contacts: [{ age: null }] }), + ).resolves.toEqual(false); + + await expect( + inst.deepPartial().isValid({ contacts: [{ age: null }] }), + ).resolves.toEqual(false); + }); + it('should alias or move keys', () => { let inst = object() .shape({