adaltas / node-http-status

Utility to interact with HTTP status code in Node.js
Other
441 stars 55 forks source link

Types are incorrect - lib returns undefined for unknown status codes #43

Closed Retro64 closed 1 year ago

Retro64 commented 1 year ago

The library returns undefined for unknown status codes, which is not reflected in the types.

import * as HttpStatus from 'http-status';

const ok = HttpStatus[200];
const thisIsUndefinedAndNotAString = HttpStatus[777];

console.log(typeof ok); // prints string
console.log(ok); // prints ok
console.log(typeof thisIsUndefinedAndNotAString); // this prints undefined
console.log(thisIsUndefinedAndNotAString); // this prints undefined

So I guess this would be a rather correct type:

declare namespace httpStatus {
  interface HttpStatus {
    readonly [key: number]: string | undefined

    readonly [key: string]:
      | string
      | number
      | HttpStatusClasses
      | HttpStatusExtra
      | undefined;

For convenience - as the lib might quite often be used to trace foreign messages, where you might not rely on correct content - the types might even be extended (last lines to give concrete values priority) with:

readonly [key: undefined]: undefined

or

readonly [key: unknown]: undefined

But these are really optional and opinionated.

I might provide a PR with the fix and/or the feature, if it is accepted

wdavidw commented 1 year ago

This feels kind of stage. @Retro64 is this a common practice ? Doesn't anyone with a good TS experience could validate ?

Retro64 commented 1 year ago

Minimum example, why this might be dangerous:

import * as HttpStatus from 'http-status';

const foo = (bar: string) =>
    console.log(bar.toLowerCase);

const test = HttpStatus[777];

foo(test);

Compiles properly, as test is typed as string, fails at runtime, as test is no string. Indicating it might be undefined leads to a warning it might not be defined during compile time.

wdavidw commented 1 year ago

OK, I get it, thank you. Please prepare a PR.

Retro64 commented 1 year ago

PR is open: https://github.com/adaltas/node-http-status/pull/44