nfroidure / whook

Build strong and efficient REST web services.
https://slides.com/nfroidure/introducing-whook
MIT License
31 stars 6 forks source link

refactor(@whook/graphql): update Apollo server to the last major version #143

Closed nfroidure closed 1 year ago

nfroidure commented 1 year ago

fix #140

nfroidure commented 1 year ago

@Oupsla yes, the main issue is with the context. In the previous version, the server were asking for a context function. In the new version, it is required at the http transaction level : https://github.com/nfroidure/whook/pull/143/files#diff-1eb63035ea7297ec70ff6178114f5f2b92254e9f3dad38f597be116d2c8080acR118-R122

I was wondering if I should change the newly create GRAPHQL_SERVER_CONTEXT service into a function to cover wider usages, WDYT? Do you use it currently? https://github.com/nfroidure/whook/pull/143/files#diff-1eb63035ea7297ec70ff6178114f5f2b92254e9f3dad38f597be116d2c8080acR84

Oupsla commented 1 year ago

I was wondering if I should change the newly create GRAPHQL_SERVER_CONTEXT service into a function to cover wider usages, WDYT?

Maybe to stay compatible with the previous context function it could be nice. At the moment, I only use the context to clear graphQL loaders between each calls



async function initGraphQLServerConfig({
  graphQLLoaders,
}: {
  graphQLLoaders: GraphQLLoadersService;
}) {
  (...)

  return {
    context: async (context) => {
      await graphQLLoaders.clearAllLoaders();
      return context;
    },
   (...)
  };
}
nfroidure commented 1 year ago

@Oupsla ok, I'll fix this before merging. Thanks for your feedback