ljharb / prop-types-tools

Custom React PropType validators
MIT License
671 stars 50 forks source link

Or Tests seem to be wrong #37

Closed merlinstardust closed 4 years ago

merlinstardust commented 6 years ago

The or tests seem to repeat the double required case.

    it('works when outer and inner are optional', () => {
      assertPasses(or([bool.isRequired, explicitNull()]), (<div a />), 'a');
      ...
    });

    it('works when outer is optional, inner is required', () => {
      assertPasses(or([bool.isRequired, explicitNull().isRequired]), (<div a />), 'a');
      ...
    });

    it('works when outer is required, inner is optional', () => {
      assertPasses(or([bool.isRequired, explicitNull()]).isRequired, (<div a />), 'a');
      ...
    });

    it('works when outer is required, inner is required', () => {
      assertPasses(or([bool.isRequired, explicitNull().isRequired]).isRequired, (<div a />), 'a');
      ...
    });

The above should instead be the following

    it('works when outer and inner are optional', () => {
      assertPasses(or([bool, explicitNull()]), (<div a />), 'a');
      ...
    });

    it('works when outer is optional, inner is required', () => {
      assertPasses(or([bool, explicitNull().isRequired]), (<div a />), 'a');
      ...
    });

    it('works when outer is required, inner is optional', () => {
      assertPasses(or([bool.isRequired, explicitNull()]), (<div a />), 'a');
      ...
    });

    it('works when outer is required, inner is required', () => {
      assertPasses(or([bool.isRequired, explicitNull().isRequired]).isRequired, (<div a />), 'a');
      ...
    });
ljharb commented 6 years ago

This is intentional because of how explicitNull works; if bool is optional, then undefined will always be allowed, and explicitNull will never trigger.

We should probably add additional tests that flip the order tho (if they aren’t already there), and those would allow for bool.