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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: TypeScript declarations of overloaded functions could use a "most permissive" final entry #2451

Open
dietrich-iti opened this issue Apr 24, 2024 · 1 comment

Comments

@dietrich-iti
Copy link

馃攷 Search Terms

overload,LogMethod,LeveledLogMethod

The problem

The way that the type declaration file (index.d.ts) declares overloaded functions (LogMethod and LeveledLogMethod) interferes with TypeScript functionality having to do with inferring the type of arguments. This is because TypeScript features like Parameters<Type> and the infer keyword use only the final overload signature, on the assumption that it is the "permissive" implementation of all the overload signatures (see discussion and refs below).

Expected behavior: TypeScript code that does any such inference on the overloaded functions yields results that are compatible with all overload signatures. This can be achieved by adding a new final signature for each function that represents the "union" of all the others.

What version of Winston presents the issue?

v3.13.0

What version of Node are you using?

v18.9.0

If this worked in a previous version of Winston, which was it?

No response

Minimum Working Example

import { LeveledLogMethod, createLogger } from 'winston'

const logger = createLogger()

function realityAssertingLoggerInfo(...args: Parameters<LeveledLogMethod>): void {
  // args has type [infoObject: object] (from last overload of LeveledLogMethod)
  if(1 > 2) {
    throw new Error('reality is an illusion!')
  }
  logger.info(...args)
}

realityAssertingLoggerInfo({ infoObjectKey: 'infoObjectVal' }) // OK
realityAssertingLoggerInfo('a log message') // TS error: "Argument of type 'string' is not assignable to parameter of type 'object'."

Additional information

This issue came up for me while trying to create spies on a logger instance's functions using Sinon.js for testing purposes. It resulted in me having to do a bunch of ugly type assertions if I wanted to check if a logger method was called with a particular string. This is because the inner workings of Sinon (well, @types/sinon, technically) do some reflection/manipulation of the types of functions it spies on.

Info about why TypeScript uses the last overload signature:

  • This quirk is called out in the TypeScript docs under Inferring Within Conditional Types: When inferring from a type with multiple call signatures (such as the type of an overloaded function), inferences are made from the last signature (which, presumably, is the most permissive catch-all case).
  • This TypeScript issue (closed as an inherent design limitation) talks about the situation more generally.

To give an example of what the change would look like, here is an updated declaration for LeveledLogMethod that I think would work:

interface LeveledLogMethod {
  // existing signatures
  (message: string, callback: LogCallback): Logger;
  (message: string, meta: any, callback: LogCallback): Logger;
  (message: string, ...meta: any[]): Logger;
  (message: any): Logger;
  (infoObject: object): Logger;

  // Add the following as "the most permissive catch-all case"
  (messageOrInfoObject: any, ...additionalArgs: any[]): Logger;
}

Explanation:

  • Every signature has at least one argument. It can be string, object, or any -- so, effectively, it is any.
  • Beyond the first argument, anything (which includes "nothing") is valid, due to ...meta: any[] in the third signature.
  • Every signature returns Logger, so that can stay as-is.
@dietrich-iti
Copy link
Author

P.S.: I recognize that this is a pretty niche TS quirk, and there's a tradeoff to be made between the fix I proposed, vs. keeping the API of the declaration file more "clean". So you can think of it as just a suggestion. 馃槉

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

No branches or pull requests

1 participant