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

TS 3.4 Type Inference Regression #30778

Closed
bradenhs opened this issue Apr 5, 2019 · 2 comments
Closed

TS 3.4 Type Inference Regression #30778

bradenhs opened this issue Apr 5, 2019 · 2 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@bradenhs
Copy link

bradenhs commented Apr 5, 2019

TypeScript Version: 3.4.1

Search Terms: type inference

Code

type QueryParamString = "query.param.string";
type QueryParamNumber = "query.param.number";
type PathParamString = "path.param.string";
type PathParamNumber = "path.param.number";
type QueryParamStringOptional = "query.param.string.optional";
type QueryParamNumberOptional = "query.param.number.optional";

type KeysMatching<T, V> = {
  [K in keyof T]: T[K] extends V ? K : never
}[keyof T];

type ParameterDefinition =
  | QueryParamString
  | QueryParamNumber
  | PathParamString
  | PathParamNumber
  | QueryParamStringOptional
  | QueryParamNumberOptional;

type ParameterDefinitionCollection = {
  [parameterName: string]: ParameterDefinition;
};

type PathFn<T extends ParameterDefinitionCollection> = (
  params: Record<KeysMatching<T, PathParamNumber | PathParamString>, string>
) => string;

type RouteDefinitionData<T extends ParameterDefinitionCollection> = {
  params: T;
  path: PathFn<T>;
};

type RouteDefinitionDataCollection = {
  [routeName: string]: RouteDefinitionData<any>;
};

declare function defineRoute<T extends ParameterDefinitionCollection>(
  params: T,
  path: PathFn<T>
): RouteDefinitionData<T>;

declare function createRouter<T extends RouteDefinitionDataCollection>(
  routeDefinitions: T
): any;

const test1 = defineRoute({
  hello: "path.param.number"
}, p => `/${p.hello}`);

createRouter({
    test1,
    test2: defineRoute({
        hello: "path.param.number"
    }, p => `/${p.hello}`)
});

Expected behavior:

No errors and properly inferring p.hello to be of type string.

Actual behavior:

For test2 (but interestingly not test1) there is this error: "Property 'hello' does not exist on type 'Record<never, string>'."

