-
Notifications
You must be signed in to change notification settings - Fork 254
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
Change dependency on Apollo Server 3 to new type package #2044
Conversation
✅ Deploy Preview for apollo-federation-docs canceled.
|
Appears to build and pass tests. Need to test that it actually works in practice with both AS3 and AS4 (and AS4 won't until we post-process the request context after execution). Need to document and explain why these changes are safe (or ways that they aren't). |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
3f0b29e
to
25c7017
Compare
const maxAge = parsed['max-age']; | ||
if (typeof maxAge === 'string' && maxAge.match(/^[0-9]+$/)) { | ||
hint.maxAge = +maxAge; | ||
} | ||
if (parsed['private'] === true) { | ||
hint.scope = CacheScope.Private; | ||
hint.scope = 'PRIVATE'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change has a runtime effect, but a trivial one as CacheScope.Private
's runtime value is 'PRIVATE'
.
gateway-js/package.json
Outdated
@@ -29,15 +29,14 @@ | |||
"@apollo/core-schema": "~0.3.0", | |||
"@apollo/federation-internals": "file:../internals-js", | |||
"@apollo/query-planner": "file:../query-planner-js", | |||
"@apollo/server-gateway-interface": "https://pkg.csb.dev/apollographql/apollo-server/commit/bdd19924/@apollo/server-gateway-interface", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These will be changed to released packages before merging this PR.
gateway-js/package.json
Outdated
@@ -29,15 +29,14 @@ | |||
"@apollo/core-schema": "~0.3.0", | |||
"@apollo/federation-internals": "file:../internals-js", | |||
"@apollo/query-planner": "file:../query-planner-js", | |||
"@apollo/server-gateway-interface": "https://pkg.csb.dev/apollographql/apollo-server/commit/bdd19924/@apollo/server-gateway-interface", | |||
"@apollo/usage-reporting-protobuf": "https://pkg.csb.dev/apollographql/apollo-server/commit/bdd19924/@apollo/usage-reporting-protobuf", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This might get changed back to apollo-reporting-protobuf
until we have a non-alpha of the above, although the package is pretty much unchanged other than name and it doesn't have any dependencies.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prettier not enforced in this repo but there are some spots you could touch up if you choose to. This will be backported as well?
@@ -126,7 +126,9 @@ describe('executeQueryPlan', () => { | |||
overrideResolversInService('accounts', { | |||
RootQuery: { | |||
me() { | |||
throw new AuthenticationError('Something went wrong'); | |||
throw new GraphQLError('Something went wrong', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is losing the name
here anything to worry about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope not? Note that it doesn't affect the JSON output (I think!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(It is also a test.)
3ff0891
to
8b11d11
Compare
Backport of #2044. This removes the last dependency on Apollo Server from Apollo Gateway. Part of apollographql/apollo-server#6057 and apollographql/apollo-server#6719 The new package `@apollo/server-gateway-interface` (in the apollo-utils repo for now, though it should move to the apollo-server repo once AS4 is fully released) defines types that are pretty close to compatible with the AS3 types previously used here, but don't require a dependency on the entirety of AS3. This new package will be used by AS4 and AS4 will convert its data into the format described by these types. This change is entirely a build-time change (other than a slight change to how a enum is referenced). So the worst case scenario if this differs unintentionally from the original AS3 definitions is that users can apply a bit of `as any` to fix it. Note that we've removed some `<TContext>` from types that it turned out were only ever instantiated with `Record<string, any>` anyway. They are left in in RemoteGraphQLDataSource because users making their own data sources can explicitly specify their context type. The types in `@apollo/server-gateway-interface` are pretty close to the AS3 types (with different names) but there are some slight differences. The cache scope enum is replaced with `any`, as enums are not structurally typed and it is otherwise difficult to type them. `GatewayInterface` now expects `onSchemaLoadOrUpdate` to exist and doesn't mention the old `onSchemaChange`. For now, we leave `apollo-reporting-protobuf` alone, so we don't have a direct dependency on a prerelease.
8b11d11
to
effd384
Compare
Backport of #2044. Some tweaks for graphql@15 and Node 12 support. This removes the last dependency on Apollo Server from Apollo Gateway. Part of apollographql/apollo-server#6057 and apollographql/apollo-server#6719 The new package `@apollo/server-gateway-interface` (in the apollo-utils repo for now, though it should move to the apollo-server repo once AS4 is fully released) defines types that are pretty close to compatible with the AS3 types previously used here, but don't require a dependency on the entirety of AS3. This new package will be used by AS4 and AS4 will convert its data into the format described by these types. This change is entirely a build-time change (other than a slight change to how a enum is referenced). So the worst case scenario if this differs unintentionally from the original AS3 definitions is that users can apply a bit of `as any` to fix it. Note that we've removed some `<TContext>` from types that it turned out were only ever instantiated with `Record<string, any>` anyway. They are left in in RemoteGraphQLDataSource because users making their own data sources can explicitly specify their context type. The types in `@apollo/server-gateway-interface` are pretty close to the AS3 types (with different names) but there are some slight differences. The cache scope enum is replaced with `any`, as enums are not structurally typed and it is otherwise difficult to type them. `GatewayInterface` now expects `onSchemaLoadOrUpdate` to exist and doesn't mention the old `onSchemaChange`. For now, we leave `apollo-reporting-protobuf` alone, so we don't have a direct dependency on a prerelease.
This removes the last dependency on Apollo Server from Apollo Gateway. Part of apollographql/apollo-server#6057 and apollographql/apollo-server#6719 The new package `@apollo/server-gateway-interface` (in the apollo-utils repo for now, though it should move to the apollo-server repo once AS4 is fully released) defines types that are pretty close to compatible with the AS3 types previously used here, but don't require a dependency on the entirety of AS3. This new package will be used by AS4 and AS4 will convert its data into the format described by these types. This change is entirely a build-time change (other than a slight change to how a enum is referenced). So the worst case scenario if this differs unintentionally from the original AS3 definitions is that users can apply a bit of `as any` to fix it. Note that we've removed some `<TContext>` from types that it turned out were only ever instantiated with `Record<string, any>` anyway. They are left in in RemoteGraphQLDataSource because users making their own data sources can explicitly specify their context type. The types in `@apollo/server-gateway-interface` are pretty close to the AS3 types (with different names) but there are some slight differences. The cache scope enum is replaced with `any`, as enums are not structurally typed and it is otherwise difficult to type them. `GatewayInterface` now expects `onSchemaLoadOrUpdate` to exist and doesn't mention the old `onSchemaChange`. For now, we leave `apollo-reporting-protobuf` alone, so we don't have a direct dependency on a prerelease.
effd384
to
9dc0438
Compare
This removes the last dependency on Apollo Server from Apollo Gateway.
Part of apollographql/apollo-server#6057 and
apollographql/apollo-server#6719
The new package
@apollo/server-gateway-interface
(in the apollo-utilsrepo for now, though it should move to the apollo-server repo once AS4
is fully released) defines types that are pretty close to compatible
with the AS3 types previously used here, but don't require a dependency
on the entirety of AS3. This new package will be used by AS4 and AS4
will convert its data into the format described by these types.
This change is entirely a build-time change (other than a slight change
to how a enum is referenced). So the worst case scenario if this differs
unintentionally from the original AS3 definitions is that users can
apply a bit of
as any
to fix it.Note that we've removed some
<TContext>
from types that it turned outwere only ever instantiated with
Record<string, any>
anyway. They areleft in in RemoteGraphQLDataSource because users making their own data
sources can explicitly specify their context type.
The types in
@apollo/server-gateway-interface
are pretty close to theAS3 types (with different names) but there are some slight differences.
The cache scope enum is replaced with
any
, as enums are notstructurally typed and it is otherwise difficult to type them.
GatewayInterface
now expectsonSchemaLoadOrUpdate
to exist anddoesn't mention the old
onSchemaChange
.