[2.0] Import graphql-extensions and apollo-engine-reporting and avoid global apollo-server-env #1259
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
* dev: Update TypeScript to latest version, v2.7.2. * dev: Update `graphql` to latest version, v0.13.2. * dev: Update jest & dependencies to latest versions. * dev: Update type definitions for `graphql`, `node` and `jest`.
) In some cases, it's conceivable that the `format()` method may need to abort its decision to provide extension information at runtime, in the event that it doesn't have the proper information to return a full-result. The `format` method already removed false-y results, so this simply changes the types to allow the same.
Makes my editor integration happier (a bugfix in tsserver I think)
Same configuration as apollo-engine-js
It's not hard to consistently pass in an actual extension object to this low-level API.
This is a backwards-incompatible change: GraphQLExtension implementations and users of GraphQLExtensionStack (ie apollo-server-core) must change their implementations, if they implement any of the xDidStart/xDidEnd APIs. This allows "didEnd" handlers to refer to closure variables from the "didStart" handler rather than needing to store state on the extension. The new "didEnd" handlers run in the opposite order of the "didStart" handlers, so that they properly nest.
We'd rather tell people confused by literal removal to tweak the signature than tell people causing outages to do so.
evans
force-pushed
the
server-2.0/avoid-global-env
branch
6 times, most recently
from
ac10e22 to
e413f2d
Compare
evans
force-pushed
the
server-2.0/avoid-global-env
branch
from
e413f2d to
b654b5b
Compare
evans
force-pushed
the
server-2.0/avoid-global-env
branch
from
1dc3459 to
9fa227c
Compare
|
@martijnwalraven Thank you! There seemed to be a large amount of noise in the commits, so I decided to squash |
Merged
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 PR imports
graphql-extensionsandapollo-engine-reportinginto this monorepo. It adjusts dependencies and configuration to make sure everything compiles and tests are able to run.It also ensures explicit imports from
apollo-server-envare used for fetch and URL types, instead of relying on these being set on the global environment. This avoids type conflicts when people compile their projects with similar global types (like those from thedomlib).Unfortunately,
apollo-fetch(used in tests) andgraphql-request(a dependency of GraphQL Playground) still rely on global fetch. So I've had to add back thedomlib totsconfig.jsonfor the packages that depend on those. This shouldn't affect anything at runtime, but we should still find a better solution for this.I also had to jump through some hoops to get the
apollo-datasource-resttests to use a mocked version ofapollo-server-env(so we can get a mockedfetch). Setting the Jestrootslet to an unfortunate side effect: all tests in dependent packages also run. As a workaround, I adjustedtestRegexto include the package name, but that feels like a hack.