Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

Support type exports too #282

Open
koenpunt opened this issue Nov 11, 2018 · 7 comments · May be fixed by #300
Open

Support type exports too #282

koenpunt opened this issue Nov 11, 2018 · 7 comments · May be fixed by #300

Comments

@koenpunt
Copy link
Contributor

koenpunt commented Nov 11, 2018

Currently only typescript types defined as export interface MyType {} are used by graphqlgen, but export type MyType = SomeOtherType should work too.

@ksaldana1
Copy link

ksaldana1 commented Jan 16, 2019

Is there any good workaround for this time being? This tool is great and I'd love to integrate it into our workflow at work, but there's a few conflicts with the yaml parsing / TS support thats blocking it.

We generate TS types from protos, but they are pretty heavily namespaced.

So I'd love to be able to do (doesn't seem supported):
override: User: ./node_modules/@my_company/user-protos/protos.ts:my_company.user.User

My next approach was type aliases (which is how I ended up here).. In models.ts:

import { my_company } '@my_company/user-protos';

export type User = my_company.user.User

I settled on using interface extension, but this has it's own problems with scaffolding default resolvers.

import { my_company } '@my_company/user-protos';

export interface User extends my_company.user.User {}

This works as far as typesafe-resolvers, the only negative is that default resolvers are not implemented. My default user resolvers are an empty object--if I inline the User properties this is not the case.

Would be happy to look into type alias support and submit a PR, just wondering if the current ticket statuses are still correct. Thanks again for all the work on this!

@jasonkuhrt
Copy link
Member

@ksaldana1 yep the labels on this issue are still correct!

@jasonkuhrt jasonkuhrt pinned this issue Jan 17, 2019
@ksaldana1
Copy link

ksaldana1 commented Jan 17, 2019

I think there's a few things going on here that are somewhat separate problems. Thanks @blakeembrey for sending me down the right direction and providing PRs that address at least some of these problems.

Here's a simple of example of how behavior works today (currently on master):

# schema.graphql
type Query {
  author(id: ID!): Author
  books: [Book!]
}

type Author {
  id: ID!
  firstName: String!
  lastName: String!
  emailAddress: String!
  books: [Book!]!
}

type Book {
  id: ID!
  title: String!
  author: Author!
}
// models.ts
export type Author = {
  id: string
  fullName: string
  bookIds: string[]
}

export type Book = {
  id: string
  title: string
  authorId: string
}

Running graphqlgen with these files gives you proper code generation--default resolvers are implemented as expected and all types are imported. So today basic inline type support is working as an interchangeable alternative to interface.

If we change models.ts

// models.ts
export interface IAuthor {
  id: string
  fullName: string
  bookIds: string[]
}

export type Author = IAuthor

export interface IBook {
  id: string
  title: string
  authorId: string
}

export type Book = IBook

Still all in one file. If you run graphqlgen on this file you will get invalid TypeScript generation--the imports for Book and Author are missing. This is happening with some code addressed in #300. If you remove the filtering in the renderImports code, this issue is resolved and inline type aliases work as expected.

There is still one more issue though. Let's say my interface definitions live in another file (such as an npm package).

// models.ts
import { IAuthor, IBook } from './otherModels'

export type Author = IAuthor
export type Book = IBook

Here the issue with the generation is more of a slight inconvenience than the one above--your default resolvers are empty. Author and Book do not get their fields resolved from the other file / package, so you are left to implement all resolvers yourself. This might be a limitation of using babel-parser, but I am not familiar enough with its TS tooling to really say.

Those were my findings looking into this today. Let me know if anything seems wrong or off-base.

@jasonkuhrt
Copy link
Member

Thanks @ksaldana1 this is really helpful.

@ksaldana1
Copy link

ksaldana1 commented Jan 25, 2019

My desire to use graphqlgen lead me down the path of getting around the current restraints around type aliases and imports. Going the parser route to deal with TypeScript import types seemed like a route to frustration and pain (personal experience), so I looked a bit more into TS's compiler API, which has pretty little documentation.

With a mixture of ast-explorer, various blogs, and my vs-code auto-complete, I was able to get something to serve as a sufficient workaround for the time being.

https://github.com/ksaldana1/ts-alias-transformer

Not in a great state as far as UX / usage as a CLI tool (as I write this), but the basic idea is to transform type aliases / nested types into their primitive structures. The idea is to avoid dependencies / be agnostic to references and imports and focus specifically on the structure of the data when feeding into graphqlgen.

It sucks to add another build step, and I'm sure there's a few rough edges I need to tweak, but it's a pretty short amount of code, so I'm hoping it won't blow up too much.

It is probably in this library's best interest to investigate incorporating the TS compiler tooling to get around the limitations of babel-parser (and using a parser approach in general). Felt a lot better to work with the compiler checker's symbol table to resolve any of the weird scenarios that could come up.

Just my two cents and I'm happy to do some workarounds to leverage the great work you all have done.

Edit:
I have published the transformer as a really minimal CLI package if anyone wants to try it out in their workflow.

@jasonkuhrt
Copy link
Member

It is probably in this library's best interest to investigate incorporating the TS compiler tooling to get around the limitations of babel-parser (and using a parser approach in general). Felt a lot better to work with the compiler checker's symbol table to resolve any of the weird scenarios that could come up.

The gist of this sounds great to me, and I'm personally excited to get more hands-on experience with this kind of approach.

@ksaldana1
Copy link

@jasonkuhrt I have written a follow-up blog post that goes into the compiler API a bit. I hope anyone interested finds it helpful.

@jasonkuhrt jasonkuhrt unpinned this issue Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants