typia icon indicating copy to clipboard operation
typia copied to clipboard

TypeScript: Property of T | undefined Check Mismatch

Open sinclairzx81 opened this issue 2 years ago • 10 comments
trafficstars

Summary

TypeScript flags the following code with an error

TypeScript Link Here

type T = { x: number | undefined }

function test(value: T) {}

test({}) 
//    ^
//    Property 'x' is missing in type '{}' but required in type 'T'

However the is and equals checks return true for the same type.

import typia from 'typia'

type T = { x: number | undefined }

const X = typia.is<T>({})
const Y = typia.equals<T>({})

console.log(X) // true | expect false
console.log(Y) // true | expect false

Notes

The intent here is to validate for the policy of T1 however Typia appears to be using the policy of T2.

type T1 = { x: number | undefined } // x property key is REQUIRED
                                    // x property value can be (number | undefined)

type T2 = { x?: number }            // x property key is OPTIONAL
                                    // x property value can be (number | undefined)

Use Case

This was previously discussed on https://github.com/samchon/typia/pull/256#issuecomment-1287826222, however may be worth revisiting for the following usage.

type T = { x: number | undefined } // expect 'x' key to be required property

const V = {}                       // an object without 'x' property

const R = typia.is<T>(V)           // reports true, suggesting 'x' exists

const K = Object.keys(V)           // error: expected ['x'] got []

System

Node: 16.17.1 TypeScript: 4.9.5 Typia: 3.5.7

Config

{
  "transform": "typia/lib/transform",
  "functional": true,  
  "numeric": true,  
  "finite": true, 
}

sinclairzx81 avatar Feb 21 '23 21:02 sinclairzx81

How to distinguish it?

samchon avatar Feb 22 '23 03:02 samchon

Compiler

The following will check for the optional modifier ? on ts.PropertySignature compiler side.

function isOptionalProperty(node: ts.PropertySignature) {
  return node.questionToken !== undefined
}

Assertion

The runtime assertion would require checking for the existence of the key x ONLY if:

  • x is Required
  • x extends undefined

To do the assertion, you will want to ensure it's only done when necessary, this is how I'd probably approach it.

During Emit:

  • For Each Property:
    • if Required:
      • typeof value.x === 'number' || (value.x === undefined && 'x' in value)
    • if Optional:
      • typeof value.x === 'number' || (value.x === undefined)

Optimization

Ensure T assertion occurs BEFORE undefined assertion in T | undefined. This to short circuit the expression and only run the slower 'x' in value check when the value is undefined.

sinclairzx81 avatar Feb 22 '23 06:02 sinclairzx81

Ah, I got it by your hint. It is possible to distinguish through obj.hasOwnProperty(key) function.

It would better to support the explicit undefined type through configuration.

This undefined configuration seems the proper case. Do you agree? Then I will develop in this weekend.

https://github.com/samchon/typia/blob/e5d4d7af0f37ac038496dc36cddf09be13489ecb/src/transformers/ITransformOptions.ts#L48-L61

samchon avatar Feb 22 '23 10:02 samchon

Ah, I got it by your hint. It is possible to distinguish through obj.hasOwnProperty(key) function.

Sure, just use which ever has the fastest runtime performance.

  • Object.keys(value).includes(key) - good for key caching with other logic.
  • value.hasOwnProperty(key) - good for inlining
  • key in value - this may also be good (needs benchmarking)

It would better to support the explicit undefined type through configuration.

I would have expected this check to be the default as I feel it's better capturing the expected semantics of TypeScript required and optional property key checks (as seen here), But if implementing as a config, would it also be possible to provide a strict option that just switches everything on?

{
  "transform": "typia/lib/transform",
  "strict": true, // function + numeric + finite + nan + required-undefined
}

Or maybe reading strict: true from tsconfig.json ?


sinclairzx81 avatar Feb 22 '23 12:02 sinclairzx81

Do you have any idea about expected type name of error instaces? (TypeGuardError and IValidation.errors)

samchon avatar Feb 24 '23 10:02 samchon

@samchon I do, but error handling is perhaps a topic for another time. I could recommend setting up a TypeGuardErrorCode enumeration similar to this, and update TypeGuardError to accept the code on it's constructor. This code can be used to differentiate between different classes of error (i.e. missing property keys), as well as make provisions for internationalization (i18n) support in future.

export enum TypeGuardErrorCode {
   ObjectRequiredProperties,    // use this code if missing property (this error)
   ObjectAdditionalProperties,  // use this code if additional properties
   Boolean,                     // use this code if not a boolean
   String,                      // use this code if not a string
   Number,                      // use this code if not a number
   Undefined,                   // use this code if not a undefined
   Null,                        // use this code if not null
   // ... more
   Unknown                      // use this code on unknown error
}

export class TypeGuardError extends Error {
  constructor(
    public readonly code: TypeGuardErrorCode, // error code detected during assertion
    public readonly path: string,             // RFC 6901 JSON Pointer
    public readonly value: unknown,           // actual: value
    public readonly expect: unknown,          // expect: type
    message: string                           // optional message (code more important!)
  ) {
    super(message)
  }
}

Just so long as typia.is<{ x: undefined }>({}) is returning false, you should be able to throw the following on assert at a later time.

throw new TypeGuardError(TypeGuardErrorCode.ObjectRequiredProperty, '/x', undefined, 'undefined', 'Expect required property')

sinclairzx81 avatar Feb 24 '23 14:02 sinclairzx81

If change the TypeGuardError, when it would be a break change causing major version update to v4.0.

I should delay this issue for a while and consider how to solve it.

samchon avatar Feb 24 '23 15:02 samchon

This has been implemented recently in TypeBox. To implement this correctly, you will need to check if the property value extends undefined. TypeBox implements a fast path extends check specifically for this case (but it should be possible perform a similar check using the Compiler API by checking for unions of undefined).

sinclairzx81 avatar Apr 06 '23 19:04 sinclairzx81

@sinclairzx81 Those links works fine now, but the branch is a moving target and the links are almost guaranteed to be broken when someone reads this in the future. Could you update to specific commit references instead?

hlovdal avatar Sep 28 '23 21:09 hlovdal

@hlovdal

Could you update to specific commit references instead?

Updated

sinclairzx81 avatar Sep 28 '23 21:09 sinclairzx81