fix: weaken internal group types to strengthen return/onCancel types #245
Conversation
This changes a few things.
First of all, trying to infer `T` while defining `T` currently doesn't
work unless there's a member without the `results` arg. This is a known
limitation that may or may not be fixed in typescript one day.
Until that happens, if ever, this dumbs down the type of `results` to be
a `Record<PropertyKey, unknown>`. This means _within a function_ you
lose strong typing, but the return type will now always be correct.
Secondly, `T` is actually the resulting already-awaited shape. This
means we do not need `Awaited<T[K]>` since `T[K]` is already the awaited
type.
For example:
```ts
type ActualType = {
foo: number;
bar: number;
};
group<ActualType>({
foo: () => Promise.resolve(303),
bar: () => Promise.resolve(808)
});
```
You can see the `ActualType` never needed `Awaited<T>` on each type
since it is already the final result.
|
|
Open in Stackblitz • @example/basic • @example/changesets commit: |
|
some examples before this PR: // Correct return type, correct `results` type in 2nd function
group({
foo: () => {
return Promise.resolve(303);
},
bar: ({results}) => {
results.foo; // works, its a number!
return Promise.resolve(808);
}
});
// Incorrect return type of {}, incorrect `results` type in both functions of {}
group({
foo: ({results}) => {
results.bar; // doesn't work, {} has no bar property
return Promise.resolve(303);
},
bar: ({results}) => {
results.foo; // doesn't work, {} has no foo property
return Promise.resolve(808)
}
});examples after this PR: // Correct return type, weak `results` type in functions
group({
foo: () => {
return Promise.resolve(303);
},
bar: ({results}) => {
results.foo; // works, but its unknown!
return Promise.resolve(808);
}
});
// Correct return type, weak `results` type in both functions
group({
foo: ({results}) => {
results.bar; // works but is unknown!
return Promise.resolve(303);
},
bar: ({results}) => {
results.foo; // works but is unknown!
return Promise.resolve(808)
}
}); |
|
Another approach is using class: Typescript Playground Usage screenshot
With this:
But the DX changes, it's not just a simple function with 2 parameters, but a class instance with multiple functions calls |
|
Yes that pretty much works just because of the method chaining. But I don't think we can change the dx so drastically and inconsistently with the other functions So we're still left with "weak types inside, but correct everywhere" or "strong types everywhere, but incorrect in most situations" |
|
Hmm I'm still not sure where I fall on this... we are looking ahead to a stable v1, so I think it would be okay to introduce a new approach for grouping prompts and deprecate |
|
that makes sense to me introduce a new interface we can more easily type too and just drop the old one in 2.x we can have a think about the shape of that on discord |
|
Related to #139 (for the chaining implementation) |
This changes a few things.
First of all, trying to infer
Twhile definingTcurrently doesn't work unless there's a member without theresultsarg. This is a known limitation that may or may not be fixed in typescript one day.Until that happens, if ever, this dumbs down the type of
resultsto be aRecord<PropertyKey, unknown>. This means within a function you lose strong typing, but the return type will now always be correct.Secondly,
Tis actually the resulting already-awaited shape. This means we do not needAwaited<T[K]>sinceT[K]is already the awaited type.For example:
You can see the
ActualTypenever neededAwaited<T[K]>on each type since it is already the final result.Importantly this does weak types within the callbacks/functions. so we need to decide between these options:
resultsis correctly typed and the return type is tooresultsis weakly typed but everything else is always correctly typed