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

Type inference breaks without currying but compiles with currying #12621

Closed
silviogutierrez opened this issue Dec 2, 2016 · 4 comments
Closed
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@silviogutierrez
Copy link

TypeScript Version: 2.1.1 / nightly (2.2.0-dev.20161202)

I'm not sure if this is the expected behavior, but it seems like the compiler does have enough information to infer correctly (given that the second example works), but fails on a single function.

Take the following code:

interface Options<TParams, TStateProps> {
    params: TParams;
    mapStateToProps: (props: {params: TParams}) => TStateProps;
    component: (props: {params: TParams} & TStateProps) => any;
}

function inferenceTest<TParams, TStateProps>(options: Options<TParams, TStateProps>) {
}


inferenceTest({
    params: {
        someRandomParam: '1',
        anotherOne: '1',
    },
    mapStateToProps: (props) => ({
        someStateParam: parseInt(props.params.someRandomParam),
    }),
    // This last line fails. The compiler cannot find props.someStateParam even though we inject it.
    component: (props) => alert(props.params.someRandomParam + props.someStateParam),
})

If, however, we break up the function, like so:

interface FirstOptions<TParams, TStateProps> {
    params: TParams;
    mapStateToProps: (props: {params: TParams}) => TStateProps;
}

interface SecondOptions<TParams, TStateProps> {
    component: (props: {params: TParams} & TStateProps) => any;
}

function curriedInferenceFirst<TParams, TStateProps>(options: FirstOptions<TParams, TStateProps>) {
return function curriedInferenceSecond(options: SecondOptions<TParams, TStateProps>) {
}
}

curriedInferenceFirst({
    params: {
        someRandomParam: '1',
        anotherOne: '1',
    },
    mapStateToProps: (props) => ({
        someStateParam: parseInt(props.params.someRandomParam),
    }),
})({
    // This compiles just fine.
    component: (props) => alert(props.params.someRandomParam + props.someStateParam),
})

This seems unexpected. The latter is a workaround for deeply inferred literals, but it'd be great to find the root cause so you don't need to break up the functions for each infer point. You can see how the more inference you need, the more curried functions you will need to use.

Curiously, if you even hint at the compiler a bit in the original function, it will resume working:

inferenceTest({
    params: {
        someRandomParam: '1',
        anotherOne: '1',
    },
    // Note the manual typing of params.
    mapStateToProps: (props: {params: {someRandomParam: string, anotherOne: string}}) => ({
        someStateParam: parseInt(props.params.someRandomParam),
    }),
    component: (props) => alert(props.params.someRandomParam + props.someStateParam),
})

Am I missing something here?

@RyanCavanaugh
Copy link
Member

@JsonFreeman shouldn't all the return types get picked up in the first pass, then we're good during the parameter fixing portion?

@JsonFreeman
Copy link
Contributor

JsonFreeman commented Dec 6, 2016

I haven't looked at the code recently, but here is what I think is going on based on my memory.

In the first pass, we process the arguments except nested functions that are contextually typed. Any nested contextually typed functions are deferred, including resolving their return types. This means that we do not find out anything about TStateProps until the second pass. However, in the second pass, we need to fix TStateProps, so we never get a chance to infer inference candidates to it.

In the curried version, this is not an issue because the fixing of TStateProps is pushed into another function altogether, so we are free to infer a candidate for it in the first part of the curry.

In the version with the manual type annotation, mapStateToProps is not contextually typed, so the resolution of its return type is done in the first pass. Therefore, by the time we need to fix TStateProps, we have already inferred a candidate for it.

To summarize, I guess the crux of the issue is that in the current algorithm, both functions require their parameters to be fixed in the same pass. If the contextually typed functions are nested in different arguments, they are processed in different passes, but in this case, they are nested in the same argument of the surrounding call expression.

In order to make the algorithm smarter, we could try to process each contextually typed function in a separate pass, even within the same argument. But then we have to figure out what order to process them. And the order is not obvious.

@silviogutierrez
Copy link
Author

@JsonFreeman : when you put it that way, it makes sense why the curried one would work. You're hinting at the algorithm what the order should be. I can see how without this, you could even get into circular dependencies.

@JsonFreeman
Copy link
Contributor

Yes, exactly. It would not know whether to do mapStateToProps first, or component first.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@RyanCavanaugh RyanCavanaugh added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Needs Investigation This issue needs a team member to investigate its status. labels Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

3 participants