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

[Typescript resolvers] type vs interface and typesPrefix #1527

Closed
P4sca1 opened this issue Mar 20, 2019 · 5 comments
Closed

[Typescript resolvers] type vs interface and typesPrefix #1527

P4sca1 opened this issue Mar 20, 2019 · 5 comments
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@P4sca1
Copy link
Contributor

P4sca1 commented Mar 20, 2019

Is there any reason why we use export type instead of export interface?
Also, it is kinda confusing that some types / interfaces are prefixed with I while others are not. In my opinion, nothing should be prefixed by default and the prefix should only be added depending on the typesPrefix option.
Right now when doing typesPrefix: I, some types have 2 I prefixed.

@dotansimha
Copy link
Owner

dotansimha commented Mar 21, 2019

@P4sca1 which version do you use? I assume it's typesPrefix because it's there from v1.0.0.
Can you provide an example for types that are being prefixed by default? I think we only have it in IResolvers

Also, we chose to use type because it's easier to use it to create type aliases, and most of the types are not intended to be inherited because they reflect the schema, and interface has a connotation of something you should inherit/implement.
Also, TypeScript interfaces can be declared multiple times in the same scope, making it harder for us to test if there are duplicated types generated.

interface A {
  a1: string;
}
interface A { // No error, and "A" will have both fields
  a2: string;
}

While types can be declared only once:

type A = {
 a1: string;
}
type A = { // Error
 a2: string;
}

@P4sca1
Copy link
Contributor Author

P4sca1 commented Mar 21, 2019

Okay that makes sense now.
For the prefix, it was only for some interfaces, maybe only IResolvers (I don’t remember atm, need to check).
Maybe we should change typesPrefix to only prefix types not interfaces then (because they are prefixed by default)?

@dotansimha
Copy link
Owner

@P4sca1 I think it's only IResolvers, @ardatan maybe we should remove it? I think you added it.

@dotansimha
Copy link
Owner

@P4sca1 I fixed it here:
#1534

I'm generating Resolvers as the root, and IResolvers pointing to Resolvers with @deprecated comment (for backward compatibility), and if you'll provide typesPrefix, it will override it.
(Also, did the same of IDirectiveResolvers)

@dotansimha dotansimha added enhancement plugins waiting-for-release Fixed/resolved, and waiting for the next stable release and removed question labels Mar 21, 2019
@P4sca1
Copy link
Contributor Author

P4sca1 commented Mar 21, 2019

LGTM!

@P4sca1 P4sca1 closed this as completed Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugins waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
Development

No branches or pull requests

2 participants