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

[v1] Weird behaviour when extending createSchemaForApollo's schema #1725

Closed
darkbasic opened this issue Jun 9, 2021 · 6 comments
Closed

Comments

@darkbasic
Copy link
Contributor

Describe the bug

I'm upgrading accounts-js to graphql-modules v1. Accounts-js will use v1 internally, but it still needs to supports users who don't want to make use of graphql-modules in their projects. These users, instead of creating their own graphql module, will simply merge their typedefs/resolvers with those provided by accounts-js.

Basically users who want to use graphql-modules will use something like this:

const myModule = createModule({
  id: 'mine',
  typeDefs,
  resolvers,
});

const accountsSchema = createApplication({
  modules: [
    createAccountsCoreModule({ accountsServer }),
    createAccountsPasswordModule({ accountsPassword }),
    myModule,
  ],
}).createSchemaForApollo();

SchemaDirectiveVisitor.visitSchemaDirectives(accountsSchema, {
  auth: AuthenticatedDirective,
} as any);

const server = new ApolloServer({
  schema: accountsSchema,
  context: ({ req, connection }) => {
    return context({ req, connection }, { accountsServer });
  },
});

while users who don't want to use graphql-modules will use something like this:

const accountsSchema = createApplication({
  modules: [
    createAccountsCoreModule({ accountsServer }),
    createAccountsPasswordModule({ accountsPassword }),
  ],
}).createSchemaForApollo();

const accountsTypeDefs = parse(printSchemaWithDirectives(accountsSchema));
const accountsResolvers = getResolversFromSchema(accountsSchema);

const schema = makeExecutableSchema({
  typeDefs: mergeTypeDefs([typeDefs, accountsTypeDefs]),
  resolvers: mergeResolvers([accountsResolvers as any, resolvers]),
  schemaDirectives: {
    auth: AuthenticatedDirective,
  } as any,
});

const server = new ApolloServer({
  schema,
  context: ({ req, connection }) => {
    return context({ req, connection }, { accountsServer });
  },
});

While the former works flawlessly, the latter shows some weird behaviour.

In this example the user wants to extend the User type provided by accounts-js with additional fields:

extend type User {
  firstName: String!
  lastName: String!
}

If I look at the generated schema in GraphiQL I can see that the type has been extended correctly:

type User {
  firstName: String!
  lastName: String!
  id: ID!
  emails: [EmailRecord!]
  username: String
}

but for some reason it doesn't return those types when I run the authenticate mutation: Cannot return null for non-nullable field User.firstName.
This is weird because in all the top level resolvers this field has been provided but somehow it's missing once it reaches the User resolver:

authenticateResolvers
{
  _id: 60bf23248f4bc1acbcab0de3,
  firstName: 'John',
  lastName: 'Doe',
  services: {
    password: {
      bcrypt: '$2a$10$PgtRhEby4y.JqDYX9z2sReukBAIhLHX6k1Qvp5LimBCdcvhb73Ioi'
    }
  },
  createdAt: 1623139108286,
  updatedAt: 1623139108286,
  emails: [ { address: '[email protected]', verified: false } ],
  id: '60bf23248f4bc1acbcab0de3'
}

LoginResultResolvers
{
  _id: 60bf23248f4bc1acbcab0de3,
  firstName: 'John',
  lastName: 'Doe',
  services: {
    password: {
      bcrypt: '$2a$10$PgtRhEby4y.JqDYX9z2sReukBAIhLHX6k1Qvp5LimBCdcvhb73Ioi'
    }
  },
  createdAt: 1623139108286,
  updatedAt: 1623139108286,
  emails: [ { address: '[email protected]', verified: false } ],
  id: '60bf23248f4bc1acbcab0de3'
}

UserResolvers
[Object: null prototype] {
  id: '60bf23248f4bc1acbcab0de3',
  emails: [
    [Object: null prototype] {
      address: '[email protected]',
      verified: false,
      __typename: 'EmailRecord'
    }
  ],
  __typename: 'User'
}

Everything works flawlessly with the former approach (the user creates its own graphql-module).
I tried without createSchemaForApollo but I've got context.ɵgetModuleContext is not a function.

To Reproduce
Steps to reproduce the behavior:

This is the accounts-js branch with graphql-modules v1: https://github.com/darkbasic/accounts/tree/f40419e14030f887066e34d1d454aa5f23cb931c

git clone https://github.com/darkbasic/accounts.git
git checkout f40419e14030f887066e34d1d454aa5f23cb931c
yarn
yarn setup
yarn compile
# Shell 1
cd examples/graphql-server-typescript
docker-compose up -d
yarn start
# Shell 2
cd examples/react-graphql-typescript
yarn start
# Create a new user (I didn't check if it works with the latter approach, change graphql-server-typescript to the former if it doesn't)
# Try to log in (gets Cannot return null for non-nullable field User.firstName)

Expected behavior

There should be a way to be able to extend the typedefs/resolvers from a project which uses graphql-modules in a project which makes use of apollo-server but not graphql-modules itself. What am I doing wrong?

Environment:

  • OS: Fedora Linux 34
  • @graphql-modules/1.4.2:
  • NodeJS: v14.17.0
@ardatan
Copy link
Collaborator

ardatan commented Jun 9, 2021

createSchemaForApollo uses wrapSchema. The real schema is wrapped inside that's why you cannot modify it directly.
You can get the typeDefs and resolvers from generated app.

const { typeDefs, resolvers }= createApplication({ ... });

@darkbasic
Copy link
Contributor Author

I tried, but I get context.ɵgetModuleContext is not a function

@ardatan
Copy link
Collaborator

ardatan commented Jun 9, 2021

@darkbasic Because you still need the context.
See #1724 (comment)

@darkbasic
Copy link
Contributor Author

So basically graphql-modules v1 is a no go for libraries who also want to target non graphql-modules users who wish to use Apollo Server: I cannot realistically expect them to manually setup weakmaps :(

@darkbasic
Copy link
Contributor Author

Would be possible to wrap the schema with createSchemaForApollo after letting the user to merge it with its own schema?

@ardatan
Copy link
Collaborator

ardatan commented Aug 16, 2021

You can use schemaBuilder to decide how to build schema from typeDefs and resolvers;
https://www.graphql-modules.com/docs/api#applicationconfig

@ardatan ardatan closed this as completed Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants