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

Unable to properly infer numeric keys in TypeScript type with mixed key types #48

Closed
aronmal opened this issue Feb 22, 2024 · 4 comments · Fixed by #49
Closed

Unable to properly infer numeric keys in TypeScript type with mixed key types #48

aronmal opened this issue Feb 22, 2024 · 4 comments · Fixed by #49

Comments

@aronmal
Copy link
Contributor

aronmal commented Feb 22, 2024

Describe the bug

When utilizing the http-status package, there is a bug related to inferring numeric keys in TypeScript when working with a mixed type declaration within the HttpStatus interface in index.d.ts.

To Reproduce

To reproduce the issue, consider the following TypeScript code snippet:

import httpStatus from "http-status"; // object itself
import type { HttpStatus } from "http-status"; // interface

// example for comparison
const testStatus = {
  100: "",
  "100_NAME": "",
  "100_MESSAGE": "",
  CONTINUE: 100,
  101: "",
  "101_NAME": "",
  "101_MESSAGE": "",
  SWITCHING_PROTOCOLS: 101,
  102: "",
  "102_NAME": "",
  "102_MESSAGE": "",
  PROCESSING: 102,
  103: "",
  "103_NAME": "",
  "103_MESSAGE": "",
  EARLY_HINTS: 103,
};

// Define a custom type to extract numeric keys
type NumberKeys<T> = {
  [K in keyof T]: T[K] extends number ? (string extends K ? never : K) : never;
}[keyof T];

// Define a function with a parameter that should only accept numeric keys
export function Response(code: NumberKeys<HttpStatus>) {
  const thisCode = httpStatus[code];
  // Rest of your function logic
}

In this case, the types do not work correctly and autocompletion does show any possible string in the ide
image

When changing the type input to the example object:

- export function Response(code: NumberKeys<HttpStatus>) {
+ export function Response(code: NumberKeys<typeof test>) {
    const thisCode = httpStatus[code];
    // Rest of your function logic
  }

it then works and shows the correct types

image

Additionally, the unnecessary declaration readonly [key: number]: string | undefined; in the HttpStatus interface contributes to the issue and might be impacting TypeScript type inference.

Additional context

The issue seems to be associated with the mixed type declaration within the HttpStatus interface in index.d.ts, specifically:

export = httpStatus;

declare const httpStatus: httpStatus.HttpStatus;

declare namespace httpStatus {
  interface HttpStatus {
    // not important for this issue, but I still see no sense in this either
    readonly [key: number]: string | undefined;

    // this causes the issue
    readonly [key: string]:
    | string
    | number
    | HttpStatusClasses
    | HttpStatusExtra
    | undefined;

This declaration allows for both string and number keys, leading to unexpected behavior in TypeScript type inference. The bug impacts TypeScript type inference in projects utilizing the http-status package, particularly when trying to enforce strict typing for numeric keys. The unnecessary readonly [key: number] declaration are causing the issue and should be considered for removal.

@aronmal
Copy link
Contributor Author

aronmal commented Feb 22, 2024

Is there a reason for the readonly [key: number]? If not I can create a pull request, but I wanted to ask first if it has an origin.

@wdavidw
Copy link
Member

wdavidw commented Feb 22, 2024

I don't think so, or I don't have a memory for it. Please propose your change if you feel confident. I personally don't use TS.

@wdavidw
Copy link
Member

wdavidw commented Feb 22, 2024

Also, add a unit test to support your claim.

@aronmal
Copy link
Contributor Author

aronmal commented Feb 22, 2024

Also, add a unit test to support your claim.

I've addressed the comment about adding a unit test. While there isn't a specific test to showcase the issue due to the nature of the problem (lack of strictness in types allowing more keys than intended), I did enhance the existing unit tests because during this process, a type error in the test file surfaced. But it has been taken care of.

If you have any further suggestions, let me know!

wdavidw pushed a commit that referenced this issue Feb 23, 2024
…Status (#49)

The TypeScript type inference for numeric keys in the HttpStatus interface was
previously unspecific, causing issues in strict typing scenarios. This fix
removes unnecessary declarations, ensuring more accurate and strict TypeScript
typing for numeric keys. It also includes updates to unit tests to reflect and
validate the improved strict typing.

Closes #48
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