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

Types are not generated for introspection query #643

Closed
danielkcz opened this issue Sep 24, 2018 · 28 comments
Closed

Types are not generated for introspection query #643

danielkcz opened this issue Sep 24, 2018 · 28 comments
Assignees
Labels
core Related to codegen core/cli plugins waiting-for-release Fixed/resolved, and waiting for the next stable release

Comments

@danielkcz
Copy link
Contributor

danielkcz commented Sep 24, 2018

Describe the bug
I have a query like this which reads introspection in runtime to figure user permissions. Unfortunately, the generated types do not include the GUserPermissions type at all. But if I add something else to that query, it appears there, but without the introspection part.

export const UserPermissionsQuery = gql`
  query GUserPermissions {
    __schema {
      queryType { fields { name } }
      mutationType { fields { name } }
    }
  }
`

To Reproduce
I am using API approach and call it like this. Full introspection is generated in JSON file. I've checked and it includes types for __schema, so that should be a problem.

  generate({
    schema: jsonPath,
    out: './src/graph/types.ts',
    template: 'graphql-codegen-typescript-template',
    args: ['./src/**/*graphql.ts'],
    skipSchema: true,
    overwrite: true,
  })

Expected behavior
To have TypeScript types based on introspection query.

Environment:

  • OS: Windows 10 x64
  • Codegen: 0.9.2
  • Node: 10.11.0
@dotansimha
Copy link
Owner

Interesting. Thanks @FredyC !

I think we might ignore fields that starts with __. I'll check.

@dotansimha dotansimha added bug core Related to codegen core/cli labels Oct 2, 2018
@dotansimha
Copy link
Owner

