Skip to content

Commit

Permalink
Reload config packages after generating models
Browse files Browse the repository at this point in the history
If models are generated in a package that has already been loaded, and
that package refers to another package that has already been loaded, we
can find ourselves in a position where it appears that a GQL `union` is
not satisfied.

For example, if we have:

```
union Subject = User
```

with this gqlgen.yml in github.com/wendorf/gqlgen-error/gql:

```
schema:
- schema.graphql
exec:
  filename: generated.go
model:

  filename: models_gen.go
models:
  User:
    model: github.com/wendorf/gqlgen-error/gql.User
  Subject:
    model: github.com/wendorf/gqlgen-error/models.Subject
```

Note that our User model is in the github.com/wendorf/gqlgen-error.gql
package, and our models_gen.go will be generated in that same package.

When we try to run gqlgen, we get this error:

```
merging type systems failed: unable to bind to interface: github.com/wendorf/gqlgen-error/gql.User does not satisfy the interface github.com/wendorf/gqlgen-error/models.Subject
```

Digging deeper, it's because we use types.Implements in
codegen/interface.go, which does a shallow object comparison. Because
the type has been reloaded, it refers to a _different_ interface type
object than the one we're comparing against, and get a false negative.

By clearing the package cache and repopulating it, the whole package
cache is generated at the same time, and comparisons across packages
work.

To see a demo of this, check out
https://github.com/wendorf/gqlgen-error and try the following:

1. Checkout the works-with-v0.10.2 branch and `go generate ./...` to see
   that it works
2. Checkout the breaks-with-v0.13.0 branch (or run go get
   github.com/99designs/gqlgen@v0.13.0 yourself) and `go generate ./...`
   to see errors
3. Checkout the works-with-pull-request branch and `go generate ./...`
   to see that it works again. This branch adds a go.mod replace
   directive to use the gqlgen code in this PR.

The demo starts at v0.10.2 since it is the last release without this
problem. #1020 introduces the
code that fails in this scenario.
  • Loading branch information
wendorf committed Mar 17, 2021
1 parent 7985db4 commit 58c3c4f
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 9 deletions.
23 changes: 15 additions & 8 deletions codegen/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,15 +194,8 @@ func (c *Config) Init() error {
}

c.injectBuiltins()

// prefetch all packages in one big packages.Load call
pkgs := []string{
"github.com/99designs/gqlgen/graphql",
"github.com/99designs/gqlgen/graphql/introspection",
}
pkgs = append(pkgs, c.Models.ReferencedPackages()...)
pkgs = append(pkgs, c.AutoBind...)
c.Packages.LoadAll(pkgs...)
c.Packages.LoadAll(c.packageList()...)

// check everything is valid on the way out
err = c.check()
Expand All @@ -213,6 +206,20 @@ func (c *Config) Init() error {
return nil
}

func (c *Config) packageList() []string {
pkgs := []string{
"github.com/99designs/gqlgen/graphql",
"github.com/99designs/gqlgen/graphql/introspection",
}
pkgs = append(pkgs, c.Models.ReferencedPackages()...)
pkgs = append(pkgs, c.AutoBind...)
return pkgs
}

func (c *Config) ReloadAllPackages() {
c.Packages.ReloadAll(c.packageList()...)
}

func (c *Config) injectTypesFromSchema() error {
c.Directives["goModel"] = DirectiveConfig{
SkipRuntime: true,
Expand Down
7 changes: 7 additions & 0 deletions internal/code/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ type Packages struct {
numNameCalls int // stupid test steam. ignore.
}

// ReloadAll will call LoadAll after clearing the package cache, so we can reload
// packages in the case that the packages have changed
func (p *Packages) ReloadAll(importPaths ...string) []*packages.Package {
p.packages = nil
return p.LoadAll(importPaths...)
}

// LoadAll will call packages.Load and return the package data for the given packages,
// but if the package already have been loaded it will return cached values instead.
func (p *Packages) LoadAll(importPaths ...string) []*packages.Package {
Expand Down
11 changes: 10 additions & 1 deletion plugin/modelgen/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,13 +214,22 @@ func (m *Plugin) MutateConfig(cfg *config.Config) error {
b = m.MutateHook(b)
}

return templates.Render(templates.Options{
err := templates.Render(templates.Options{
PackageName: cfg.Model.Package,
Filename: cfg.Model.Filename,
Data: b,
GeneratedHeader: true,
Packages: cfg.Packages,
})
if err != nil {
return err
}

// We may have generated code in a package we already loaded, so we reload all packages
// to allow packages to be compared correctly
cfg.ReloadAllPackages()

return nil
}

func isStruct(t types.Type) bool {
Expand Down

0 comments on commit 58c3c4f

Please sign in to comment.