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

Interface-handling regression in 5.4.7? #138

Open
jstasiak opened this issue Feb 9, 2024 · 4 comments
Open

Interface-handling regression in 5.4.7? #138

jstasiak opened this issue Feb 9, 2024 · 4 comments

Comments

@jstasiak
Copy link

jstasiak commented Feb 9, 2024

Hey, first of all thanks for developing this library.

We bumped into an interesting TS compilation problem when trying to upgrade from 5.4.6 to 6.0.0, I narrowed it down to version 5.4.7 and this commit: 3ab58ee.

I'm not entirely sure if we're doing something wrong or is there an actual regression.

A piece of code to reproduce the problem:

interface Camel {
    userName: string
}

function f1(c: Camel): {user_name: string} {
    const snake = snakecaseKeys(c)
    return snake
}

function f2(c: Camel[]): {user_name: string}[] {
    const snake = snakecaseKeys(c, { deep: true })
    return snake
}

With snakecase-keys 5.4.6 it compiles as expected.

With 5.4.7 it fails to compile:

file.ts:1496:33 - error TS2345: Argument of type 'Camel' is not assignable to parameter of type 'readonly any[] | Record<string, unknown>'.
  Type 'Camel' is not assignable to type 'Record<string, unknown>'.
    Index signature for type 'string' is missing in type 'Camel'.

1496     const snake = snakecaseKeys(c)
                                     ~

file.ts:1497:5 - error TS2322: Type 'readonly any[] | { [x: string]: unknown; }' is not assignable to type '{ user_name: string; }'.
  Property 'user_name' is missing in type 'readonly any[]' but required in type '{ user_name: string; }'.

1497     return snake
         ~~~~~~

  src/apiModels/v1Adapters.ts:1495:25
    1495 function f1(c: Camel): {user_name: string} {
                                 ~~~~~~~~~
    'user_name' is declared here.

file.ts:1502:5 - error TS2322: Type 'Camel[]' is not assignable to type '{ user_name: string; }[]'.
  Property 'user_name' is missing in type 'Camel' but required in type '{ user_name: string; }'.

1502     return snake
         ~~~~~~

  src/apiModels/v1Adapters.ts:1500:27
    1500 function f2(c: Camel[]): {user_name: string}[] {
                                   ~~~~~~~~~
    'user_name' is declared here.

TS version: 5.3.2

Happy to provide more details if that helps.

@guansss
Copy link

guansss commented Feb 22, 2024

We are getting this problem too.

microsoft/TypeScript#50087 (comment) likely explains this. Maybe the Record<string, any> shouldn't be changed to Record<string, unknown>?

@yutak23
Copy link
Contributor

yutak23 commented Feb 22, 2024

The following problems recur when using any.
#116

I think the essence of the error lies in the index signature part.
sindresorhus/camelcase-keys#114 (comment)

The following methods may help.
sindresorhus/camelcase-keys#114 (comment)

@guansss
Copy link

guansss commented Feb 22, 2024

Hi @yutak23 , thanks for the solution, it works for my case!

However, after diving deep into this issue, I think the fix for #116 in #117 is inaccurate because the use of Record<string, unknown> is just skipping the case conversion for anything that's not assignable to it, which includes interfaces and classes.

As a result, it breaks nested classes:

class Foo {
  someVar = 1
  someMethod() {}
}

const a = snakecaseKeys({ foo: new Foo() })
// the type is { foo: Foo }
// the value is { foo: { some_var: 1 } }

As well as nested interfaces:

interface Bar {
  someVar: number
}

const bar: Bar = { someVar: 1 }
const b = snakecaseKeys({ bar })
// the type is { bar: Bar }
// the value is { bar: { some_var: 1 } }

So I still think Record<string, any> should be used to ensure type correctness.

As for removing functions from the object (a fix for #116), I believe a simple condition will do the trick:

type WithoutFunctions<T extends Record<string, any>> = {
  [K in keyof T as T[K] extends Function ? never : K]: T[K]
}

type _Date = WithoutFunctions<Date>
// {}

type _Error = WithoutFunctions<Error>
// { name: string; message: string; stack?: string | undefined; }

@yutak23
Copy link
Contributor

yutak23 commented Feb 23, 2024

I believe that using this library to change the case of object other than plain object, such as instances of a class or function, is not the intended use of the library.

Although the readme simply mentions object, I think it specifically refers to plain object. This can be understood by looking at the readme of camelcase-keys, a similar library to snakecase-keys.

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

No branches or pull requests

3 participants