ResolverImplementer has a break change that the implementor can't know a field status.

[tsingsun]

What happened?

I implemented the plugin.ResolverImplementer to generate some CRUD method, but that breaks when bumping from v0.17.38 to v.0.17.41.
Those implemented and edited code will be overridden by the original template initialization code.

I found #2850:

in v0.17.38 /plugin/resolvergen/resolver.go

                          if implementation == "" {
				// Check for Implementer Plugin
				var resolver_implementer plugin.ResolverImplementer
				var exists bool
				for _, p := range data.Plugins {
					if p_cast, ok := p.(plugin.ResolverImplementer); ok {
						resolver_implementer = p_cast
						exists = true
						break
					}
				}

				if exists {
					implementation = resolver_implementer.Implement(f)
				} else {
					implementation = fmt.Sprintf("panic(fmt.Errorf(\"not implemented: %v - %v\"))", f.GoFieldName, f.Name)
				}

			}

in v0.17.41 /plugin/resolvergen/resolver.go

diff:

• 0.17.38 if the implementation is empty, it is passed to the implementer.
• 0.17.41 the Field is passed to the ResolverImplementer regardless of whether it is already implemented

What did you expect?

Let the ResolverImplementer can know the field status, whether the field needs to be implemented, field is new or has been resolved, if not , can you keep the v0.17.38 behavior the way it was

Minimal graphql.schema and models to reproduce

versions

• go run github.com/99designs/gqlgen version 0.17.41
• go version 1.21

[StevenACoffman]

@roneli I'm not quite clear on this problem. Can you please take a look?

[roneli]

Hi,

I am implemented #2850, I didn't break the original interface as I didn't want it to cause issues with backwards compatability. Basically, I concluded that not passing if it exists doesn't allow the implementer to decide / modify in future runs.

@StevenACoffman I can modify the interface to also include the prevDecl (will be empty if it was never declared etc') so the resolver implementor can decide if to change it or not. Question is are we okay with breaking the interface? I can make a new one that also accepts previous and switch case based on the provided implementation to keep backwards compatibility.

I'll dive deeper into the code and upload a proposed interface tomorrow.

[roneli]

Hi, @tsingsun

I created a PR that gives the resolver implementor the previous method body, it will be empty if no previous body was created before.

It does break the interface, but I would assume its a minor inconvenience for the anyone who uses this interface to update.

@StevenACoffman would love for any comments on the interface change

[tsingsun]

@roneli @StevenACoffman Sorry for the late reply. I added a rewriter to solve the interface changed in 17.41. Now I have updated to 17.45, the code generation is the same as before, thank you for your work.

[StevenACoffman]

@tsingsun Can you comment on #2886 ?

[tsingsun]

done