Skip to content

Forgive "No overload matches this call" for union-type arguments, if assignment can satisfy a union of overloads #44452

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

Closed
5 tasks done
Birch-san opened this issue Jun 5, 2021 · 4 comments
Labels
Duplicate An existing issue was already created

Comments

@Birch-san
Copy link

Suggestion

When determining if a union-typed argument is assignable to the parameters of an overloaded function: we should allow the assignment if — for each member in that union — there exists a compatible overload. the returned type should be the union of all applicable overloads.

// current state of TS compiler, as of TS v4.3.2
function double(val: number): number;
function double(val: string): string;
function double(val: string | number): string | number {
    if (typeof val === 'string') {
        return `${val}${val}`;
    }
    return val*2;
}

const num: number = double(2); // works fine
const str: string = double('hey'); // works fine

const process = (val: string | number): void => {
    double(val); // fails with "No overload matches this call"
    const doubled: string | number = double(val); // fails for same reason
}

The externally-visible overloads of double are:

(val: number): number;
(val: string): string;

Neither overload (taken individually) can accept a val: string | number, but we can see that for each member in the union string | number, there is a compatible overload.
There's enough information here to deduce that "assigning either string or number is supported", and that "if we did so: the result would be either string or number".

In the current example, the union of applicable overloads (val: string | number): string | number happens to be equivalent to making the implementation signature visible.
but this will not be true in the general case — for example if an overload (val: Date): Date; existed: it would not be considered applicable (the implementation signature would be required to accommodate it, whereas our union of applicable overloads could exclude it).

🔍 Search Terms

No overload matches this call.
The call would have succeeded against this implementation, but implementation signatures of overloads are not externally visible.

✅ Viability Checklist

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Elaborated in full above.

📃 Motivating Example

I hope the minimal repro above is sufficiently clear.

A brief summary could be:
Permit assignment of union-type arguments to overloaded functions, if that assignment can be satisfied by a union of overloads. This eliminates a case where the compiler might reject an assignment, stating "No overload matches this call".

💻 Use Cases

A less contrived example is what I'm trying to contribute to the Snowpack devserver code — I receive a request req: http.IncomingMessage | http2.Http2ServerRequest, and wish to pluck serializable properties off that union argument, creating a new union Http1RequestData | Http2RequestData.

/**
 * {@link http.IncomingMessage} as a value type -- exposes data properties
 * without exposing access to sockets, streams, methods or iterators.
 */
export type Http1RequestData = Pick<
http.IncomingMessage,
'aborted' | 'httpVersion' | 'httpVersionMajor' | 'httpVersionMinor' | 'complete' | 'headers' | 'rawHeaders' | 'trailers' | 'rawTrailers' | 'method' | 'url'
>
/**
 * {@link http2.Http2ServerRequest} as a value type -- exposes data properties
 * without exposing access to sockets, streams, methods or iterators.
 */
export type Http2RequestData = Pick<
http2.Http2ServerRequest,
'aborted' | 'httpVersion' | 'httpVersionMajor' | 'httpVersionMinor' | 'complete' | 'headers' | 'rawHeaders' | 'trailers' | 'rawTrailers' | 'method' | 'url' | 'authority' | 'scheme'
>
/**
 * {@link http.IncomingMessage} or {@link http2.Http2ServerRequest} as a value type -- exposes data properties
 * without exposing access to sockets, streams, methods or iterators.
 * This is provided so that users of dev-server APIs may make decisions about
 * how to route a request (or transform its response), without being able to perform
 * side-effects upon the request handle.
 */
export type HttpRequestData = Http1RequestData | Http2RequestData;

function isHttp2Request(req: http.IncomingMessage | http2.Http2ServerRequest): req is http2.Http2ServerRequest {
  return req.httpVersionMajor === 2;
}

function dataPropertiesOfRequest(req: http.IncomingMessage): Http1RequestData;
function dataPropertiesOfRequest(req: http2.Http2ServerRequest): Http2RequestData;
function dataPropertiesOfRequest(req: http.IncomingMessage | http2.Http2ServerRequest): HttpRequestData {
  // there's a large number of props common to both request types, but since they have different optionality and readonly-ness:
  // I've opted to resist doing a "destructure common props".
  if (isHttp2Request(req)) {
    const {aborted, httpVersion, httpVersionMajor, httpVersionMinor, complete, headers, rawHeaders, trailers, rawTrailers, method, url, authority, scheme} = req;
    const requestData: Http2RequestData = {aborted, httpVersion, httpVersionMajor, httpVersionMinor, complete, headers, rawHeaders, trailers, rawTrailers, method, url, authority, scheme};
    return requestData;
  }
  const {aborted, httpVersion, httpVersionMajor, httpVersionMinor, complete, headers, rawHeaders, trailers, rawTrailers, method, url} = req;
  const requestData: Http1RequestData = {aborted, httpVersion, httpVersionMajor, httpVersionMinor, complete, headers, rawHeaders, trailers, rawTrailers, method, url};
  return requestData;
}

const server: http.Server | http2.Http2Server = createServer(async (
      req: http.IncomingMessage | http2.Http2ServerRequest,
      res: http.ServerResponse | http2.Http2ServerResponse
    ) => {
  // this assignment fails!
  const requestData: HttpRequestData = dataPropertiesOfRequest(req);

  // this assignment fails too! for brevity I won't show the `convertToSnowpackResponse` function, 
  // or `SnowpackHttpServerResponse` union type — it's the same idea as above.
  // mainly showing this to demonstrate that the problem hit me twice.
  const snowpackResponse: SnowpackHttpServerResponse = convertToSnowpackResponse(requestData, res);
});

Other options could be to make the union of overloads externally-visible (which means repeating yourself, and serves counter to the purpose of my providing overloads with correlated parameter/return types):

function dataPropertiesOfRequest(req: http.IncomingMessage): Http1RequestData;
function dataPropertiesOfRequest(req: http2.Http2ServerRequest): Http2RequestData;
function dataPropertiesOfRequest(req: http.IncomingMessage | http2.Http2ServerRequest): HttpRequestData;
function dataPropertiesOfRequest(req: http.IncomingMessage | http2.Http2ServerRequest): HttpRequestData {

Another option is type-narrowing & duplication at the call-site:

// recall that `type HttpRequestData = Http1RequestData | Http2RequestData;`
let requestData: HttpRequestData | undefined;
if (isHttp2Request(req)) {
  // req is narrowed to http2.Http2ServerRequest; expression returns Http2RequestData
  requestData = dataPropertiesOfRequest(req);
} else {
  // req is narrowed to http.IncomingMessage; expression returns Http1RequestData
  requestData = dataPropertiesOfRequest(req);
}
assert(requestData); // make the variable truthy from now on

this wasn't super nice because we had to add a condition, a type-guard, implementation duplication and a truthiness assertion at runtime, purely to satisfy the type-system.

for this particular feature, I have the option of hand-waving a bit — the existing code starts with req: any, downstream code assumes req: http.IncomingMessage, so I could just tiptoe around that rather than trying to retrofit typesafety.

@MartinJohns
Copy link
Contributor

Duplicate of #44392 and many more. This is intentional. You're free to add another signature accepting the union type.

@Birch-san
Copy link
Author

@MartinJohns do we know the reason why this is intended? it was unintuitive to me.

@MartinJohns
Copy link
Contributor

It's computationally too expensive for too little of a benefit. You just picked the most simple case.

#1805 (comment)

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Jun 9, 2021
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' 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
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants