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

Tg feature resolvers types #33

Merged

Conversation

theGlenn
Copy link
Contributor

@theGlenn theGlenn commented May 22, 2018

Hello!

As mentioned in #22 : createResolver in resolver.d.ts indicates that the return type is a Promise<any>.

export declare const createResolver: (resFn: any, errFn: any) => (root: any, args?: {}, context?: {}, info?: {}) => Promise<any>;

Seems like tsc type inference doesn't recognize the objects structure in resolvers.ts

Proposed implementation: Adding concrete types definition

export interface ResultFunction<ResulType> {
  (root, args, context, info): Promise<ResulType> | ResulType | void
}

export interface ErrorFunction<ErrorType> {
  (root, args, context, err): ErrorType | void
}

export interface CreateResolverFunction {
  <R, E>(resFn: ResultFunction<R>, errFn?: ErrorFunction<E>): Resolver<R>
}

export interface Resolver<ResulType> {
  (root, args: {}, context: {}, info: {}): Promise<ResulType>
  createResolver?: CreateResolverFunction
}

Copy link
Owner

@thebigredgeek thebigredgeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remove dist please?

@theGlenn
Copy link
Contributor Author

theGlenn commented Jun 6, 2018

@thebigredgeek done in d137a18

@corydeppen
Copy link

Looks like dist should also be added to .gitignore.

@thebigredgeek
Copy link
Owner

thebigredgeek commented Jun 6, 2018

@corydeppen nope not exactly. We do want dist in github, but we want it to be last stable release for now.

EDIT: Actually, you are correct haha. dist has never been pushed. I assumed I hadn't added dist to the git ignore because I wanted to ensure installs against the github repo worked correctly, but that's not the case!

@thebigredgeek thebigredgeek merged commit f0a1267 into thebigredgeek:master Jun 6, 2018
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

Successfully merging this pull request may close these issues.

3 participants