-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Incorrect inference when using an optional argument #57021
Comments
I have some idea how this might get fixed. Stay tuned π I wonder though - what's |
It's an inferable schema object (something like Zod):
We infer the type from this schema library and thus instead of
|
Ye, I get that - but I wonder what's the nature of the outer 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 It's funny-ish that the intra-expression inference is responsible for making this work: type Schema = Record<string, unknown>;
type StepFunction<TSchema extends Schema = Schema> = (anything: unknown) => {
readonly schema: (thing: number) => TSchema;
readonly toAnswers?: (keys: keyof TSchema) => unknown;
};
function step<TSchema extends Schema = Schema>(
stepVal: StepFunction<TSchema>,
): StepFunction<TSchema> {
return stepVal;
}
const nowItsOk = step((_something) => ({
schema: (thing) => ({
attribute: "anything",
}),
toAnswers: (keys) => {
keys;
// ^?
type Test = string extends typeof keys ? never : "true";
const test: Test = "true"; // ok
return { test };
},
})); 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. |
Oh, I thought you meant for the inner It's a type StoreFor<TSchema extends Schema> = {
get: <T extends keyof TSchema>(key: T) => TSchema[T]
}
type StepFunction<
TSchema extends Schema = Schema,
> = (store: StoreFor<TSchema>) => {
readonly schema: TSchema
readonly toAnswers?: (keys: keyof TSchema) => unknown
} 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. |
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 π |
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 |
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 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
e.g. in this example you can assume I have a |
To make things simpler - the step function is actually generated through something I call
|
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. |
This is basically #47599, specifically the part about
|
I don't quite understand this quote here. Doesn't 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. |
Poor wording on my part. What I mean there is that because |
This issue has been marked as "Duplicate" and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
@Andarist should have it been closed? |
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 π |
Amazing, thanks for your effort! |
Hey @Andarist - did you have any chance to look into this? |
π 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
Link
π» Code
This is a followup on #49951 which @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:
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 doParameters<typeof myStepValue>
even when the argument is missing, it's inferred correctly (!)π 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
The text was updated successfully, but these errors were encountered: