mixed.oneOf doesn't respect nullable

[fnky]

The mixed.oneOf doesn't accept null when nullable is added.

Example

const fruitsOrNone = yup.string().oneOf(['apple', 'banana']).nullable(true)

fruitsOrNone.validate(null) // => throws ValidationError: 'this must be one the following values: apple, banana'
fruitsOrNone.validate('apple') // => apple

Expected

It's expected to accept the value null when nullable is set on the schema.

[jquense]

yes this confusing but expected. you have a contradiction between your schema on one hand you are saying its can only be 'apple' or 'banana' and on the other say it can also be null, the best way to handle this is to add null to your oneOf config

[aphillipo]

I strongly think that nullable() should add the null to the oneOf constraint automatically. Sometimes you are reusing the options passed to oneOf for your dropdowns (for example) but you don't want a null option field added.

I'm currently doing .oneOf([null].concat(RELATIONSHIPS), message) or whatever fields you have in there and it looks mega weird!

[nfantone]

yes this confusing but expected. you have a contradiction between your schema on one hand you are saying its can only be 'apple' or 'banana' and on the other say it can also be null

@jquense I could make the same argument for every schema definition that includes .nullable(). For instance, by using yup.number().nullable() you'd claim that "this is confusing" because "you have a contradiction" by saying that your value "can only be a number" and "on the other side, it says it can also be null" (two completely different types in JS). The point is: yes, that is exactly what I'm saying it should happen.

This boils down to a matter of semantics and expressing intent. I honestly wouldn't find it confusing at all to read yup.string().oneOf(['apple', 'banana']).nullable() (so, "apple", "banana" and null are all fine) - in the same way, I wouldn't think yup.number().nullable() is contradictory or unexpected. On the other hand, reading something like:

yup.string().oneOf([ null, ...MY_VALUES ]);

...is bound to raise some eyebrows.

[jquense]

I realize that yes this can be confusing or a bit odd. Much of the choices in yup straddle that line, because a lot fo this is is subjective and up to what you feel makes sense in terms of expectations. We can of course keep trying to hit the sweet spot fro the most people but somethings are always bound to raise eyebrows for some people. In this particular case I think the current behavior, while somewhat surprising, is the most flexible approach, and consistent with Joi's behavior so there is some precedent.

[ghost]

Btw, yup.number().nullable() does not work for me in case of {field: {label: 'Some', value: null }}:

yup.object().shape({
  // ...
  field: yup.object().shape({
    label: yup.string(),
    value: yup.number().nullable()
  }),
});

Is this also an expected behavior?

[kryten87]

@jquence Reading this thread helped me to understand the choices you made here. Perhaps we could update the docs for mixed.oneOf to clarify this behaviour?

[Philipp91]

Agree with @fnky @aphillipo and @nfantone.

the current behavior, while somewhat surprising, is the most flexible approach, and consistent with Joi's behavior

IIUC, yup.oneOf([noNulls, inHere]).nullable(true|false) currently does not allow null values, no matter what I pass to nullable(..). That is, it's exactly equivalent to yup.oneOf([noNulls, inHere]). And the same is true if null is contained in the list of allowed values, then one wouldn't need nullable() on top anymore.

So firstly, I don't see how the current behavior is the most flexible approach. Unless I'm overlooking something, it makes nullable useless when combined with oneOf.

And secondly, it seems like changing the behavior won't hurt anyone. The way the current behavior (and apparently also Joi's behavior?) is, nobody would be combining these two. So these users (and consistently with Joi) can't be negatively affected by changing the combined behavior.

[juni0r]

I agree with @nfantone and @Philipp91. The current semantics are confusing and I don't think that consistency with Joi's behavior is a strong argument. If Joi's got it wrong, why not fix it?

[micktdj]

You can implement your own method with Yup.addMethod to handle this case :

Yup.addMethod(Yup.string, 'oneOfOptional', function(arr, message) {
  return Yup.mixed().test({
    message,
    name: 'oneOfOptional',
    exclusive: true,
    params: {},
    test(value) {
      return value == null ? true : arr.includes(value);
    },
  });
});

export default Yup.string().oneOfOptional;

Then, you can use it like this :

import * as Yup from 'yup';
import oneOfOptional from './oneOfOptional';

export default Yup.object().shape({
  value: oneOfOptional(['toto','foo','bar'])
});

