Require legacy incremental execute as an ApolloServer option instead of a dynamic import#8161
Require legacy incremental execute as an ApolloServer option instead of a dynamic import#8161jerelmiller merged 9 commits intomainfrom
ApolloServer option instead of a dynamic import#8161Conversation
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removedBuild ID: 0a865973beb6431c37d3ebc9 URL: https://www.apollographql.com/docs/deploy-preview/0a865973beb6431c37d3ebc9 |
| ])('basic @defer working with accept: %s', async (accept) => { | ||
| const app = await createApp({ typeDefs, resolvers }); | ||
| const { legacyExecuteIncrementally } = await import( | ||
| // @ts-ignore might not be installed |
There was a problem hiding this comment.
Then what happens to the test if it's not installed?
There was a problem hiding this comment.
It gets installed when in smoke-tests/prepare.sh, but only when the GRAPHQL_JS_VERSION environment variable is set (which happens conditionally).
apollo-server/smoke-test/prepare.sh
Lines 21 to 25 in ae1be29
Since this isn't installed by default though, we get TypeScript errors when the tests are run normally (i.e. npm test), so I added this to allow the normal test suite to pass without TypeScript complaining. These tests successfully execute because they are only run when GraphQL 17 is installed (you can see the condition higher up in the test suite).
There was a problem hiding this comment.
Just want to make sure you know this is a package that we publish to npm that integration authors can depend on for a test suite (ie it's not only run in the context of this repo's CI) — I think that works with this approach though.
| async function tryToLoadLegacyExecuteIncrementally() { | ||
| try { | ||
| // @ts-ignore `@yaacovcr/transform` is an optional peer dependency | ||
| const transform = await import('@yaacovcr/transform'); |
There was a problem hiding this comment.
Shouldn't we drop the peer dep?
There was a problem hiding this comment.
We never actually added one because we didn't want it to be auto-installed by some package managers which might conflict with the current version of graphql.
I probably should have put optional peer dependency in quotes in this comment since its not an actual peer dependency listed in package.json 😆:
apollo-server/packages/server/package.json
Lines 149 to 151 in ae1be29
|
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. |
| ``` | ||
|
|
||
| There is nothing else to configure. Apollo Server will load the necessary utility if this package is installed. | ||
| You will then need to configure Apollo Server with the `legacyExperimentalExecuteIncrementally` option in order to provide support for the legacy incremental format. |
There was a problem hiding this comment.
The command is right above this line so I think that covers it?
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @apollo/server@5.2.0 ### Minor Changes - [#8161](#8161) [`51acbeb`](51acbeb) Thanks [@jerelmiller](https://github.com/jerelmiller)! - Fix an issue where some bundlers would fail to build because of the dynamic import for the optional peer dependency on `@yaacovcr/transform` introduced in `@apollo/server` 5.1.0. To provide support for the legacy incremental format, you must now provide the `legacyExperimentalExecuteIncrementally` option to the `ApolloServer` constructor. ```ts import { legacyExecuteIncrementally } from '@yaacovcr/transform'; const server = new ApolloServer({ // ... legacyExperimentalExecuteIncrementally: legacyExecuteIncrementally, }); ``` If the `legacyExperimentalExecuteIncrementally` option is not provided and the client sends an `Accept` header with a value of `multipart/mixed; deferSpec=20220824`, an error is returned by the server. ## @apollo/server-integration-testsuite@5.2.0 ### Patch Changes - Updated dependencies \[[`51acbeb`](51acbeb)]: - @apollo/server@5.2.0 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixes #8159
Dynamically importing
@yaacovcr/transformif available has proven to be a difficult solution because some bundlers fail to build when the package isn't installed.This PR addresses this issue by making the legacy execute function an option that can be provided to the
ApolloServerconstructor. This means no direct dependency on@yaacovcr/transformand makes it possible to use other legacy execution functions if available.To migrate, import the
legacyExecuteIncrementallyand provide it as thelegacyExperimentalExecuteIncrementallyoption to theApolloServerconstructor.