Typia Omits NaN Checks (By Default)

[sinclairzx81]

Summary

Hey, have noticed Typia doesn't appear to check for NaN values. Note that NaN (i.e. Not-A-Number) is a unrepresentable / non-computable IEEE754 numeric that can only result in another NaN. JavaScript supports NaN as a consequence of IEEE754; and TypeScript interprets it as a number static type, but for validation purposes, not validating for it is a very common source of errors.

const N = parseFloat('foo')        // NaN
const R = typia.is<number>(N)      // true

Comparison

Assertion for NaN is generally expected for number validation, the following are a list of libraries I've tested so far.

// ----------------------------------------------------------------------
// Typia
// ----------------------------------------------------------------------

import { is, equals } from 'typia'
console.log('typia:is', is<number>(NaN))                         // true
console.log('typia:equals', equals<number>(NaN))                 // true

// ----------------------------------------------------------------------
// Zod
// ----------------------------------------------------------------------

import zod from 'zod'
console.log('zod', zod.number().safeParse(NaN).success)          // false

// ----------------------------------------------------------------------
// Ajv
// ----------------------------------------------------------------------

import Ajv from 'ajv'
console.log('ajv', (new Ajv()).validate({ type: 'number'}, NaN)) // false

// ----------------------------------------------------------------------
// Superstruct
// ----------------------------------------------------------------------

import superstruct from 'superstruct'
console.log('superstruct', superstruct.number().is(NaN))         // false

// ----------------------------------------------------------------------
// Yup
// ----------------------------------------------------------------------

import * as yup from 'yup'
console.log('yup', yup.number().isValidSync(NaN))                // false

// ----------------------------------------------------------------------
// Joi
// ----------------------------------------------------------------------

import * as joi from 'joi'
console.log('joi', joi.number().validate(NaN))                  // result: [Error [ValidationError]: "value" must be a number]

// ----------------------------------------------------------------------
// Class Validator
// ----------------------------------------------------------------------

import { validate, IsNumber } from 'class-validator';
class Post {
  @IsNumber()
  value: number = NaN
}
validate(new Post()).then(console.log)                          // throws: 'value must be a number conforming to the specified constraints'

Performance

Note, You are almost certainly going to see a performance degradation as a result of implementing this check (which largely accounts for the excessive performance disparity as shown in your projects readme, specifically ObjectSimple). However not implementing this check does put Typia at odds with other validators (and probably at odds with the expectations of users).

A possible solution may be to make the NaN check configurable (as TypeBox has done to get visibility on this disparity) which will allow Typia to keep on with the 15,000x performance claim, but I do think Typia should probably assert for NaN by default (with a possible --allow-nan command line option to allow Typia to opt out of NaN checks when benchmarking).

Note: JSON.parse('NaN') will result in a deserialization error, so a case can be make for opting out of NaN checks in scenarios where values originate from JSON parsing. In this respect, providing an option to opt out may be reasonable.

Micro Benchmark

The following shows the comparative performance with and without the NaN check. When factoring the Point3D check for ObjectSimple, that almost certainly accounts for the 12x performance for this object.

const N = 10000

{ // With NaN check
  const S = Date.now()
  for(let i = 0; i < 1_000_000_000; i++) {
    const _ = typeof N === 'number' && !isNaN(N)
  }
  console.log(Date.now() - S) // 708ms
}

{ // Without NaN check
  const S = Date.now()
  for(let i = 0; i < 1_000_000_000; i++) {
    const _ = typeof N === 'number'
  }
  console.log(Date.now() - S) // 352ms
}

System

Node: 16.17.1
Typescript: 4.9.5
Typia: 3.5.4

References

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/NaN
https://en.wikipedia.org/wiki/IEEE_754

For consideration

[samchon]

Special functions like assertStringify(), assertEncode() and assertClone() checks Number.isFinite().

As you know, typia is designed to validate only TypeScript type. In the TypeScript type system, Number.INF and NaN are not invalidating number type. It would be invalidate type only when converting to JSON string or Protocol Buffer data. For an example, the representative use case of assert() is @nestia.core.TypedBody() function. It validates request body data after JSON.parse() function, and it doesn't need to validate Number.isFinite() after JSON.parse().

• https://github.com/samchon/nestia/wiki/Core-Library#decorators

If you want to turn on the Number.isFinite() checking manually, configure finite property as true in plugin configuration of tsconfig.json file. Otherwise, use special functions like assertStringify() (assert() + stringify()) or assertEncode() (assert() + encode() for Protobol Buffer). As you've said, the Number.isFinite() (not Number.isNaN()) must be checked when converting to JSON or Protobuf data and, typia does it.

typia/test/tsconfig.json

Lines 106 to 112 in b2fa3fe

	"plugins": [
	{
	"transform": "../src/transform.ts",
	"functional": true,
	"numeric": true,
	"finite": true,
	},

[sinclairzx81]

@samchon Well, fair enough, however NaN does literally mean "Not A Number" and I do think the general expectation here would be that Typia would flag it as a default rather than requiring users to opt into it via finite (which you might want to make a bit more obvious in the docs). But it's good to see it's at least configurable!

Will close off this issue as it sounds default NaN checks are not likely as a default, and opt in through configuration is quite workable.
Cheers

[samchon]

As above statement is not a compile error in Typescript (of course, it is nonsensible), I have to keep current policy. Also, as I've invented this typia to support nestia and reactia, I must consider the use that validating after JSON.parse().

Unfortunately, typia became a little bit famous earlier than nestia, but nestia is the ultimate goal of this typia. Also, I've completed to implement reactia, but writing documents and creating example projects are left, and it is hard thing for me to accomplish in leisure time.

Therefore, I decided to not use isNaN() function as default, but you can turn on the isNaN() checking on your benchmark program, just by configuring numeric: true, finite: false config. Thanks for advise.

[sinclairzx81]

@samchon It's fine. I think aligning strictly to the policies of TypeScript is quite reasonable with the configuration overrides, so all good there. But please document the numeric and finite configs in the readme! (I couldn't find them)

While I do think most people would probably recommend NaN assertions should be enabled by default (and probably Infinity too), I can easily see a reason for not implementing them if the goal is to be completely aligned to TypeScript. I've also made similar concessions in TB aligning it to JSON Schema, so I do understand.

Just on the topic of configurations / performance though, you can actually use the following to have TypeBox adopt TypeScript checking semantics as well. This should let you directly measure AOT and JIT performance for equivalent assertion logic. Both of these are false by default.

import { TypeSystem } from '@sinclair/typebox/system'
TypeSystem.AllowArrayObjects = true
TypeSystem.AllowNaN = true

Would be curious to see the performance cost of NaN run against varying spoiler data that you have implemented on the Typia benchmarks. (JIT and AOT performance is somewhat of interest to me atm given the variability I've seen between Node versions). I may take a look at running these later this week (given how these benchmarks currently look)

Anyway, Good work! Typia's looking good (also, glad you went with name change!)