Playground Link: https://www.typescriptlang.org/play/#src=type%20QueryParamString%20%3D%20%22query.param.string%22%3B%0D%0Atype%20QueryParamNumber%20%3D%20%22query.param.number%22%3B%0D%0Atype%20PathParamString%20%3D%20%22path.param.string%22%3B%0D%0Atype%20PathParamNumber%20%3D%20%22path.param.number%22%3B%0D%0Atype%20QueryParamStringOptional%20%3D%20%22query.param.string.optional%22%3B%0D%0Atype%20QueryParamNumberOptional%20%3D%20%22query.param.number.optional%22%3B%0D%0A%0D%0Atype%20KeysMatching%3CT%2C%20V%3E%20%3D%20%7B%0D%0A%20%20%5BK%20in%20keyof%20T%5D%3A%20T%5BK%5D%20extends%20V%20%3F%20K%20%3A%20never%0D%0A%7D%5Bkeyof%20T%5D%3B%0D%0A%0D%0Atype%20ParameterDefinition%20%3D%0D%0A%20%20%7C%20QueryParamString%0D%0A%20%20%7C%20QueryParamNumber%0D%0A%20%20%7C%20PathParamString%0D%0A%20%20%7C%20PathParamNumber%0D%0A%20%20%7C%20QueryParamStringOptional%0D%0A%20%20%7C%20QueryParamNumberOptional%3B%0D%0A%0D%0Atype%20ParameterDefinitionCollection%20%3D%20%7B%0D%0A%20%20%5BparameterName%3A%20string%5D%3A%20ParameterDefinition%3B%0D%0A%7D%3B%0D%0A%0D%0Atype%20PathFn%3CT%20extends%20ParameterDefinitionCollection%3E%20%3D%20(%0D%0A%20%20params%3A%20Record%3CKeysMatching%3CT%2C%20PathParamNumber%20%7C%20PathParamString%3E%2C%20string%3E%0D%0A)%20%3D%3E%20string%3B%0D%0A%0D%0Atype%20RouteDefinitionData%3CT%20extends%20ParameterDefinitionCollection%3E%20%3D%20%7B%0D%0A%20%20params%3A%20T%3B%0D%0A%20%20path%3A%20PathFn%3CT%3E%3B%0D%0A%7D%3B%0D%0A%0D%0Atype%20RouteDefinitionDataCollection%20%3D%20%7B%0D%0A%20%20%5BrouteName%3A%20string%5D%3A%20RouteDefinitionData%3Cany%3E%3B%0D%0A%7D%3B%0D%0A%0D%0Adeclare%20function%20defineRoute%3CT%20extends%20ParameterDefinitionCollection%3E(%0D%0A%20%20params%3A%20T%2C%0D%0A%20%20path%3A%20PathFn%3CT%3E%0D%0A)%3A%20RouteDefinitionData%3CT%3E%3B%0D%0A%0D%0Adeclare%20function%20createRouter%3CT%20extends%20RouteDefinitionDataCollection%3E(%0D%0A%20%20routeDefinitions%3A%20T%0D%0A)%3A%20any%3B%0D%0A%0D%0Aconst%20test1%20%3D%20defineRoute(%7B%0D%0A%20%20hello%3A%20%22path.param.number%22%0D%0A%7D%2C%20p%20%3D%3E%20%60%2F%24%7Bp.hello%7D%60)%3B%0D%0A%0D%0AcreateRouter(%7B%0D%0A%20%20%20%20test1%2C%0D%0A%20%20%20%20test2%3A%20defineRoute(%7B%0D%0A%20%20%20%20%20%20%20%20hello%3A%20%22path.param.number%22%0D%0A%20%20%20%20%7D%2C%20p%20%3D%3E%20%60%2F%24%7Bp.hello%7D%60)%0D%0A%7D)%3B%0D%0A

Related Issues: Maybe #30074

I honestly don't know the compiler well enough to know if this is a duplicate or not. The manifestation of this issue is certainly different than in some of the other 3.4 regression issues.

I tried to reduce noise as much as possible in the above example while still preserving context of what is actually trying to be accomplished. The above code works as expected in 3.3.4000 but not in 3.4.1.

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Apr 9, 2019
@RyanCavanaugh
Copy link
Member

The issue here is that in 3.4 you now have two competing sources of inference for T - one from the function call, and another from the contextual type of createRouter via return type inference. This is causing the hello value to not be typed as a literal type.

Possible fixes:

  1. Add const type assertion:
createRouter({
    test1,
    test2: defineRoute({
        hello: "path.param.number"
    } as const, p => `/${p.hello}`) // <-- 'as const'
});
  1. Apply a better type here:
type RouteDefinitionDataCollection = {
  [routeName: string]: RouteDefinitionData<ParameterDefinitionCollection>;
                                            // was 'any'
};
  1. Probably others but I think the second one is the best choice

@bradenhs
Copy link
Author

bradenhs commented Apr 9, 2019

Thanks Ryan! The second one is definitely the route I'd like to go down. Unfortunately, it looks like that fix doesn't work in strict mode (specifically strictFunctionTypes). You can check it out here:

