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

Remove or reimplement modules constructor option #6062

Closed
glasser opened this issue Jan 27, 2022 · 10 comments
Closed

Remove or reimplement modules constructor option #6062

glasser opened this issue Jan 27, 2022 · 10 comments
Assignees
Milestone

Comments

@glasser
Copy link
Member

glasser commented Jan 27, 2022

Right now there are many different ways to get a schema into Apollo Server:

  1. Directly pass a schema which is an executable GraphQLSchema object that you make yourself
  2. Use gateway to have a managed schema that can be updated over time (and is provided as GraphQLSchema via that interface)
  3. Use typeDefs and resolvers (and parseOptions) which get passed to makeExecutableSchema from a particular version of @graphql-tools/schema
  4. Use modules which gets passed to buildServiceDefinition from @apollographql/apollo-tools (a package in the deprecated apollo-tooling repo)

makeExecutableSchema and buildServiceDefinition are very similar (honestly I don't understand how they differ) and it doesn't make much sense that we use two completely different implementations of the same logic just because you might want to have an extra layer of array in your nesting.

Most likely, we will just remove modules rather than reimplement it in terms of makeExecutableSchema. Folks who currently use modules can just pass new ApolloServer({ typeDefs: modules.map(({ typeDefs }) => typeDefs, resolvers: modules.map(({ resolvers }) => resolvers }) to migrate.

We could also reimplement it. That said, the current modules name is quite misleading. One might expect that there's actually something modular about it, rather than just being the same as typeDefs/resolvers but with an array in a different place.

Specifically, one might expect that something like:

new ApolloServer({
  modules: [{
    typeDefs: 'type Query { x: ID }',
    resolvers: { Query: { y: 'asdf' } },
  }, {
    typeDefs: 'extend type Query { y: ID }',
    resolvers: { Query: { x: 'bla' } },
  }]
})

would be some kind of error, since the resolvers and typeDefs don't actually match each other for a given module. But it's not because the implementation just merges all the things together anyway.

@glasser
Copy link
Member Author

glasser commented May 12, 2022

For an alpha we should at least drop the @apollographql/apollo-tools dep (either by removing the modules option entirely or reimplementing it in terms of makeExecutableSchema). Whether we can do more can be a later question.

@glasser glasser added rebase and removed rebase labels May 12, 2022
@glasser glasser changed the title Only one mechanism for converting GraphQL source/AST to schemas Remove or reimplement modules constructor option May 16, 2022
@glasser
Copy link
Member Author

glasser commented May 16, 2022

I factored out part of this into #6434.

The part that remains we actually did a while ago on 1081080.

@glasser glasser closed this as completed May 16, 2022
@vella-nicholas
Copy link

I am migrating from v3 to v4 and I need to refactor the modules constructor option. My application is written in TS and in order to keep my code clean, I created classes per entity. These classes implement the GraphQLSchemaModule interface and have the typeDefs and resolvers definitions.

What is a suggested way to add definitions and resolvers form these classes please?

@trevor-scheer
Copy link
Member

@vella-nicholas if your classes implement GraphQLSchemaModule (i.e. they just have a resolvers and typeDefs property on them) then the snippet in the migration guide should be helpful.

@vella-nicholas
Copy link

@trevor-scheer Correct my classes implemented GraphQLSchemaModule, I am pasting an example:

import gql from 'graphql-tag';
import { DocumentNode } from 'graphql';
import { BrandService } from '../../application/services/implementations/brand.service';
import { IBrandService } from '../../application/services/interfaces/IBrandService';
import { GetBrandRequest } from '../../application/services/messaging/BrandRequest';

export class Brand {
  private brandService: IBrandService;

  constructor();
  constructor(brandService: IBrandService);
  constructor(brandService?: IBrandService) {
    this.brandService = brandService ?? new BrandService();
  }

  get typeDefs(): DocumentNode {
    return gql`
      extend type Query {
        brands: [Brand]
        brand(request: BrandRequest!): Brand
      }

      type Brand {
        id: String
        name: String
        vertical: String
      }

      input BrandRequest {
        id: String!
        vertical: String!
      }
    `;
  }

  get resolvers() {
    return {
      Query: {
        brands: () => this.brandService.getBrands(),
        brand: (obj: any, args: any, context: any, info: any) =>
          this.brandService.getBrand(<GetBrandRequest>args.request),
      },
    };
  }
}

Will the snippet from the documentation still be applicable please?

@trevor-scheer
Copy link
Member

@vella-nicholas yes that should work. Try it out and open a new issue if you run into any problems.

@vella-nicholas
Copy link

@trevor-scheer ok thank you! One last thing please in the snippet from the documentation, what does modules.map represent?

@trevor-scheer
Copy link
Member

@vella-nicholas The array that you previously passed to the modules config option, i.e. in your case [Brand, ...]

@vella-nicholas
Copy link

@trevor-scheer Great thank you!

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants