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

Incorrect overload resolution in 3.2.0-rc #28567

Closed
Jessidhia opened this issue Nov 16, 2018 · 3 comments
Closed

Incorrect overload resolution in 3.2.0-rc #28567

Jessidhia opened this issue Nov 16, 2018 · 3 comments
Assignees
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@Jessidhia
Copy link

Jessidhia commented Nov 16, 2018

TypeScript Version: 3.2.0-rc

Search Terms:
Overload

Code

import { createStore } from 'redux'

const initialStore: { foo: {} } = { foo: {} }
const store = createStore(x => x, initialStore)

Expected behavior:

The second overload of createStore should be chosen, as initialStore is not a StoreEnhancer but a preloadedState.

Actual behavior:

The StoreEnhancer, a function, overload of createStore is chosen despite initialStore being absolutely, definitely not a function, causing the type of x to be incorrectly inferred as {} | undefined.

@ahejlsberg
Copy link
Member

The error is not that we're picking the wrong overload, but rather that none of the overloads are applicable.

Here's a simplified repro:

declare function test<T>(f: (x: T | undefined) => T, enhancer: () => void): T;
declare function test<T>(f: (x: T | undefined) => T, state: T): T;

test(x => x, { a: 'hello' });  // Ok in 3.1, error in 3.2
test(x => x || { a: 'xxx' }, { a: 'hello' });  // Ok in both

The correct inference for T is { a: string } and therefore it is an error for the arrow function to return x ({ a: string } | undefined is not assignable to { a: string }).

The reason it succeeds in 3.1 is somewhat subtle: We do multiple passes on the overloaded signatures, initially making an inference of { a: string } for T. In a later pass we end up with a contra-variant inference of { a: string } and a co-variant inference of { a: string } | undefined. In 3.1 we'd then go with the less specific inference of { a: string } | undefined, but in 3.2 we now prefer the more specific and more correct { a: string }. This changed in the commit here as part of #27028.

The fact that 3.1 gets it wrong becomes obvious if you comment out the first overload (which shouldn't really matter as it is not applicable). 3.1 then behaves the same as 3.2.

@Jessidhia
Copy link
Author

I see. This is likely then an error with the redux type definitions -- if there is an initial state, then the reducer won't be called with undefined but rather with... the initial state.

@ahejlsberg ahejlsberg added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript High Priority labels Nov 19, 2018
@ahejlsberg ahejlsberg removed this from the TypeScript 3.2.1 milestone Nov 19, 2018
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants