Repository: jquense/yup
Pull request #1982 (2023-04-14 13:30:47)

Potentially breaking: Make null validation errors consistent across schema
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 `null`s. 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).

jquense: fixes @jarnaiz issue in #1952

Patch:
diff --git a/README.md b/README.md
index 518b8e8..86894f2 100644
--- a/README.md
+++ b/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
 
diff --git a/src/date.ts b/src/date.ts
index 1155869..ca1e367 100644
--- a/src/date.ts
+++ b/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);
 
diff --git a/src/number.ts b/src/number.ts
index 3662b0c..818ffac 100644
--- a/src/number.ts
+++ b/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);
       });
diff --git a/src/object.ts b/src/object.ts
index 4b534e7..5b307b3 100644
--- a/src/object.ts
+++ b/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;
diff --git a/src/schema.ts b/src/schema.ts
index 322edcb..6bfa84b 100644
--- a/src/schema.ts
+++ b/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 patch:
diff --git a/test/date.ts b/test/date.ts
index 603b949..d81d4f3 100644
--- a/test/date.ts
+++ b/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'),
+        ),
+      ),
     ]);
   });
 
diff --git a/test/number.ts b/test/number.ts
index edfdd02..2ea8031 100644
--- a/test/number.ts
+++ b/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'),
+        ),
+      ),
     ]);
   });
 
diff --git a/test/object.ts b/test/object.ts
index 8b859f8..d18d118 100644
--- a/test/object.ts
+++ b/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',
+      });
     });
   });