Finally, it works :

schema.isValid("toto") // true
schema.isValid(null) // true
schema.isValid(undefined) // true
schema.isValid('42') // false

[zhouzi]

It seems that adding null to the list of elements is not enough, it must also be nullable. Here's the final working solution:

const fruits = ["banana", "apple"];
const schema = string().oneOf([...fruits, null]).nullable();

It can be tested here: https://runkit.com/zhouzi/5f2d264f20ab43001a495276

[catamphetamine]

Ha ha, imagine how many dev hours you've wasted just by being stubborn and not handling this case automatically inside the library code.

... DEV DAYS WASTED BECAUSE OF ONE OF
NOT BEING ABLE TO HANDLE NULLS PROPERLY

Next person in this issue can increment the counter.

[dartess]

Due to this non-obvious behavior, I cannot make a reusable description of the field. In a perfect world, I could do this:

function consensus() {
    return string()
        .oneOf(consensuses, consensusError);
}

And use this with native means as field: consensus() or field: consensus().nullable().

In the current implementation, I need to do crutches with passing parameters to my description.

@jquense I think this issue should be open for further discussion.

[filkalny-thimble]

Here is the typing for @micktdj's oneOfOptional

import type {
    MixedLocale, Ref
} from "yup";

declare module "yup" {
    interface StringSchema<T, C> {
        oneOfOptional<U extends T>(
            arrayOfValues: ReadonlyArray<U | Ref>,
            message?: MixedLocale["oneOf"],
        ): StringSchema<U | null | undefined, C>;
    }
}

[micktdj]

Here is the typing for @micktdj's oneOfOptional

import type {
    MixedLocale, Ref
} from "yup";

declare module "yup" {
    interface StringSchema<T, C> {
        oneOfOptional<U extends T>(
            arrayOfValues: ReadonlyArray<U | Ref>,
            message?: MixedLocale["oneOf"],
        ): StringSchema<U | null | undefined, C>;
    }
}

Thank you filkalny-thimble

[OoDeLally]

I would add that yup claims to be composable thanks to every method returning a new yup. Because of this questionable decision, this good principle is broken.

// Some reusable yup
export const yesNoAnyYup = yup.mixed<YesNoAny>().oneOf(Object.values(YesNoAny));

// at usage time
yesNoAnyYup.nullable()
// Here there is nothing I can do unless I completely override the initial `oneOf()`, 
// which break the LoD. 

Pleeease do something about it 🙏

[kn327]

In a standard form, there is no way for a user to enter null into a text field, drop down list, or any other form of list without writing custom onChange behavior to clear this out. Even through formik, the field will be set to an empty string once the user enters and exits the form or if all input is deleted

My expected behavior when I add the .nullable() constraint to any schema is for the validation to short circuit and pass if the value is null, "", or undefined (a bit hesitant on this one since technically speaking undefined could also mean a schema mismatch to the original data).

This would make the behavior consistent to the .required() constraint which appears to compensate for empty strings and nulls appropriately...

In my opinion, this is certainly not a conflict of intention here, but an expectation of consistency in implementation between the different validation constraints I can place on a particular object.

My particular implementation uses enums to determine the possible values and can be implemented as below

yup.addMethod(yup.string, "enum", function(enumType, message) {
  const keys = Object.keys(enumType);
  const set = new Set(keys);
  return this.test("enum", message, function (str) {
      if (str && !set.has(str)) {
        if (!message) {
          message = `${this.path} must be one of ${keys.join(", ")}`;
        }
        return this.createError({
          path: this.path,
          message: message
        });
      }
      return true;
  });
});

[mile-mijatovic]

Here is how I did it

const validationSchema = Yup.object().shape({
  image: Yup.mixed().test("fileSize", "The image is too large", (value) => {
    if (!value.length) return true; // <-- did the trick
    return value[0].size <= 3000000;
  }),
});

[EvHaus]

...the best way to handle this is to add null to your oneOf config

Note that if you do this, you must put .nullable() before .oneOf() otherwise TypeScript will fail with:

error TS2322: Type 'null' is not assignable to type 'string | Reference<unknown> | undefined'.

162    ['a', 'b', null],

// This won't work
const schema = yup.string().oneOf(["a", "b", null]).nullable();

// This works
const schema = yup.string().nullable().oneOf(["a", "b", null]);