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

Exported Express/Connect Middleware #1907

Closed
wants to merge 10 commits into from
Closed

Exported Express/Connect Middleware #1907

wants to merge 10 commits into from

Conversation

wyattjoh
Copy link

@wyattjoh wyattjoh commented Oct 31, 2018

Exposes the graphqlExpress, ExpressGraphQLOptionsFunction, and graphqlConnect from apollo-server-express that would support the use case described #1117 (comment)

Used with great effect here:

https://github.com/coralproject/talk/blob/482b9a61585deb689acb454e1bbeef3ec003c34d/src/core/server/graph/common/middleware/index.ts

TODO:

  • Update CHANGELOG.md with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

@apollo-cla
Copy link

@wyattjoh: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

@ghost ghost added the ⛲️ feature New addition or enhancement to existing solutions label Oct 31, 2018
@BrendanBall
Copy link

What's holding this up? I really need this to be able to use apollo server 2

@wyattjoh
Copy link
Author

wyattjoh commented Feb 5, 2019

This "hack" works for now:

https://github.com/coralproject/talk/blob/482b9a61585deb689acb454e1bbeef3ec003c34d/src/core/server/graph/common/middleware/index.ts#L5-L9

Not ideal, but am currently a bit unsure as to why this is being kept internal. A balance has to be struck with "ease of use" and "deep integration" to use this server going forward.

@abernix
Copy link
Member

abernix commented Jun 30, 2019

#2435 should offer a better solution here and has been merged into v2.7.0 (currently being staged on #2937 and (currently) at 2.7.0-alpha.2.

While it's still a "composed" middleware that is retrieved from getMiddleware, it should be possible to toggle the various aspects of that composition on and off (with options to getMiddleware) and it will still provide the full functionality of the Apollo Server request pipeline (caching, plugins, etc.), which this PRs proposal doesn't handle.

Sorry for the very long delay in responding to this, but feedback very much appreciated!

@abernix abernix closed this Jun 30, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⛲️ feature New addition or enhancement to existing solutions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants