sinclairzx81
TypeCheck.Decode return type inferred incorrectly?
Let's say I have this schema and TypeCheck defined:
const MySchema = Type.Object({
foo: Type.String(),
});
const typeCheck = TypeCompiler.Compile(MySchema);
The type of typeCheck is as expected: TypeCheck<TObject<{foo: Type.String()}>>.
If I let Typescript freely infer the return type of typeCheck.Decode, it's also correct:
const a = typeCheck.Decode(unknownValue); // a: { foo: string }
However, if Typescript has any other hints for the return type, those hints will override the default without giving any error or warning. For example:
const a: { foo: number } = typeCheck.Decode(unknownValue); // a: { foo: number } -- no error
This can lead to interesting problems where a developer can accidentally mix different types. For example:
function validate(unknownValue: unknown): { foo: number } { // expect typescript to throw a build error if I try to return anything that's not { foo: number }
if(randomInt(2) === 1) {
const decoded = typeCheck.Decode(unknownValue); // this is inferred to be { foo: string }
return decoded; // so typescript will refuse to compile this (Types of property 'foo' are incompatible ... ...)
}
else { // but if I don't use an intermediate variable...
return typeCheck.Decode(unknownValue); // it compiles fine! (it shouldn't?)
}
}
Is this behavior expected? If I go to compiler.d.mts and patch the type manually like so, it solves my problem:
// before
Decode<R = StaticDecode<T>>(value: unknown): R;
// after
Decode<R extends StaticDecode<T> = StaticDecode<T>>(value: unknown): R;
But I'm not sure if it would have any unintended side effects.
@paulius-valiunas Hi,
This is an interesting issue. You would expect TypeScript to be able to correctly determine that the type isn't valid for the left-side assignment, but it looks like it hasn't evaluated the type in a way it is able to do that. This might be a TS bug, so might be worth chasing up on to get a bit of insight into the nature of the behavior (it should be possible to repro this without TypeBox)
Ill do some digging before actioning anything here. I think your suggested update would be workable, with perhaps the signatures updated to the following to mitigate duplicate inference on StaticDecode.
public Decode<Static extends StaticDecode<T>, Result extends Static = Static>(value: unknown): Result {...}
public Encode<Static extends StaticEncode<T>, Result extends Static = Static>(value: unknown): Result {...}
This would need to be applied to the Value Encode and Decode functions also and tested to ensure this doesn't result in instantiation issues. Will drop a review tag on this and take a look when I next get some time.
Thanks for reporting! This is a good catch. S
I think it makes sense from Typescript side. If there's no default generic argument, it will try to infer the type from surrounding code and throw if it fails. The default argument = StaticDecode<T> only replaces the throw, but doesn't change the way the type is inferred otherwise. I guess it might be different depending on the version of Typescript, in this case I tried 5.2.2 and 5.5.4.
See my last comment here: https://github.com/sinclairzx81/typebox/issues/815
R extends StaticDecode<T> = StaticDecode<T> will still allow R to be any arbitrary subtype of the actual type, so can include fields that are not actually present or validated
type SubType = {
foo: string
bar: string
}
const a: SubType = typeCheck.Decode({ foo: 'oof' }); // a: { foo: string; bar: string } -- no error
// Uncaught TypeError: Cannot read properties of undefined (reading 'toUpperCase')
console.log(a.bar.toUpperCase())
I don't think that this can be solved. TypeScript generics are covariant by design. I.e. constraining generics is always an <T extends SomeType>, because it's perfectly valid to do e.g.
const example = (arg: { foo: string }) => console.log(arg.foo)
const value = { foo: 'oof', bar: 'rab' }
example(value)
I get Value.Decode is not a function error. Is Decode Deprecated? I don't see anything in the docs.
"@sinclair/typebox": "^0.33.7"
@noelsoong Try
import { Value } from '@sinclair/typebox/value'
const result = Value.Decode(Type.String(), 'hello')
@ehaynes99 Hi, thanks for helping to take a look at this!
So here was the implementation I was looking at. I do think the expectation would be that decode should probably constrain to the expected type. I note: when only setting the default type for T, I TS will attempt to infer T through usage (in which case T assumes the type of the function return type in decode1).
// --- Current (default T is expected type)
declare function decode1<T = { x: string }>(): T
function test1(): { x: number } {
return decode1() // no error (but property x string is not assignable to number)
}
// --- Proposed (constrain T to expected type)
declare function decode2<T extends { x: string } = { x: string }>(): T
function test2(): { x: number } {
return decode2() // error - (this seems correct)
}
This is perhaps better illustrated below where left side annotations infer the generic argument.
declare function decode1<T = { x: string }>(): T
// note: generic argument T inferred via annotation
const value: { x: number } = decode1() // function decode1<{
// x: number; // x: number?
// }>(): {
// x: number; // x: number?
// }
So it's interesting. I am keen to get your thoughts on the above specifically. I tend to lean in favor of constraining the type to prevent the generic argument being unintentionally overridden though default inference (or explicitly via annotation, which ideally shouldn't be permitted). I'm not sure this works against covariance for this specific scenario, but open to getting some additional thoughts on the above.
The enforcement would normally happen in the implementation of decode
function decode2<T extends First = First>(): T {
// typescript: Type '{ x: string; }' is not assignable to type 'T'.
// '{ x: string; }' is assignable to the constraint of type 'T', but 'T'
// could be instantiated with a different subtype of constraint 'First'. [2322]
return { x: 'foo' }
}
But as I think about it more, why make the return type here generic at all? In my other linked issue, you mentioned that TypeBox uses that technique internally (your link doesn't point to the right line in the source anymore, so not sure which function you were referring to). That solves the issue I filed about not getting type checking inside of my function, but I ended up not going that direction because the opposite problem was IMO far worse: users could inadvertently mess up the types. I felt safer constraining the lack of type safety to my library code that was well tested. The very nature of Decode seems like you're inevitably going to have an "unsafe" cast in there somewhere, but it's not actually unsafe because you're doing runtime validation on the value.
In short, this exposes a type-safe contract with end users:
import { type StaticDecode, type TSchema, Type } from '@sinclair/typebox'
type First = { x: string }
type Second = { x: string; y: string }
const First = Type.Object({
x: Type.String(),
})
declare function Decode<T extends TSchema>(schema: T, value: unknown): StaticDecode<T>
// typescript: Property 'y' is missing in type '{ x: string; }' but required in type 'Second'. [2741]
const decoded: Second = Decode(First, {})
// typescript: Type '{ x: string; }' is not assignable to type 'string'. [2322]
const decoded2: string = Decode(First, {})
@ehaynes99 Hey, thanks for the follow up.
In my other linked issue, you mentioned that TypeBox uses that technique internally (your link doesn't point to the right line in the source anymore, so not sure which function you were referring to).
Had a quick look through previous issues and found this https://github.com/sinclairzx81/typebox/issues/680#issuecomment-1833051204. The linked code would have been referencing the Value.Decode signatures which are similar to TypeCheck.Decode (0.33.7)
But as I think about it more, why make the return type here generic at all?
The main reason I made the signatures generic was to work around a specific TypeScript issue related to "deep and possibly infinite instantiation" that occurred when using StaticDecode<T> as a direct return type. I can't remember the exact implementations that triggered the issue, but I do recall encountering problems after the introduction of instantiation expressions in TypeScript 4.7.
At the time, I theorized that TypeScript might be using a different evaluation strategy for computed generics versus computed return types. Specifically, it seemed that the computed return type would exceed instantiation limits when using decoded types as return types in other functions (granted enough complexity), but shifting the evaluation to a computed generic helped avoid this problem. The idea was that by computing the static type and storing it in a generic parameter, TypeScript would retain the precomputed type, which reduced the work needed when instantiating the type at call sites. The following illustrates this high-level thinking.
import { TSchema, StaticDecode, TObject, TNumber, Ensure } from '@sinclair/typebox'
// Computed Return Type (Lazy)
declare function DecodeLazy<T extends TSchema>(schema: T): StaticDecode<T>
// defer return type until call site:
// - store T
// - compute and return T at call site (issue here)
// Computed Generic (Eager)
declare function DecodeEager<T extends TSchema, R = StaticDecode<T>>(schema: T): R
// defer return type until call site:
// - precompute R with respect to T
// - store R
// - return R at call site
// Parameterized Instantiation Expression: Lazy vs Eager
type Lazy = typeof DecodeLazy<TObject<{ x: TNumber, y: TNumber }>> // <T extends TSchema>(...) => StaticDecode<T>
type Eager = typeof DecodeEager<TObject<{ x: TNumber, y: TNumber }>> // () => { x: number, y: number }
type R1 = ReturnType<Lazy> // { x: number, y: number } - requires deferred call to StaticDecode<T>
type R2 = ReturnType<Eager> // { x: number, y: number }
So, the above is the background rationale for why the return type is computed and stored in a generic argument. Unfortunately it's very difficult show an implementation that demonstrates where one breaks and the other doesn't, nor assert that the compiler is still evaluating instantiation expressions the same as it appeared to in 4.7, but do think the reasoning is moderately sound.
For now, I'm fairly keen to keep the computed generic parameter as is. I am actually working on a version of TypeBox in the background which is making heavy use of computed generics to optimize inference for all computed types (including those resolved through Static)
With this said, I still leaning towards constraining the generic type to the computed static type. I don't expect it would cause many issues doing that. but may catch correctness issues in implementations that explicitly annotate types but where those annotations may be overriding the generic type.
@noelsoong Try
import { Value } from '@sinclair/typebox/value' const result = Value.Decode(Type.String(), 'hello')
I found out that i had to delete my pnpm-lock first and node_modules . Stupid me :3
So, to summarize the discussion:
<R extends StaticDecode<T>>does not solve the problem because it still allows assigning to a type with extra properties- Removing the generic altogether and returning
StaticDecode<T>directly will cause compilation performance issues (and possibly errors?) - @sinclairzx81 is "still leaning towards constraining the generic type to the computed static type"
Is there a potential solution to this? From purely declarative perspective, removing the generic would be the obvious solution. But we might be facing a technical limitation of the Typescript compiler here, in which case we need to find a way around it. If we can't remove the generic, maybe we can move it to the class definition?
export declare class TypeCheck<T extends TSchema, TDecode = StaticDecode<T>, TEncode = StaticEncode<T>> {
/* ... */
Decode(value: unknown): TDecode;
Encode(value: unknown): TEncode;
}
If I understand the problem correctly, that should mitigate the issue mentioned in #680. While it still doesn't offer absolute type safety, it would reduce the risk of breaking it - someone would have to specifically override the generic defaults and it's not something that can be an unintended side effect of an assignment or return operator.
@paulius-valiunas Hi! Thanks for the summary (appreciate there's a bit nuance to this)
Is there a potential solution to this? From purely declarative perspective, removing the generic would be the obvious solution. But we might be facing a technical limitation of the Typescript compiler here, in which case we need to find a way around it. If we can't remove the generic, maybe we can move it to the class definition?
Yes I think there is a solution to this .... basically your original suggestion :) Have gone ahead and published this on 0.33.8 and will trial this for the next few revisions. The Decode and Encode signatures are now as follows.
public Decode<Static extends StaticDecode<T>, Result extends Static = Static>(value: unknown): Result {...}
public Encode<Static extends StaticEncode<T>, Result extends Static = Static>(value: unknown): Result {...}
Revision 0.33.8
The update should make it impossible to override known types derived from Decode and Encode. However the updates still allows left side annotations and generics to extend inferred structures (as per C and D). This should prevent unintended overriding of the inferred type, while still leaving room to structurally extend the inferred type.
const T = Type.Object({ x: Type.Number() })
// --------------------------------------
const A: {} = Value.Decode(T, null)
const B: { x: number } = Value.Decode(T, null)
const C: { x: number, y: number } = Value.Decode(T, null)
const D: { x: number | string } = Value.Decode(T, null)
const E: { x: string } = Value.Decode(T, null) // cannot override known type
// ^
// Type '{ x: number; }' is not assignable to type '{ x: string; }'.
// Types of property 'x' are incompatible.
// Type 'number' is not assignable to type 'string'
Will close up this issue for now, but will keep an eye on things. Also thanks @ehaynes99 for your inputs on this one. Would be keen to get some feedback / testing on the implementation that's been published. I wouldn't expect it to breaking change, but if you do spot anything, I am open to reverting this functionality and deferred till a minor revision.
Thanks all! S
Hey, I was out on vacation last week, so sorry for the late reply. I definitely think constraining the generic parameter is valid. That at least ensures that the value "at least" adheres to the schema, even if we can inadvertently widen it. I can't think of any valid case where that is incorrect, so only upsides as I see it.
Also thanks @ehaynes99 for your inputs on this one. Would be keen to get some feedback / testing on the implementation that's been published.
Everything compiles correctly on my end with the new version. This is in a large monorepo with roughly 4,200 Type.Something.
I am actually working on a version of TypeBox in the background which is making heavy use of computed generics to optimize inference for all computed types
I didn't want to clutter up this thread, and am happy to take the discussion elswhere, but on the surface, it seems like it would be possible to have a staticDecode property on schemas that would vastly improve compiler performance and eliminate many (all?) of the recursion issues. It would just be the same as static for non-transform types. FWIW, I had to give up on StaticDecode altogether and start wrapping my transform types in Type.Unsafe due to editor performance issues. I have a very small number of transform types, and have yet to find a single instance where I actually care about the StaticEncode type, so this made a drastic improvement without giving up any type safety that I actually care about. Many of my woes ATM are more related to TypeScript's implementation of project references than anything, though.