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

1.0.0-rc.2 Schema updated, but resolver not called #662

Closed
russell-dot-js opened this issue Jun 24, 2020 · 6 comments
Closed

1.0.0-rc.2 Schema updated, but resolver not called #662

russell-dot-js opened this issue Jun 24, 2020 · 6 comments
Labels
Question ❔ Not future request, proposal or bug issue Solved ✔️ The issue has been solved

Comments

@russell-dot-js
Copy link

Describe the Bug
Before I get into it... please forgive me for only knowing how to describe this bug in the context of graphql modules. While the repro is inside the context of graphql modules, I believe the bug is in type-graphql, as 0.17.6 works fine, but 1.0.0-rc.2 leads to an issue. However, the bug might be in graphql itself, not in type-graphql.

The main benefit of Graphql modules is that the library allows defining separate "modules", each with their own schemas and resolvers, and essentially compiles them all together, allowing developers to write clean, modular code. Modules can depend on and essentially extend each other, and we've been using it in production for almost 2 years now with great success. I'm looking into incorporating type-graphql, which graphql modules supports and has specific documentation on, which is how I found this bug.

The problem is, that when using type-graphql 1.0.0-rc.2, a resolver for [what I believe you refer to as a] "computed" field is not called, even though the schema is properly updated with that field definition.

To Reproduce
type-graphql resolver not called repro.zip

In the attachment above are two simple projects - one works, the other does not. The code in both projects is exactly the same, and the only difference is the version of type-graphql (and its peer dependency, graphql). Simply download and unzip, and for each of the two projects, yarn, and yarn local. This will pull up playground on port 4000.

When running the "working" project, execute the following query from playground:

# # Write your query or mutation here
query {  
  user(id: "1") {
    name
    orders {
      id
    }
  }
}

You will get back:

{
  "data": {
    "order": null,
    "user": {
      "name": "Russell",
      "orders": []
    }
  }
}

This is the expected behavior.

Now, from the "broken" project, run the query again. You will receive error "Cannot return null for non-nullable field User.orders."

Expected Behavior
Result should be

{
  "data": {
    "order": null,
    "user": {
      "name": "Russell",
      "orders": []
    }
  }
}

Rather than error "Cannot return null for non-nullable field User.orders."

Logs
This is where I wish I could provide more help - I've tried logging out the outputs of the generated resolver, but am not getting anything useful. The best I can get is confirm that typeof (new UserResolver()).orders is "function", as expected

Environment (please complete the following information):

  • OS: macOS Catalina
  • Node (e.g. 10.5.0): 12.10.0
  • Package version 1.0.0-rc.2
  • TypeScript version 3.9.5

Additional Context
This could also be a bug with graphql, rather than type-graphql, but could use your help debugging and routing to the right place.

@MichalLytek
Copy link
Owner

type-graphql resolver not called repro.zip

Zip file pasted on code hosting repository 😅 2020 is a crazy year

I believe the bug is in type-graphql, as 0.17.6 works fine, but 1.0.0-rc.2 leads to an issue.

TypeGraphQL 1.0 has a proper schema isolation, so maybe you forgot to register some resolver or reference some type in the other module.

Maybe this can guide you to find the root cause:
#607

If the repro zip is minimal enough, I will take a look, but I would prefer really small, clean and 3rd party deps free repo 😉

@MichalLytek MichalLytek added Community 👨‍👧 Something initiated by a community Need More Info 🤷‍♂️ Further information is requested labels Jun 24, 2020
@russell-dot-js
Copy link
Author

@MichalLytek it's a couple hundred LOC. If you prefer a git repo over zip, here you go: https://github.com/kingsmendv/typegraphql-issue-662

@MichalLytek
Copy link
Owner

graphql-modules makes it really hard to debug for me. I belive the issue is that it fails to merge the schemas properly.

I believe the bug is in type-graphql, as 0.17.6 works fine, but 1.0.0-rc.2 leads to an issue.

The bug was in 0.17.6 - every buildSchema call was producing a schema from a global, shared metadata storage (between graphql-modules) with all the types registered. So the modules was only a virtual thing, there was no isolation.

But now the v1.0 has a proper schema isolation, so if the UsersModule doesn't contain owners field resolver, it won't have it available and won't be callable by the graphql runtime.

If you can't provide my a clean, type-graphql only reproduction repository, without the modules merging magic, I can't help you with that.
Please open an issue on graphql-modules repository and ask them for support for type-graphql 1.0. They may need to adapt to this change and to be aware of it while trying to merge the two isolated schemas from two modules.

@MichalLytek
Copy link
Owner

I've created an example for GraphQL Modules and everything works fine:
https://github.com/MichalLytek/type-graphql/tree/master/examples/graphql-modules

Maybe you need to try to play with orphanedTypes to find the issue in your code. I'm not familiar enough with the GraphQL Modules to help you and it doesn't look like TypeGraphQL issue apart from the schema isolation breaking change, so closing this one 🔒

@MichalLytek MichalLytek added Question ❔ Not future request, proposal or bug issue Wontfix ❌ This will not be worked on and removed Need More Info 🤷‍♂️ Further information is requested labels Jul 5, 2020
@russell-dot-js
Copy link
Author

Thanks @MichalLytek! Looks like orphanedTypes did the trick. I was trying to find more information on the "schema isolation breaking change" and wasn't able to intuit what changed and that the orphanedTypes option was required. For anyone else who's curious in the future, the fix ultimately led me to this documentation:

https://typegraphql.com/docs/bootstrap.html#create-executable-schema

@MichalLytek
Copy link
Owner

In case of my example, it's about adding a field via the @Resolver(of => Type) - the Type is not referenced anywhere else in the schema, thus the field and type is not emitted in the partial module schema.

I am going to fix that issue so the usage with GraphQL Modules and similar cases will be less problematic.
orphanedTypes is only a workaround for edge cases, it shouldn't be required to use that.

@MichalLytek MichalLytek added Solved ✔️ The issue has been solved and removed Community 👨‍👧 Something initiated by a community Wontfix ❌ This will not be worked on labels Jul 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question ❔ Not future request, proposal or bug issue Solved ✔️ The issue has been solved
Projects
None yet
Development

No branches or pull requests

2 participants