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

support intermediate interface #180

Closed
wants to merge 1 commit into from

Conversation

Code-Hex
Copy link
Contributor

#169 is inactive. I created a new one

required also: #178

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 92.396% when pulling daaed17 on Code-Hex:intermediate-interfaces into 2776c98 on vektah:master.

@Code-Hex
Copy link
Contributor Author

@StevenACoffman Could you handle this PR?
I will create a new PR to support this feature for gqlgen after merged and released this one

@StevenACoffman
Copy link
Collaborator

Have you already figured out what is necessary to change in gqlgen to support this? We had to reverta PR here recently because it broke gqlgen and I'd like to avoid repeating that.

What is different in your change here from the changes in #169 : https://github.com/vektah/gqlparser/pull/169/files

@wilhelmeek FYI

@Code-Hex
Copy link
Contributor Author

Code-Hex commented Feb 21, 2022

@StevenACoffman I understand the case (maybe this one?). I believe the broken is caused by updated introspection.
this change is not included updating introspection. so this works fine

What is different in your change here from the changes in

These are the same PR. but another one is draft. If #169 is open, I'm OK to merge it and close this one (but I want this change as soon as possible 🙏 )

@Code-Hex
Copy link
Contributor Author

Code-Hex commented Feb 21, 2022

In addition, If I were to mention this issue, We would have to modify the InputFields() to ignore deprecated directives for code generation as in this code

https://github.com/99designs/gqlgen/blob/5ad012e3d7be1127706b9c8a3da0378df3a98ec1/graphql/introspection/type.go#L73-L75

- func (t *Type) InputFields() []InputValue {
+ func (t *Type) InputFields(includeDeprecated bool) []InputValue {
	if t.def == nil || t.def.Kind != ast.InputObject {
		return []InputValue{}
	}

	res := []InputValue{}
	for _, f := range t.def.Fields {
+		if !includeDeprecated && f.Directives.ForName("deprecated") != nil {
+			continue
+		}
		res = append(res, InputValue{
			Name:         f.Name,
			Description:  f.Description,
			Type:         WrapTypeFromType(t.schema, f.Type),
			DefaultValue: defaultValue(f.DefaultValue),
		})
	}
	return res
}

@StevenACoffman
Copy link
Collaborator

@Code-Hex I rebased and merged #169 since it preserves the original author for attribution. However, when I then rebased #178 the tests continued to fail. I wasn't able to push the rebased version back up to @vvakame fork, so I created #181 to demonstrate the continued failure.

Unfortunately, I'm not going to be able to spend more time looking into this at present, so if anyone else can sort it out, I would welcome a PR that passed the tests.

@Code-Hex
Copy link
Contributor Author

@StevenACoffman Thank you for merged!

Sure. Because this is necessary a part of process.
If I can take to solve the problem I will send PR.

However, when I then rebased #178 the tests continued to fail

Thank you

@Code-Hex Code-Hex closed this Feb 22, 2022
@Code-Hex Code-Hex deleted the intermediate-interfaces branch February 22, 2022 00:29
@Code-Hex
Copy link
Contributor Author

@StevenACoffman Could you release for merged one?

You said

We had to reverta PR here recently because it broke gqlgen and I'd like to avoid repeating that.

If so, We have to do "small change, small release". Thanks.

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