Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Lonli-Lokli opened this issue Jun 25, 2024 · 7 comments · Fixed by #932
Closed

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

Lonli-Lokli opened this issue Jun 25, 2024 · 7 comments · Fixed by #932

Comments

@Lonli-Lokli
Copy link

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
Copy link
Owner

@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
Copy link
Author

@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
Copy link
Owner

@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
Copy link
Author

@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
Copy link
Owner

@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
Copy link
Author

@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
Copy link
Owner

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants