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 namespaces to be considered? #5633

Closed
danielkcz opened this issue Oct 30, 2018 · 12 comments
Closed

Typescript namespaces to be considered? #5633

danielkcz opened this issue Oct 30, 2018 · 12 comments
Milestone

Comments

@danielkcz
Copy link

danielkcz commented Oct 30, 2018

After some initial bickering that Typescript support does not include this and that I've decided to give it a try. Unfortunately, I've stumbled upon a big obstacle.

I am using graphql-code-generator package to generate strong types for my GraphQL queries. In the essence, it generates types like this.

export namespace GLogoutWidget {
  export type Variables = {
    id: string
  }

  export type Query = {
    __typename?: 'Query'
    user: User
  }

  export type User = {
    __typename?: 'User'
    id: string
    firstName?: string | null
    lastName?: string | null
  }
}

Usage of namespaces in this is extremely helpful as it clearly separates types for each query/mutation. Compared to apollo-codegen which outputs extremely long and annoying names for types, this is a clear win.

I would like to know if namespace support is such a huge technical problem that it cannot be included. I mean when bundling the app it should be mostly about stripping type information and let the TypeScript worry about type checking. I am kinda lost why such a decision has been made to have a separate TypeScript parser that's limited like this.

/cc @Timer

@Timer
Copy link
Contributor

Timer commented Oct 30, 2018

You should be able to define these in a .d.ts file, correct? They just can't live in a .ts(x)? file.

Can you co-locate these types next to the source file in a .d.ts version?

@Timer Timer added this to the 2.1.1 milestone Oct 30, 2018
@danielkcz
Copy link
Author

Well, I don't want to use .d.ts because I want to explicitly import these types when I am using them. There is too many of those and having them global would pollute autocomplete and it would be rather annoying.

@yordis
Copy link

yordis commented Oct 30, 2018

namespace are not supported on @babel/preset-typescript so if we try to support this we need to use ts-loader.

@Timer
Copy link
Contributor

Timer commented Oct 30, 2018

Seems like the best path forward is to have graphql-code-generator generate modern type definitions (based on ES Modules).
Can we file an issue with them?

@danielkcz
Copy link
Author

danielkcz commented Oct 30, 2018

That module supports custom generators I can write myself. However, I am not sure how it should look like. What do you mean by "modern type definitions"?

@danielkcz
Copy link
Author

danielkcz commented Oct 30, 2018

If I understand correctly what you mean, the result should look like this?

  export type GLogoutWidgetVariables = {
    id: string
  }

  export type GLogoutWidgetQuery = {
    __typename?: 'Query'
    user: User
  }

  export type GLogoutWidgetUser = {
    __typename?: 'User'
    id: string
    firstName?: string | null
    lastName?: string | null
  }

If that's what you mean then I don't like it at all because that's the same thing that apollo-codegen is doing. A major deal breaker is that I can have namespace that contains like 10 different types. If I want to reference all of them, it means I have to import for each one of them separately. Compare to when I can just import "GLogoutWidget" and have everything neatly packaged.

@Timer
Copy link
Contributor

Timer commented Oct 30, 2018

Why couldn't you just import * as GLogoutWidget from './ThatFile' and get them all packaged?

@danielkcz
Copy link
Author

danielkcz commented Oct 30, 2018

That does not make much sense. These types are generated from all queries and put into single types.ts file and thanks to namespaces I can import only namespaces I want.

Ok, nevermind I guess. It's not worth spending time on this. I found related babel issue and it's apparent that namespace will never be part of that plugin because they are legacy .

Oh well. I am so glad that react-script-ts are going to be maintained, otherwise the future would be rather dark.

@brunolemos
Copy link
Contributor

brunolemos commented Oct 30, 2018

@FredyC Try using export declare namespace instead of export namespace:

-export namespace GLogoutWidget {
+export declare namespace GLogoutWidget {

Let me know if that works for you.

@Timer
Copy link
Contributor

Timer commented Oct 31, 2018

That does not make much sense. These types are generated from all queries and put into single types.ts file and thanks to namespaces I can import only namespaces I want.

Ok, nevermind I guess. It's not worth spending time on this. I found related babel issue and it's apparent that namespace will never be part of that plugin because they are legacy .

I'm sorry and understand that this is likely frustrating, but namespaces are a proprietary, legacy feature. They have been replaced by specification behavior: ES Modules.

If this isn't satisfactory, you are free to continue using this feature until it is deprecated in TypeScript.

You can try what @brunolemos recommended, but aside from that, we'll not be taking any action at present time.

We've provided you with two solutions that work with our implementation, so I'm closing this as resolved.

@Timer Timer closed this as completed Oct 31, 2018
@arackaf
Copy link

arackaf commented Oct 31, 2018

namespace are not supported on @babel/preset-typescript

Seems like that should just about settle this issue. Either convince the Babel folks to support this, or shoot me a PR over at customize-cra adding the ts-loader, and removing the current Babel solution.

@danielkcz
Copy link
Author

@Timer As it was said a linked babel issue, I wasn't able to find any information about why are namespaces legacy. Later in that discussion, it's revealed that it's more of a technical issue than a problem of being deprecated. I can live what that, but let's not spread the false accusations here :)

@brunolemos Thanks for the tip, but I've already switched back to TS fork and don't want to spend time on this anymore. It's kinda a shame. In a sense, I was looking into being able to tap into Babel world while using TypeScript without extra hassle, but this is too much blocker for me.

@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants