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

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

Closed
Retro64 opened this issue Jan 4, 2023 · 4 comments · Fixed by #44
Closed

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

Retro64 opened this issue Jan 4, 2023 · 4 comments · Fixed by #44

Comments

@Retro64
Copy link
Contributor

Retro64 commented Jan 4, 2023

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
Copy link
Member

wdavidw commented Jan 4, 2023

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

@Retro64
Copy link
Contributor Author

Retro64 commented Jan 4, 2023

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
Copy link
Member

wdavidw commented Jan 5, 2023

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

Retro64 added a commit to Retro64/node-http-status that referenced this issue Jan 9, 2023
@Retro64
Copy link
Contributor Author

Retro64 commented Jan 9, 2023

PR is open: #44

wdavidw pushed a commit that referenced this issue Jan 10, 2023
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 a pull request may close this issue.

2 participants