fix: make null validation errors consistent across schema (#1982)

Previously, schema that parse to invalid values like InvalidDate or NaN always tripped the typeError validation before the nullability error. This was the due to typeError no longer firing on null values, with the expectation that it would always hit the newer nullability check.

This change fixes the issue for Number and Date schema, so that they act the same as String and other schema, where null values do not trigger the typeError validation. The consequence of this is that casting null values for non-nullable number schema, now return null instead of NaN and dates, return null instead of InvalidDate. Essentially these schema will not fruitlessly attempt to coerce null into a number or date anymore.

I am marking this as a bug fix since it is, and as with most bug fixes, could also be considered a breaking change. I'm going to optimistically not cut a v2 for this. If you have unexpected changes it is likely due to number/date schema hitting different validation errors (nullability).

Previously for these schema it was common practice to change the typeError for these schema to something like "this is required"assuming the issue was due to invalid nulls. NOW the schema will show the locale.notNull message, so if you may want to adjust that to something more user friendly if you surface default locale messages to users (we don't generally recommend that).

README.md

@@ -193,6 +193,7 @@ const num = number().cast('1'); // 1
 const obj = object({
   firstName: string().lowercase().trim(),
 })
+  .json()
   .camelCase()
   .cast('{"first_name": "jAnE "}'); // { firstName: 'jane' }
 ```
@@ -206,14 +207,14 @@ const reversedString = string()
 ```
 
 Transforms form a "pipeline", where the value of a previous transform is piped into the next one.
-If the end value is `undefined` yup will apply the schema default if it's configured.
+When an input value is `undefined` yup will apply the schema default if it's configured.
 
 > Watch out! values are not guaranteed to be valid types in transform functions. Previous transforms
 > may have failed. For example a number transform may be receive the input value, `NaN`, or a number.
 
 ### Validation: Tests
 
-Yup has robust support for assertions, or "tests", over input values. Tests assert that inputs conform to some
+Yup schema run "tests" over input values. Tests assert that inputs conform to some
 criteria. Tests are distinct from transforms, in that they do not change or alter the input (or its type)
 and are usually reserved for checks that are hard, if not impossible, to represent in static types.
 
@@ -241,7 +242,7 @@ jamesSchema.validateSync('Jane'); // ValidationError "this is not James"
 > Heads up: unlike transforms, `value` in a custom test is guaranteed to be the correct type
 > (in this case an optional string). It still may be `undefined` or `null` depending on your schema
 > in those cases, you may want to return `true` for absent values unless your transform makes presence
-> related assertions
+> related assertions. The test option `skipAbsent` will do this for you if set.
 
 #### Customizing errors
 

src/date.ts

@@ -47,7 +47,10 @@ export default class DateSchema<
 
     this.withMutation(() => {
       this.transform((value, _raw, ctx) => {
-        if (!ctx.spec.coerce || ctx.isType(value)) return value;
+        // null -> InvalidDate isn't useful; treat all nulls as null and let it fail on
+        // nullability check vs TypeErrors
+        if (!ctx.spec.coerce || ctx.isType(value) || value === null)
+          return value;
 
         value = isoParse(value);
 

src/number.ts

@@ -54,7 +54,9 @@ export default class NumberSchema<
           parsed = +parsed;
         }
 
-        if (ctx.isType(parsed)) return parsed;
+        // null -> NaN isn't useful; treat all nulls as null and let it fail on
+        // nullability check vs TypeErrors
+        if (ctx.isType(parsed) || parsed === null) return parsed;
 
         return parseFloat(parsed);
       });

src/object.ts

@@ -320,11 +320,9 @@ export default class ObjectSchema<
     );
   }
 
-  protected _getDefault(
-    options?: ResolveOptions<TContext>,
-  ) {
+  protected _getDefault(options?: ResolveOptions<TContext>) {
     if ('default' in this.spec) {
-      return super._getDefault();
+      return super._getDefault(options);
     }
 
     // if there is no default set invent one
@@ -335,8 +333,20 @@ export default class ObjectSchema<
     let dft: any = {};
     this._nodes.forEach((key) => {
       const field = this.fields[key] as any;
+
+      let innerOptions = options;
+      if (innerOptions?.value) {
+        innerOptions = {
+          ...innerOptions,
+          parent: innerOptions.value,
+          value: innerOptions.value[key],
+        };
+      }
+
       dft[key] =
-        field && 'getDefault' in field ? field.getDefault(options) : undefined;
+        field && 'getDefault' in field
+          ? field.getDefault(innerOptions)
+          : undefined;
     });
 
     return dft;

src/schema.ts

@@ -607,9 +607,7 @@ export default abstract class Schema<
     }
   }
 
-  protected _getDefault(
-    _options?: ResolveOptions<TContext>,
-  ) {
+  protected _getDefault(_options?: ResolveOptions<TContext>) {
     let defaultValue = this.spec.default;
 
     if (defaultValue == null) {
@@ -801,8 +799,9 @@ export default abstract class Schema<
     next.internalTests.typeError = createValidation({
       message,
       name: 'typeError',
+      skipAbsent: true,
       test(value) {
-        if (!isAbsent(value) && !this.schema._typeCheck(value))
+        if (!this.schema._typeCheck(value))
           return this.createError({
             params: {
               type: this.schema.type,

test/date.ts

@@ -1,7 +1,8 @@
 import { ref, date } from '../src';
+import * as TestHelpers from './helpers';
 
-function isValidDate(date: any): date is Date {
-  return date instanceof Date && !isNaN(date.getTime());
+function isInvalidDate(date: any): date is Date {
+  return date instanceof Date && isNaN(date.getTime());
 }
 
 describe('Date types', () => {
@@ -19,13 +20,18 @@ describe('Date types', () => {
     expect(inst.cast('2016-08-10T11:32:19.2125Z')).toEqual(
       new Date(1470828739212),
     );
+
+    expect(inst.cast(null, { assert: false })).toEqual(null);
   });
 
-  it('should return invalid date for failed casts', function () {
+  it('should return invalid date for failed non-null casts', function () {
     let inst = date();
 
-    expect(isValidDate(inst.cast(null, { assert: false }))).toBe(false);
-    expect(isValidDate(inst.cast('', { assert: false }))).toBe(false);
+    expect(inst.cast(null, { assert: false })).toEqual(null);
+    expect(inst.cast(undefined, { assert: false })).toEqual(undefined);
+
+    expect(isInvalidDate(inst.cast('', { assert: false }))).toBe(true);
+    expect(isInvalidDate(inst.cast({}, { assert: false }))).toBe(true);
   });
 
   it('should type check', () => {
@@ -39,7 +45,7 @@ describe('Date types', () => {
   });
 
   it('should VALIDATE correctly', () => {
-    let inst = date().required().max(new Date(2014, 5, 15));
+    let inst = date().max(new Date(2014, 5, 15));
 
     return Promise.all([
       expect(date().isValid(null)).resolves.toBe(false),
@@ -49,11 +55,27 @@ describe('Date types', () => {
       expect(inst.isValid(new Date(2014, 7, 15))).resolves.toBe(false),
       expect(inst.isValid('5')).resolves.toBe(true),
 
-      expect(inst.validate(undefined)).rejects.toEqual(
+      expect(inst.required().validate(undefined)).rejects.toEqual(
         expect.objectContaining({
           errors: ['this is a required field'],
         }),
       ),
+
+      expect(inst.required().validate(undefined)).rejects.toEqual(
+        TestHelpers.validationErrorWithMessages(
+          expect.stringContaining('required'),
+        ),
+      ),
+      expect(inst.validate(null)).rejects.toEqual(
+        TestHelpers.validationErrorWithMessages(
+          expect.stringContaining('cannot be null'),
+        ),
+      ),
+      expect(inst.validate({})).rejects.toEqual(
+        TestHelpers.validationErrorWithMessages(
+          expect.stringContaining('must be a `date` type'),
+        ),
+      ),
     ]);
   });
 

test/number.ts

@@ -46,7 +46,8 @@ describe('Number types', function () {
     it('should return NaN for failed casts', () => {
       expect(number().cast('asfasf', { assert: false })).toEqual(NaN);
 
-      expect(number().cast(null, { assert: false })).toEqual(NaN);
+      expect(number().cast(new Date(), { assert: false })).toEqual(NaN);
+      expect(number().cast(null, { assert: false })).toEqual(null);
     });
   });
 
@@ -70,7 +71,7 @@ describe('Number types', function () {
   });
 
   it('should VALIDATE correctly', function () {
-    let inst = number().required().min(4);
+    let inst = number().min(4);
 
     return Promise.all([
       expect(number().isValid(null)).resolves.toBe(false),
@@ -83,11 +84,21 @@ describe('Number types', function () {
       expect(inst.isValid(5)).resolves.toBe(true),
       expect(inst.isValid(2)).resolves.toBe(false),
 
-      expect(inst.validate(undefined)).rejects.toEqual(
+      expect(inst.required().validate(undefined)).rejects.toEqual(
         TestHelpers.validationErrorWithMessages(
           expect.stringContaining('required'),
         ),
       ),
+      expect(inst.validate(null)).rejects.toEqual(
+        TestHelpers.validationErrorWithMessages(
+          expect.stringContaining('cannot be null'),
+        ),
+      ),
+      expect(inst.validate({})).rejects.toEqual(
+        TestHelpers.validationErrorWithMessages(
+          expect.stringContaining('must be a `number` type'),
+        ),
+      ),
     ]);
   });
 

test/object.ts

@@ -336,7 +336,7 @@ describe('Object types', () => {
       });
     });
 
-    it('should pass options to children', () => {
+    it('should propagate context', () => {
       const objectWithConditions = object({
         child: string().when('$variable', {
           is: 'foo',
@@ -346,19 +346,16 @@ describe('Object types', () => {
       });
 
       expect(
-        objectWithConditions.getDefault({ context: { variable: 'foo' } }))
-          .toEqual({ child: 'is foo' },
-      );
+        objectWithConditions.getDefault({ context: { variable: 'foo' } }),
+      ).toEqual({ child: 'is foo' });
 
       expect(
-        objectWithConditions.getDefault({ context: { variable: 'somethingElse' } }))
-          .toEqual({ child: 'not foo' },
-      );
+        objectWithConditions.getDefault({
+          context: { variable: 'somethingElse' },
+        }),
+      ).toEqual({ child: 'not foo' });
 
-      expect(
-        objectWithConditions.getDefault())
-        .toEqual({ child: 'not foo' },
-      );
+      expect(objectWithConditions.getDefault()).toEqual({ child: 'not foo' });
     });
 
     it('should respect options when casting to default', () => {
@@ -371,16 +368,18 @@ describe('Object types', () => {
       });
 
       expect(
-        objectWithConditions.cast(undefined, { context: { variable: 'foo' } })
+        objectWithConditions.cast(undefined, { context: { variable: 'foo' } }),
       ).toEqual({ child: 'is foo' });
 
       expect(
-        objectWithConditions.cast(undefined, { context: { variable: 'somethingElse' } })
+        objectWithConditions.cast(undefined, {
+          context: { variable: 'somethingElse' },
+        }),
       ).toEqual({ child: 'not foo' });
 
-      expect(
-        objectWithConditions.cast(undefined)
-      ).toEqual({ child: 'not foo' });
+      expect(objectWithConditions.cast(undefined)).toEqual({
+        child: 'not foo',
+      });
     });
   });