Repository navigation
Incorrect inference when using an optional argument #57021
Description
🔎 Search Terms
Inference, Return, Arguments, Parentheses, Identity Function.
🕗 Version & Regression Information
This reproduces in all available playground versions (tried down to 3.3.3), and also in 5.3.3.
⏯ Playground Link
💻 Code
This is a followup on #49951 which Mateusz Burzyński (@Andarist) asked me to open a follow-up issue. Also relates to #47599
Probably better to check the playground for code examples, but here's the TL;DR:
TYPES DECLERATIONS:
type Schema = Record<string, unknown> // modified from original for the sake of the example, if it doesn't make sense
type StepFunction<
TSchema extends Schema = Schema,
> = (anything: unknown) => {
readonly schema: TSchema
readonly toAnswers?: (keys: keyof TSchema) => unknown
}
function step<TSchema extends Schema = Schema>(
stepVal: StepFunction<TSchema>,
): StepFunction<TSchema> {
return stepVal
}EXAMPLES:
Notice the returned object of all functions is the same! The difference is in whether we have the argument for the step function or not. Note that if I do Parameters<typeof myStepValue> even when the argument is missing, it's inferred correctly (!)
// WORKS: `keys` is inferred based on the `schema`
// - no argument for `step` function
// - no `return` keyword
const workingExample = step(() => ({
schema: {
attribute: 'anything',
},
toAnswers: keys => {
// RESULT: `keys` inferred successfully as `attribute`
type Test = string extends typeof keys ? never : 'true'
const test: Test = 'true'
return { test }
},
}))// FAILS: `keys` is not inferred based on the `schema`
// - has argument for `step` function
const nonWorkingA = step(_something => ({
schema: {
attribute: 'anything',
},
toAnswers: keys => {
// RESULT: `keys` failed to be inferred hence defaults to `string`
type Test = string extends typeof keys ? never : 'true'
const test: Test = 'true'
return { test }
},
}))🙁 Actual behavior
Nested value's function argument cannot be inferred correctly.
If I change the property (toAnswers) from a function to a simple property, there are no inference issues.
🙂 Expected behavior
Nested value function argument should be inferred correctly regardless of the return keyword or declaring the arguments.
Additional information about the issue
Like mentioned above, this is a followup on #49951 and #47599
Activity
Andarist commentedon Jan 11, 2024
I have some idea how this might get fixed. Stay tuned 😉 I wonder though - what's
anythingin your real code? Is it really meant to always beunknown?ShayDavidson commentedon Jan 12, 2024
It's an inferable schema object (something like Zod):
We infer the type from this schema library and thus instead of
keyswe actually get the actual shape of the object. In this case:Andarist commentedon Jan 12, 2024
Ye, I get that - but I wonder what's the nature of the outer
anythingparameter. It stays unused in this example.I think the fix for this is introducing an extra inference pass on return expressions from context-sensitive functions. I was thinking first about extending the intra-expression inference to handle this but I think that would be a worse solution. It wouldn't handle
{ toAnswers, schema }(consumer before producer).It's funny-ish that the intra-expression inference is responsible for making this work:
Usually, context-sensitive functions make things to be inferred worse (like in your original example) - but this example above infers better when an extra context-sensitive function is included. This is a little bit unusual and purely related to how all of this is already nested in one context-sensitive function.
ShayDavidson commentedon Jan 12, 2024
Oh, I thought you meant for the inner
anything.It's a
storethat allows to get data based on the inference of the schema, something like:Still fails and demonstrated in this playground link.
Update:
See below, this is not a good example. The store's schema should be different. Elaborated later in the thread but keeping the original so the context for the conversation remains.
ShayDavidson commentedon Jan 12, 2024
As for adding a function - well, I could add it - but it's a breaking change for the consumers of my library hence I prefer to refrain from it 😅
Andarist commentedon Jan 12, 2024
Oh, I certainly didn't imply that you should change this anyhow. I just forked ur example to demonstrate a TS behavior 😉 it was more convenient than having to retype the same thing with some dummy function names etc.
Do you have any example of when this
storewould actually be used and when its type argument would be inferred? My hot take on this example is that there is no easy way to make this one work. It's a circular dependency of sorts. Your return type might depend on this parameter but its type depends on that return type.ShayDavidson commentedon Jan 12, 2024
Yes, I'll demonstrate how this is used in actual code more or less - I understand why this is confusing.
The schema defined in my example is not the same schema defined for the
store.Imagine I'm writing a wizard flow, and I have a parent global schema, that I can get previously answered values through the store. Each step in the wizard can define a sub-schema for transient answers (which is defined through the
schemaattribute). ThetoAnswersfunction simply takes these transients answers and needs to return an object that satisfies the parent schema partially.e.g. in this example you can assume I have a
storeargument that knows it supports only the parent-schema, which hasfirstNamehas an attribute, but is completely decoupled from theschemainside the step definition.ShayDavidson commentedon Jan 12, 2024
To make things simpler - the step function is actually generated through something I call
StepCreator, which literally accepts the parent-schema as argument (so no need to do hard inference there):Andarist commentedon Jan 12, 2024
Ah, ok - then the snipper in #57021 (comment) doesn't exactly show the real usage. Thanks for the explanation.
If those are two different schemas then ye, it might work.
RyanCavanaugh commentedon Jan 12, 2024
This is basically #47599, specifically the part about
Andarist commentedon Jan 12, 2024
I don't quite understand this quote here. Doesn't
_a => 0useTin a covariant position? This0is a potential inference source forT.Regardless, I think this particular issue here can be solved. The fix wouldn't have to rely on any change to context-sensitivity. It's just that returns from context-sensitive functions are not inferred from today (whereas annotated parameters are inferred from). Inferring from such returns recursively would fix this. I plan to make a PR for it soon-ish.
#47599 was already mentioned in this thread but I find this umbrella issue to not be that helpful. All of the called-out examples in the first post work today. I understand that some of the ones posted below might still not work but it's quite hard to understand at a glance what still doesn't work. At the very least that first post could be revised to include those other broken examples.
RyanCavanaugh commentedon Jan 12, 2024
Poor wording on my part. What I mean there is that because
_a's corresponding position in(n: number) => Tisn: number, which has no occurrence ofT, then we can be sure that treating_a => 0as not-context-sensitive for the purposes of inferringTis safe.typescript-bot commentedon Jan 15, 2024
This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes.
ShayDavidson commentedon Jan 15, 2024
Mateusz Burzyński (@Andarist) should have it been closed?
Andarist commentedon Jan 15, 2024
It has been marked as a duplicate of that umbrella issue - and many issues were linked to it as its duplicates. I think this one was nice since it focused on a more specific problem. I still think this one is fixable (I fixed it locally without breaking any test cases). It was a busy weekend for me though and I'm investigating a better fix than what I have so far. I'll report back when I'm done but it might take some days. TLDR: I still plan to open a PR fixing this 😉
ShayDavidson commentedon Jan 15, 2024
Amazing, thanks for your effort!
ShayDavidson commentedon Feb 11, 2024
Hey Mateusz Burzyński (@Andarist) - did you have any chance to look into this?