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

FieldProps not accepting an interface as generic #9102

Closed
paulo9mv opened this issue Jul 14, 2023 · 7 comments · Fixed by #9092
Closed

FieldProps not accepting an interface as generic #9102

paulo9mv opened this issue Jul 14, 2023 · 7 comments · Fixed by #9092

Comments

@paulo9mv
Copy link
Contributor

What you were expecting:
On version 4.10.1, before updating to v4.12.1, the following code was working without errors:

import { FieldProps } from "react-admin"

interface A {
   title: string
}

const Foo = (props: FieldProps<A>) => {}

What happened instead:
But then the following error started to appear:

import { FieldProps } from "react-admin"

interface A {
   title: string
}

const Foo = (props: FieldProps<A>) => {} // Type 'A' does not satisfy the constraint 'Record<string, unknown>'.
                                         // Index signature for type 'string' is missing in type 'A'.

This error doesn´t show if we define A as a type or extends it from RaRecord or Record<string, any>.

Related code:

https://codesandbox.io/s/beautiful-wescoff-rkt57q?file=/src/App.tsx

Other information:
I imagine that this started because of this change: https://marmelab.com/blog/2023/06/13/react-admin-june-2023-update.html#generic-field-components about generic field components.
But was this described scenario expected? Should we change all interfaces to types or make them extend from Record?

Environment

@Gabriel-Malenowitch
Copy link
Contributor

Gabriel-Malenowitch commented Jul 14, 2023

Would it be an option to extend the field record generic with an empty object instead of Record<string, unknown>?

image

@paulo9mv
Copy link
Contributor Author

For now we are using an adapter, but could this be implemented in react-admin side?

type FieldPropsAdapter<T> = Pick<T, keyof T>


import { FieldProps } from "react-admin"

type FieldPropsAdapter<T> = Pick<T, keyof T>

interface A {
   title: string
}

const Foo = (props: FieldProps<A>) => {} // Type 'A' does not satisfy the constraint 'Record<string, unknown>'.
                                         // Index signature for type 'string' is missing in type 'A'.

const FooB = (props: FieldProps<FieldPropsAdapter<A>>) => {} // success

@slax57
Copy link
Contributor

slax57 commented Jul 17, 2023

Thanks for reporting this.
Indeed this seems like a bug to me.
Good news: the fix is already on the way! https://github.com/marmelab/react-admin/pull/9092/files#diff-a86be7071e1dd7245c41caa795bfdd2c6759e44dea7f790313c6b84f4f65424e

@fzaninotto
Copy link
Member

This bug challenges my comprehension of TypeScript generics and excess property checking. I've tried and tweaked an example provided by @slax57 in the TypeScript playground, but it only brings me perplexity.

https://www.typescriptlang.org/play?#code/C4TwDgpgBAsiBKEDGB7ATgEwCrmgXigG8AoKMqASwwC4oA7AVwFsAjCNAblPOAuABsItAM7A0FOgHMuAX2LEJwdgDMAhkmhxEqTAEk6StGo1FuZKrUat2XclF4ChUUeKmz5qOqKhpVAMQY6JF4UOigCMygAHm10bFwoCAAPJToMYShYzCiXCUkAGihAgGs6FAB3OgA+cMzkOJyxPMLVOhAqmsiACjR6zFos+MgASnCa3p0MLmJPbxYUFGKcSChaLT6h-FM7CygARgAmAGZ8yIdBWgAiAHVVNChWjCgABQh1CEviOVmUQQA6fgoSQ9fyBYIUUJdeaLZYQYbDLg-f6A4G+AJBEJ0KLrSawqpQhZLXDw6azYBQaHFfSGYzQNYIDbUlTvWokHY0fbHU52c5OG53B5pF5vDSfb6hYS-CAAoEg9HgyGUplGd4kmYSqUy1GgjEQrE4uLK2n4pUGZkaNXEIA

Now I've tested your proposed solution of using {} instead of Record<any, unknown>, but it seems to me that this allows passing any argument, not just object literals, so I don't understand why we even need {} and not any. Would you mind explaining the rationale? And why does it fix the bug?

As for the Pick trick, I don't understand it either.

@Gabriel-Malenowitch
Copy link
Contributor

According to my tests and research, the interface has the role of typing something with the amount of keys already predefined. While type is a more generic type. Another interesting thing is that if you use Record<string, any> the typescript will support interfaces, but when using Record<string, unknown> it does not.

If you extend interface: interface N extends Record<string, unknown>, it will work on RA fields (<T extends Record<string, unknown>(...)=>... ), since it now extended to an object with indeterminate number of keys (a adaptive generic object with indeterminate keys like type). My conclusion was that type can extend to Record<string, unknown> simply because it is a type (a adaptive generic type, just as Record<string, unknown> is).

Those were my conclusions based on testing and language study and research. I would like to know your views on the subject. CC: I think this comment might clear our minds.


I don't understand why we even need {} and not any. Would you mind explaining the rationale?

Indeed {} is an extremely generic type (More infos: Stackoverflow by Comment). From my understanding of the comment in the typescript repository: We don't have a good way to guarantee that an object is an object and still encompass interfaces. Perhaps a better alternative of extends {} is to extend it to { [key: string]: any } or Record<string, any> so that we at least exclude the primitive types. More alternatives can be studied.
image

@paulo9mv
Copy link
Contributor Author

For what I understood, mainly by reading this long issue discussion in TS repo, there's a difference on how index signatures works in interfaces vs types.

Typescript doesn't support implicit index signatures on interfaces. It only supports implicit index signatures on types. For interfaces, you need to explicit declare the index signature, like this:

interface A {
   [key: string]: string
   title: string
}

But this also would make the below expression acceptable, even if we haven't declared id.

const a: A = { title: "Test", id: "1" } // success

Since FieldProps new typing added a RecordType extends Record<string, unknown> = Record<string, any>, Typescript needs to check if RecordType index signature is a string, but it cannot implicitly do that on interfaces and then we got the error.


The Pick trick basically cast an interface to a type, allowing it to have implicit index signature. I'm trying to find a way to achieve the same result without using it, but no luck so far

@fzaninotto
Copy link
Member

I don't feel copmfortable about this change. I'll wait for the rest of the core team to come back from holidays so we can make a good decision (probably end of August).

In the meantime, you can use the quick fix consisting of using type instead of interface.

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

Successfully merging a pull request may close this issue.

4 participants