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

Generic type doesn't infer correctly without number type in function #51826

Closed
wbolduc opened this issue Dec 8, 2022 · 3 comments
Closed

Generic type doesn't infer correctly without number type in function #51826

wbolduc opened this issue Dec 8, 2022 · 3 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@wbolduc
Copy link

wbolduc commented Dec 8, 2022

Bug Report

number annotation is required despite typescript correctly identifying that same type

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

interface QueryLoading {
	isSuccess: false;
	data: undefined;
}
interface QueryLoaded<T> {
	isSuccess: true;
	data: T;
}
type UseQueryResult<T> = QueryLoaded<T> | QueryLoading;

interface Thing {
	id: number;
	name: string;
	description: string;
}
const useGetThing = (id: number | null | undefined): UseQueryResult<Thing> =>
	id
		? {
				isSuccess: true,
				data: { id, name: "fake name", description: "fake description" },
		  }
		: { isSuccess: false, data: undefined };

/// pretend to be reactquery ^^

interface NameId {
	id: number;
	name: string;
}
interface WeirdRendererProps<T extends NameId, FieldName extends keyof T> {
	id: number;
	useGetter: (id: number) => UseQueryResult<T>;
	field_name: FieldName;
	render: (value: T[FieldName]) => string;
}

const weirdRenderer = <T extends NameId, FieldName extends keyof T>({
	id,
	useGetter,
	field_name,
	render,
}: WeirdRendererProps<T, FieldName>) => {
	const { data, isSuccess } = useGetter(id);
	return isSuccess ? render(data[field_name]) : null;
};

export const A = () =>
	weirdRenderer({
		id: 1,
		useGetter: (i_am_a_number: number) => useGetThing(i_am_a_number),
		//                            ^ Why does it seem like 'number' is required here to get the types to work
		field_name: "description",
		render: (value) => value.toString(),
	});

export const B = () =>
	weirdRenderer({
		id: 1,
		useGetter: (i_am_a_number) => useGetThing(i_am_a_number),
		//               ^?
		field_name: "description",
		render: (value) => value.toString(),
	});

export const C = () =>
	weirdRenderer({
		id: 1,
		useGetter: useGetThing,
		field_name: "description",
		render: (value) => value.toString(),
	});

πŸ™ Actual behavior

typescript is throwing an error off of the generic

πŸ™‚ Expected behavior

If typescript can correctly identify the type of the function arguments it should make no difference if I manually add or remove that annotation

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Dec 8, 2022
@RyanCavanaugh
Copy link
Member

The order of inference that would be needed here -- first check the return type of useGetThing, then check field_name, then figure out render's parameter type -- is pretty far off from a supported ordering of inference steps. See #30134

@wbolduc
Copy link
Author

wbolduc commented Dec 11, 2022

What I don't understand here though is that in the playground it looks like ts is correctly identifying it as a number. If it can give me the intelisense on what that value should be why does it matter if I do or do not specify it explictily

@RyanCavanaugh
Copy link
Member

Intellisense doesn't compute the entire type flow of the program -- instead, it tries to compute the minimum amount possible so that results can come back faster. In some cases like this, these shortcuts can produce slightly inaccurate results compared to what would be produced from a proper inference.

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

2 participants