sindresorhus / ow

Function argument validation for humans
https://sindresorhus.com/ow/
MIT License
3.8k stars 105 forks source link

Unclear error message when checking NaN against the number type #231

Open ghost opened 2 years ago

ghost commented 2 years ago

When testing NaN against ow.number, the following error message is emitted:

ArgumentError: Expected argument to be of type `number` but received type `number`
at ow (node_modules/ow/dist/index.js:29:28) 
{
  validationErrors: Map(1) {
    'number' => Set(1) {
      'Expected argument to be of type `number` but received type `number`'
    }
  }
}

This behaviour can be verified by running this two line snippet in the Node REPL:

const { default: ow } = require('ow')
ow(NaN, ow.number)

I think that this error message would be quite confusing for those who encounter it. I see two possible ways to solve this problem:

sindresorhus commented 2 years ago

I agree. That should be improved. I think we should do both suggestions.

panva commented 2 years ago

@sindresorhus would you prefer the first suggestion to be a breaking change in @sindresorhus/is or a special case added to ow?

sindresorhus commented 2 years ago

would you prefer the first suggestion to be a breaking change in @sindresorhus/is or a special case added to ow?

How would you fix it in @sindresorhus/is?

panva commented 2 years ago

@sindresorhus change https://github.com/sindresorhus/is/blob/v4.4.0/source/index.ts#L112-L113

to

        case 'number':
            return Number.isNaN(value) ? 'NaN' : 'number';
sindresorhus commented 2 years ago

Ah yes. Let's fix it in @sindresorhus/is, but it might take a while before we can do a major version there, so better to fix it in both places, actually.

panva commented 2 years ago

Ok, ignore #232 then.

sindresorhus commented 2 years ago

See comment update.