sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
4.85k stars 155 forks source link

TypeCompiler.Compile incorrectly parsing `Record<any, any>` #916

Closed Lonli-Lokli closed 2 months ago

Lonli-Lokli commented 3 months ago

Incorrectly parsing Record<any, any>, while works with Record<number, string>

 const schema = TypeCompiler.Compile(
    Type.Object({
      prop: Type.Optional(Type.Record(Type.Any(), Type.Any())),
    })
  );

 JSON.stringify([
    ...schema.Errors({ prop: { a: 1, b: 1 } }),
  ]);

// Output 
// [{"type":33,"schema":{"not":{}},"path":"/prop","value":{"a":1,"b":1},"message":"Never"}]

https://stackblitz.com/edit/vitejs-vite-1bhkml?file=counter.js&terminal=dev

sinclairzx81 commented 2 months ago

@Lonli-Lokli Hi, sorry for the delay in reply (Have been very busy of late)

TypeBox only supports a small subset of types for Record keys. These are String, Number, Boolean, TemplateLiteral and Union of Literal types. Rather than using Type.Any(), would recommend using Type.String() as it represents all permissible JSON keys (and is generally more type safe)

Will close of this issue for now, but if you have any questions, feel free to ping on this thread. Cheers S

Lonli-Lokli commented 2 months ago

@sinclairzx81 First of all, it's not mentioned anywhere in the docs, so I wasn't ready for a production runtime error.

Second, even if it's not supported why did you close this issue? Not supported !== Closed

sinclairzx81 commented 2 months ago

@Lonli-Lokli Heya,

Second, even if it's not supported why did you close this issue? Not supported !== Closed

Should TypeBox support Any for keys? It would presumably also need to support Unknown and Never keys. It's feasible for TypeBox to implement a mapping for this, however in the Any and the Unknown case, the patternProperties schema would need to resolve to String (meaning Any and Unknown would just be aliases for String).

I am open to considering this, however it would be good to get some insight into why you need Any for a Record key when String is both stricter and more correct (when considering Json validation where property keys are all string)

Keep me posted.

Lonli-Lokli commented 2 months ago

@sinclairzx81 Yeah, so I have cases in business Logic when keys must be a number. either direct or via enum. So for me it's easier to write parser in any

sinclairzx81 commented 2 months ago

@Lonli-Lokli Hiya,

Have published an update to resolve this on 0.32.35. Record keys can now be Any or Never. If an invalid key type is passed to Record, the return type will be TNever (indicating no valid Record type can be constructed). Note that Any is just an alias for String, and Never produces a regular expression for patternProperties that will always fail (inline with TS semantics)

Hope this helps S

Lonli-Lokli commented 2 months ago

@sinclairzx81 Thanks!

Is there any way to make compilation error in this scenario? Eg in the snippet from first post, will it be possible to have an error from TypeCompiler.Compile that there is a never type in chain?

sinclairzx81 commented 2 months ago

@Lonli-Lokli Heya,

Is there any way to make compilation error in this scenario? Eg in the snippet from first post, will it be possible to have an error from TypeCompiler.Compile that there is a never type in chain?

Never types are analogous to the TypeScript never type and are considered standard (so should compile the same as they do in TypeScript). In TypeBox, the TNever type is used to hint that a user type composition produced an illogical type that will always fail to check a value. They are partially used to signal composition error but are not actually errors. Generally, you should always check the return type of a composition to ensure it generated the expected type.

Pretty keen to keep the Never type as is; as it is a critical component of the type system.