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

[fusion-plugin-apollo] ApolloContextToken is broken #613

Closed
conradreuter opened this issue Aug 6, 2019 · 4 comments
Closed

[fusion-plugin-apollo] ApolloContextToken is broken #613

conradreuter opened this issue Aug 6, 2019 · 4 comments

Comments

@conradreuter
Copy link

Since the latest version (2.0.3) of fusion-plugin-apollo, using a custom Apollo Context seems to be broken.

I created a repo reproducing the issue: https://github.com/conradreuter/fusion-plugin-apollo-context-bug-reproduction

During my rather short investigation I figured out that the context passed to Apollo Server might be wrong. The custom context is passed to the ApolloClientPlugin instead of the original Fusion context.

@conradreuter
Copy link
Author

conradreuter commented Aug 6, 2019

Workaround:

The project I am working on creates a custom Apollo Context roughly like this:

function createContext(fusionCtx) {
  const ctx = {}
  // ... add stuff to ctx
  return ctx
}

This is ugly, but works:

function createContext(fusionCtx) {
  const ctx = {}
  // ... add stuff to ctx
  return Object.assign(ctx, fusionCtx)
}

Fortunately enough, the custom context and the Fusion context don't seem to share any property names.

@ganemone
Copy link
Contributor

ganemone commented Aug 6, 2019

Hey - thanks for catching this. Looks like we have some code that assumes that the fusion ctx is used as the ctx. I will say, I think the best approach is to use the default context behavior. Since resolvers are plugins, anything you want to inject into resolvers via context can be done via the fusion DI system. If you don't have the fusion context in the resolvers, you end up seriously limiting what can be done in resolvers.

@conradreuter
Copy link
Author

I see the point of using DI, but that would require me to change quite a substantial amount of code.

The Fusion context is already available in the resolvers, as a field of the custom context.

@conradreuter
Copy link
Author

Fixed by fa877ad

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