@FredyC We are ignoring the internal fields of GraphQL because otherwise it will bloat the results (see https://github.com/dotansimha/graphql-code-generator/blob/master/packages/graphql-codegen-core/src/schema/schema-to-template-context.ts#L27) .

I can make this filtering an option, and then it will solve your issue.

@patricknazar
Copy link
Contributor

I've noticed that when I use simply a __typename in fragment, I get similar issues:

fragment MyFragment on MyType {
    ... on TypeOne {
        __typename
    }
}

gives me:

export namespace MyFragment {

    export type TypeOneInlineFragment =

}

Notice the trailing =. Will this resolve this problem too? It's not an introspection query really is it? It is an internal field, right?

@Jonatthu
Copy link

Jonatthu commented Feb 5, 2019

@dotansimha I am having the same issues on the latest version, I really need to do some internal types queries

@kamilkisiela
Copy link
Collaborator

I tried to fix it in #1380, could you guys check if that works in your projects? A canary release under 0.18.0-alpha.d365409c, make sure you bump all packages from codegen group.

@dotansimha dotansimha added the waiting-for-release Fixed/resolved, and waiting for the next stable release label Mar 1, 2019
@dotansimha
Copy link
Owner

Fixed in 0.18.0 🎉

@Jonatthu
Copy link

@dotansimha 0.19.0-alpha.e87c7ccf in this version the services are being generated like this:

@Injectable({
	providedIn: "root"
})
export class LoadEntityDetailsQueryGQL extends Apollo.Query<
	LoadEntityDetailsQueryQuery,
	LoadEntityDetailsQueryQueryVariables
> {

They should be like this:

@Injectable({
	providedIn: "root"
})
export class LoadEntityDetailsQueryGQL extends Apollo.Query<
	LoadEntityDetailsQuery.Query,
	LoadEntityDetailsQuery.Variables
> {

@dotansimha
Copy link
Owner

@Jonatthu the alpha version you are using is the latest refactor, and it should output without any namespaces at all.

@Jonatthu
Copy link

@dotansimha Well it is having typescript issues or bugs like this, this is giving me a compilation error

@danielkcz
Copy link
Contributor Author

danielkcz commented Mar 16, 2019

@Jonatthu You shouldn't be probably using alpha version if you look for stability ;) Either way, namespaces are going to be dropped, so you will either have to adapt or stick to a current version for a while.

Btw namespaces are legacy and being removed from TypeScript (sadly), so you will do yourself a service if you start adapting for non-namespaces environment. I really recommend you to have a look at https://graphql-code-generator.com/docs/plugins/typescript-react-apollo plugin if you use React. It will generate whole bunch of stuff for you and you don't need to bother with types that much.

@Jonatthu
Copy link

Jonatthu commented May 9, 2019

@FredyC I think namespaces was a really good solution to the individual types, I do not understand why the change...

@danielkcz
Copy link
Contributor Author

@Jonatthu Well, ask the Microsoft, they have deprecated namespaces from TypeScript. For the same reason, they are not supported by Babel TypeScript implementation, so it's a kinda wrong path to follow altogether.

@Jonatthu
Copy link

Jonatthu commented May 9, 2019

@FredyC So what's the workaround now?
I would like to pick sub types from a query to be able to create computed properties like this:

public get subProperty(): MyQuery.__Field
{
    return this.myQueryResult.Items[0].SomeType.SomeProperty;
}

@Jonatthu
Copy link

Jonatthu commented May 9, 2019

@FredyC microsoft/TypeScript#30994
Maybe they are not deprecated...

@danielkcz
Copy link
Contributor Author

Ok, that's certainly new information, but I don't really miss namespaces. I don't follow why do you need them for your use case. You can as well namespace with a custom delimiter of yours (or none), eg. MyQuery__Field.

@Jonatthu
Copy link

Jonatthu commented May 9, 2019

@FredyC

First now I am only able to define the type of my query but not of a selection of a section of the query

image

Second on hover the definition of my second property, this definition is really not dev friendly, specially if is really big type

image

And finally, if I wanna strong type this property my only option is to put the base type of this

image

and this tooltip is showing a good definition of my selection BUT this will not match if on my query I only selected one of the properties.

@danielkcz
Copy link
Contributor Author

Well, sorry, I don't have any viable workarounds here. Since I started using generators, I don't bother with types that much to be bothered by it.

@Jonatthu
Copy link

Jonatthu commented May 9, 2019

@FredyC, @dotansimha
Would be hard to create a namespace plugin for those that miss this feature?

@dotansimha
Copy link
Owner

@Jonatthu you can use https://graphql-code-generator.com/docs/plugins/typescript-compatibility

@Jonatthu
Copy link

Jonatthu commented May 9, 2019

@dotansimha Looks like introspection namespaces are not being created, is there a flag that we are missing on the docs?
graphql-codegen --include-introspection-types I tried this but did not work

@dotansimha
Copy link
Owner

@Jonatthu actually, the internal introspection types was not included in 0.18, so it's not an issue with backward compatibility.
We have a solution for it in >1.0, so at the moment you can only get the introspection types with that.

@Jonatthu
Copy link

@dotansimha What’s the solution? Is it something on development or something that I can use now?

@dotansimha
Copy link
Owner

@Jonatthu They are generated only if one of your documents asks for it.
See here: https://github.com/dotansimha/graphql-code-generator/blob/master/packages/plugins/typescript/typescript/src/index.ts#L112

So if your operations are querying for fields that are declared as internal introspection fields, it will generate those fields as part of your typescript plugin output.

@kamilkisiela implemented this a while ago.

@RyanCavanaugh
Copy link

RyanCavanaugh commented May 13, 2019

@FredyC

Btw namespaces are legacy and being removed from TypeScript (sadly)

This is not correct.

@Jonatthu
Copy link

Jonatthu commented May 13, 2019

@dotansimha @FredyC I know could be hard but based on this...

Could be bring back the namespace convention that this project used to have?
The plugin compatibility generates some models but still really hard to read by example:

export namespace LoadPropertyPermissions {
	export type Variables = LoadPropertyPermissionsQueryVariables;
	export type Query = LoadPropertyPermissionsQuery;
	export type GenPermission = LoadPropertyPermissionsQuery["genPermission"];
	export type Items = LoadPropertyPermissionsQuery["genPermission"]["items"][0];
	export type RolePermissions = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["rolePermissions"];
	export type _Items = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["rolePermissions"]["items"][0];
	export type Role = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["rolePermissions"]["items"][0]["role"];
	export type ClientPermissions = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["clientPermissions"];
	export type __Items = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["clientPermissions"]["items"][0];
	export type UserPermissions = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["userPermissions"];
	export type ___Items = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["userPermissions"]["items"][0];
	export type User = LoadPropertyPermissionsQuery["genPermission"]["items"][0]["userPermissions"]["items"][0]["user"];
}

Would be nice to keep the old behavior creating interfaces instead of grabbing parts of a type this way

export type UserPermissions = LoadPropertyPermissionsQuery["genPermission"]["items"][0]

Anyways this is an amazing project and it is doing a great job, but sometimes old features are still totally fine to keep. Specially if helps to devs to maintain and understand the code generated right away.

Maybe we can improve the compatibility plugin some how to generate this interfaces but aside of that, the query interface should use this models instead this:

export type LoadPropertyPermissionsQuery = {
	genPermission: Maybe<{
		items: Maybe<
			Array<
				Maybe<
					Pick<
						GenPermissionGraphType,
						"description" | "guid" | "id" | "key" | "value"
					> & {
						rolePermissions: Maybe<{
							items: Maybe<
								Array<
									Maybe<
										Pick<
											GenRolePermissionGraphType,
											"roleId" | "permissionId"
										> & {
											role: Maybe<
												Pick<
													RoleGraphType,
													| "id"
													| "name"
													| "description"
												>
											>;
										}
									>
								>
							>;
						}>;
						clientPermissions: Maybe<{
							items: Maybe<
								Array<
									Maybe<
										Pick<
											GenClientPermissionGraphType,
											"clientId"
										>
									>
								>
							>;
						}>;
						userPermissions: Maybe<{
							items: Maybe<
								Array<
									Maybe<{
										user: Maybe<
											Pick<
												UserGraphType,
												"username" | "email"
											>
										>;
									}>
								>
							>;
						}>;
					}
				>
			>
		>;
	}>;
};

This new way to use Maybe and Pick models and it's usage on this generated definition is no longer easy to read, would be nice to use the namespace and it's unique interface just like before.

@dotansimha
Copy link
Owner

@Jonatthu we choose this path because we would to be able to create base package and use it.
I know it's a bit harder to read, but in most cases it just fulfil the need for auto-complete from a query response.
We decided to remove the intermediate types because it caused a lot of issues with the generated output, and the typescript-compatibility is there only to make it easier to migrate.

I think, maybe, in the future, we can add a configuration for resolving the type and not using Pick<> and this way you'll have the primitive type directly there, instead of picking it. (but that depends on our time and resources)

@Jonatthu
Copy link

@dotansimha I think for the future would be a great investment, but thanks for all the current work, the cli is looking awesome and feeling professional.

@dotansimha
Copy link
Owner

dotansimha commented May 28, 2019

@Jonatthu I opened a new issue for that: #1925 , and regarding resolving the actual type: #1904 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Related to codegen core/cli plugins waiting-for-release Fixed/resolved, and waiting for the next stable release
Projects
None yet
Development

No branches or pull requests

6 participants