https://www.typescriptlang.org/play/#src=type%20QueryParamString%20%3D%20%22query.param.string%22%3B%0D%0Atype%20QueryParamNumber%20%3D%20%22query.param.number%22%3B%0D%0Atype%20PathParamString%20%3D%20%22path.param.string%22%3B%0D%0Atype%20PathParamNumber%20%3D%20%22path.param.number%22%3B%0D%0Atype%20QueryParamStringOptional%20%3D%20%22query.param.string.optional%22%3B%0D%0Atype%20QueryParamNumberOptional%20%3D%20%22query.param.number.optional%22%3B%0D%0A%0D%0Atype%20KeysMatching%3CT%2C%20V%3E%20%3D%20%7B%0D%0A%20%20%5BK%20in%20keyof%20T%5D%3A%20T%5BK%5D%20extends%20V%20%3F%20K%20%3A%20never%0D%0A%7D%5Bkeyof%20T%5D%3B%0D%0A%0D%0Atype%20ParameterDefinition%20%3D%0D%0A%20%20%7C%20QueryParamString%0D%0A%20%20%7C%20QueryParamNumber%0D%0A%20%20%7C%20PathParamString%0D%0A%20%20%7C%20PathParamNumber%0D%0A%20%20%7C%20QueryParamStringOptional%0D%0A%20%20%7C%20QueryParamNumberOptional%3B%0D%0A%0D%0Atype%20ParameterDefinitionCollection%20%3D%20%7B%0D%0A%20%20%5BparameterName%3A%20string%5D%3A%20ParameterDefinition%3B%0D%0A%7D%3B%0D%0A%0D%0Atype%20PathFn%3CT%20extends%20ParameterDefinitionCollection%3E%20%3D%20(%0D%0A%20%20params%3A%20Record%3CKeysMatching%3CT%2C%20PathParamNumber%20%7C%20PathParamString%3E%2C%20string%3E%0D%0A)%20%3D%3E%20string%3B%0D%0A%0D%0Atype%20RouteDefinitionData%3CT%20extends%20ParameterDefinitionCollection%3E%20%3D%20%7B%0D%0A%20%20params%3A%20T%3B%0D%0A%20%20path%3A%20PathFn%3CT%3E%3B%0D%0A%7D%3B%0D%0A%0D%0Atype%20RouteDefinitionDataCollection%20%3D%20%7B%0D%0A%20%20%5BrouteName%3A%20string%5D%3A%20RouteDefinitionData%3CParameterDefinitionCollection%3E%3B%0D%0A%7D%3B%0D%0A%0D%0Adeclare%20function%20defineRoute%3CT%20extends%20ParameterDefinitionCollection%3E(%0D%0A%20%20params%3A%20T%2C%0D%0A%20%20path%3A%20PathFn%3CT%3E%0D%0A)%3A%20RouteDefinitionData%3CT%3E%3B%0D%0A%0D%0Adeclare%20function%20createRouter%3CT%20extends%20RouteDefinitionDataCollection%3E(%0D%0A%20%20routeDefinitions%3A%20T%0D%0A)%3A%20any%3B%0D%0A%0D%0Aconst%20test1%20%3D%20defineRoute(%7B%0D%0A%20%20hello%3A%20%22path.param.number%22%0D%0A%7D%2C%20p%20%3D%3E%20%60%2F%24%7Bp.hello%7D%60)%3B%0D%0A%0D%0AcreateRouter(%7B%0D%0A%20%20%20%20test1%2C%0D%0A%20%20%20%20test2%3A%20defineRoute(%7B%0D%0A%20%20%20%20%20%20%20%20hello%3A%20%22path.param.number%22%0D%0A%20%20%20%20%7D%2C%20p%20%3D%3E%20%60%2F%24%7Bp.hello%7D%60)%0D%0A%7D)%3B%0D%0A

(looks like the playground doesn't persist options selections to the url so you'll have to manually set "strictFunctionTypes" to true)

Summary of what I saw:

  • 3.3.4000 worked as expected before the fix you suggested with or without strict mode on.
  • 3.3.4000 did not work as expected after applying the fix with strict mode on.
  • 3.4.1 didn't work before applying the fix you suggested.
  • 3.4.1 did work after the fix without strict mode on but with strict mode it does not work.

Any suggestions here? I'd like to get this working no matter the compiler settings someone has set. I really appreciate your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

3 participants