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

Contextually type unions of differing signatures, rather than giving up #17819

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Aug 15, 2017

If a function expression was contextually typed by multiple signatures, and those signatures were not sufficiently similar, we gave up on contextually typing the function, according to our specification. This results in a noImplicitAny error, if enabled. With this change, we generate a synthetic signature whose arguments are the union of all possibilities for those arguments, and whose return is the intersection of all of the returns those expect (🚲 🏠 - since contextual typing is only performed on implementations, it is likely safe and more useful to use a union of return types instead, like we do for similar signatures; but I'd like some feedback before jumping to that conclusion), and use that to inform contextual typing decisions, rather than giving up. This gives better inferences for the arguments of function expressions contextualized by non-like signatures, however the downside is that this is less permissive outside of strict mode than we were before (as previously it would be typed any).

If this is too much of a breaking change (see functionExpressionContextualTyping1.errors.txt), I think it is OK to have this gated behind the noImplicitAny flag, since the inference result prior to this would have been an error anyway.

Fixes #16019. (The root issue was the that interfaces used yielded two call signatures for a ref, each of which with a different parameter type; causing inference to fail.)

This is also tangentially related to #7763, as the same signature-merging methodology could be used for the inference of the implementation signature.
Additionally, this could help remove the design limitation in the root issue of #11936 (by fixing the contextual typing - not changing overload resolution; all that would need to be done is to treat overloads, once contextual signature finding has failed, the same as a union of those signatures).

TL;DR:
With this change, given this code:

type Attributes = { ref: (x: A) => void } & { ref: (x: B) => void };
declare function f(cb: Attributes): void;

f({
    ref: x => x
});

We now infer the type of x as A | B instead of any.

@ghost
Copy link

ghost commented Sep 8, 2017

Might fix #17127?

@weswigham
Copy link
Member Author

weswigham commented Sep 21, 2017

Per brief post-design meeting discussion from the week before last, I've "ripped off the bandaid", as it were, and now combine signatures regardless of if the source was a union or not, for the purpose of contextual typing. As:

  1. If the source is a union, then you could fulfill any one signature, so a union of parameter type allows you the most information during your initial definition until you need to narrow what you handle; while also still being compatible with the union itself - in effect, this is the broadest compatible type (without knowledge of supertypes) you could write.
  2. If the source is an intersection or object literal type then you must fulfill all present signatures, and so being given a union of parameter types is the minimal type required to actually fulfill the constraint.

I'll report on how this affects RWC results shortly.

@weswigham weswigham force-pushed the contextually-type-complex-signature-unions branch from 33cc0c5 to c11d001 Compare September 22, 2017 23:19
@simonbuchan
Copy link

The mentioned issues are different enough in details, would this fix this should-be-common case?

interface NodeCallback<T> {
  (err: any, result?: undefined | null): void;
  (err: undefined | null, result: T): void;
}
const cb: NodeCallback<T> = (err, result) => { // 'err', 'result' are implicitly any
  // ...
};

Node-style callbacks are (obviously!) very commonly used, and at the moment have a too-weak type on the consumer (callback caller) side, presumably because otherwise actually providing the callback is too verbose.

@typescript-bot
Copy link
Collaborator

Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React: Type inference not working for ref callback
5 participants