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

Base abstract class is referenced/instantiated when child class should be #1492

Open
comp615 opened this issue Aug 16, 2023 · 2 comments
Open
Labels
Community 👨‍👧 Something initiated by a community Need More Info 🤷‍♂️ Further information is requested Question ❔ Not future request, proposal or bug issue

Comments

@comp615
Copy link

comp615 commented Aug 16, 2023

Describe the Bug
Assume you have some type of union or polymorphic object as shown in the example. You may want to have an abstract value which can be set by any child/implementation. In many of the examples, this is shown as an abstract class with an @ObjectType({ implements: BaseClass }) in children.

However, when it comes to actually running the methods, it seems like they are being executed on the BaseClass instead of the ChildClass. This means that concrete base class methods cannot reference or use abstract methods on the base class (since they are not instantiated).

While I haven't spelunked enough to understand exactly what's going on, my hunch is that typegraphql is calling BaseClass.value() instead of ChildClass.value(). This distinction would not matter normally, but comes into play when you truly start using abstraction and inheritance.

To Reproduce
https://codesandbox.io/p/sandbox/typegraphql-v2-demo-forked-wkyhwm?file=/src/index.ts:106,33

Expected Behavior
The function as shown should work and return the value from the child class

Logs

          "TypeError: this._publicationDate is not a function",
          "    at BaseItem.publicationDate (/project/sandbox/src/index.ts:30:17)",
          "    at /project/sandbox/node_modules/type-graphql/dist/resolvers/create.js:64:45",
          "    at applyMiddlewares (/project/sandbox/node_modules/type-graphql/dist/resolvers/helpers.js:59:16)",
          "    at /project/sandbox/node_modules/type-graphql/dist/resolvers/create.js:52:47",
          "    at field.resolve (/project/sandbox/node_modules/@apollo/server/src/utils/schemaInstrumentation.ts:82:22)",
          "    at executeField (/project/sandbox/node_modules/graphql/execution/execute.js:492:20)",
          "    at executeFields (/project/sandbox/node_modules/graphql/execution/execute.js:414:22)",
          "    at completeObjectValue (/project/sandbox/node_modules/graphql/execution/execute.js:925:10)",
          "    at completeValue (/project/sandbox/node_modules/graphql/execution/execute.js:646:12)",
          "    at completeValue (/project/sandbox/node_modules/graphql/execution/execute.js:595:23)"

Environment (please complete the following information):

  • OS: all
  • Node 16
  • Package version - v2
  • TypeScript version - 4.7
@MichalLytek
Copy link
Owner

  1. In order to include the subtypes in the schema, you need to reference them in the types of your schema (query return type, other field type, etc.)

image

  1. Then you need to let know the runtime what is the subtype of the type that you return:
    https://typegraphql.com/docs/interfaces.html#resolving-type

Be aware that when our object type is implementing a GraphQL interface type, we have to return an instance of the type class in our resolvers. Otherwise, graphql-js will not be able to detect the underlying GraphQL type correctly.

If you had done step 1, you would get the error in the response.

  1. As you can see from your example, you can have an interface type without any subtypes, so it can't be abstract and rely on abstract methods that the child class will provide the implementation.

@MichalLytek MichalLytek added Question ❔ Not future request, proposal or bug issue Community 👨‍👧 Something initiated by a community Need More Info 🤷‍♂️ Further information is requested labels Aug 17, 2023
@comp615
Copy link
Author

comp615 commented Aug 17, 2023

Appreciate the response. Ok, I updated the example. It was an incomplete demo of that aspect for sure. Thanks for bearing with me as I work through this in my brain. We read through the example on the site, but I can't seem to get it going.

I updated the sandbox to try a few methods:
https://codesandbox.io/p/sandbox/typegraphql-v2-demo-forked-wkyhwm?file=/src/index.ts:88,5-94,19

As you mentioned, it seems like returning the data as the appropriate class works. However, I wasn't able to get the resolverType method to work. Is that expected? We're trying to migrate a legacy system to TypeGraphQL so trying to figure out patterns and how we might do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Need More Info 🤷‍♂️ Further information is requested Question ❔ Not future request, proposal or bug issue
Projects
None yet
Development

No branches or pull requests

3 participants
@comp615 @MichalLytek and others