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

Seemingly incomplete GraphQL documentation #614

Closed
Edo78 opened this issue Jan 17, 2020 · 5 comments
Closed

Seemingly incomplete GraphQL documentation #614

Edo78 opened this issue Jan 17, 2020 · 5 comments

Comments

@Edo78
Copy link
Contributor

Edo78 commented Jan 17, 2020

I just tryed to play with graphql so I created a new foalts app and followed the instruction on https://foalts.gitbook.io/docs/topic-guides/api/graphql

Sadly I got some errors

❯ npm run develop                                                                                                                                                                                                        ─╯

> foal-graphql@0.0.0 develop /home/edo/test/foal-graphql
> npm run build:app && concurrently "npm run build:app:w" "npm run start:w"


> foal-graphql@0.0.0 build:app /home/edo/test/foal-graphql
> copy-cli "src/**/*.html" build && tsc -p tsconfig.app.json

node_modules/graphql/subscription/subscribe.d.ts:44:3 - error TS2304: Cannot find name 'AsyncIterableIterator'.

44   AsyncIterableIterator<ExecutionResult<TData>> | ExecutionResult<TData>
     ~~~~~~~~~~~~~~~~~~~~~

node_modules/graphql/subscription/subscribe.d.ts:57:3 - error TS2304: Cannot find name 'AsyncIterableIterator'.

57   AsyncIterableIterator<ExecutionResult<TData>> | ExecutionResult<TData>
     ~~~~~~~~~~~~~~~~~~~~~

node_modules/graphql/subscription/subscribe.d.ts:86:12 - error TS2304: Cannot find name 'AsyncIterable'.

86 ): Promise<AsyncIterable<any> | ExecutionResult<TData>>;
              ~~~~~~~~~~~~~


Found 3 errors.

This happens because tsconfig.json lacks the lib esnext.asynciterable (adding it solves those problems).
I double checked to be sure but I can't find any reference in the doc.

If needed I can create a PR to update the doc. Let me know

@LoicPoullain
Copy link
Member

LoicPoullain commented Jan 22, 2020

Hi @Edo78

Thank you for pointing this out.

It looks like new versions of the graphql package need that setting. This is unfortunate because this appeared between two minor versions (14.3.0 works but 14.5.8 does not).

Yes, feel free to open a PR. I think the best place to add this is just after the npm install command in https://github.com/FoalTS/foal/blob/master/docs/api-section/graphql.md. Maybe something like this:

tsconfig.json

{
  "compilerOptions": {
    ...
    "lib": [
      ...
      "ESNext.AsyncIterable"
    ]
  }
  ...
}

Thanks!

@Edo78
Copy link
Contributor Author

Edo78 commented Jan 23, 2020

I also noticed that the actual graphql docs doesn't talk about typeorm & type-graphql integration (unless I missed it).
In my opinion it should have a prominent role and I start to write some code experimenting on them.
If you will I can add something about this argument in this very same PR (or in another one to keep a better separation).

@LoicPoullain
Copy link
Member

LoicPoullain commented Jan 23, 2020

@Edo78 the documentation actually mentions how to integrate TypeGraphQL the GraphQLController: https://foalts.gitbook.io/docs/topic-guides/api/graphql#using-typegraphql .

I would prefer not to add more documentation on TypeGraphQL and TypeORM because they both have their own docs with many examples and I think this would be a bit out of the scope of the framework documentation.

@Edo78
Copy link
Contributor Author

Edo78 commented Jan 24, 2020

Ok,I may have been to vague. Let me rephrase it.
I see the section about type-graphql but IMO is missing the point for a meaningful integration with the framework itself.

You wrote section about having a graphql schema completely separated from the db schema but this implies that who want to have the same schema for both must replicate it. Using type-graphql to annotate the typeorm schema can simplify this procedure.
For example the todo entity from the example can be written as

import { Column, Entity, PrimaryGeneratedColumn } from 'typeorm';
import { Field, ObjectType } from 'type-graphql';

@Entity()
@ObjectType()
export class Todo {

  @PrimaryGeneratedColumn()
  @Field()
  id: number;

  @Column()
  @Field()
  text: string;
}

I may be wrong but having a graphql schema resembling the db ones is a common practice and so it should have be at least an example. Just to provide users a way to understand how this technology could be integrated in the framework.

Obviously this is just my 2 cents.

This was referenced Jan 28, 2020
@LoicPoullain
Copy link
Member

Resolved with #623 and #624

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

No branches or pull requests

2 participants