Revert "Expose composed middleware via getMiddleware()" #3046
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for the explanation! Looking forward to 2.8 :) |
abernix
added a commit
that referenced
this pull request
Aug 23, 2019
* Re-land "Expose composed middleware via getMiddleware()" This reverts commit d05aceb which was originally #2435, but was reverted in 52ab22e (#3046) as a precaution, prior to releasing 2.7.0. This attempts to reland it. * Use an `express.Router` rather than `compose-middleware`. Aside from avoiding unnecessary dependencies, by using this Router, we avoid the `apollo-server` base package from behaving different than it has in the past. Specifically, `apollo-server` uses `/` as the default path, but it does so in a wild-card way which serves GraphQL requests on _any_ path. That means that `/graphqlllll` and `/graphql` both also served the GraphQL Playground interface, and also responded to GraphQL execution requests. Since some may be leveraging that behavior, we should preserve it, if we can. * Add CHANGELOG.md link to #3184, for awareness.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 4149a97 from #2435,
which has resulted in a change to middleware endpoints which I need to think about a bit more and just caught just before the graduation of the 2.7.0 release to
latest.In a nutshell 🌰: While this affects the
apollo-server-expressandapollo-server-koaintegrations directly, the previous pattern allowed thepathto function in a wildcard-y/glob-y manner which also allowed, e.g.,/graphql-literally-anythingto work as the GraphQL endpoint (for both GraphQL Playground and actual GraphQL execution). In reality, I don't think that should have been permitted, but if anyone had inadvertently leveraged this pattern, they might be surprised by this change. In particular, theapollo-serverpackage, which leveragesapollo-server-expressunder the hood, specifiespath: '/', so it falls into this category of permissiveness.The change being reverted here implemented a more exacting path match, which is arguably more correct, but I need to convince myself that this is fine for a bit longer so I'm going to punt this to 2.8.0 without enough time to discuss further.
That saddens me greatly as I'm quite excited about the newfound freedom that
getMiddlewarewould have provided to those that previously felt constrained by theapplyMiddlewarepattern, but I think I can get clarity on this relatively soon and re-ship it. It does seem to work quite well!