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

remove ApolloServer.willStart #5050

Closed
glasser opened this issue Mar 22, 2021 · 2 comments
Closed

remove ApolloServer.willStart #5050

glasser opened this issue Mar 22, 2021 · 2 comments

Comments

@glasser
Copy link
Member

glasser commented Mar 22, 2021

#4981 (v2.22) replaces the protected willStart with start and ensureStarting. It leaves the protected method around for use by other integrations but they should switch to either start (if the goal is to stop and wait until starting is done, throwing any errors) or ensureStarting (if the goal is to begin startup in the background; you can rely on graphQLServerOptions to await success). AS3 should remove willStart.

@glasser glasser added this to the Release 3.x milestone Mar 22, 2021
@glasser
Copy link
Member Author

glasser commented Apr 30, 2021

Also consider making start required, and changing getMiddleware to async.

@glasser glasser added this to To Organize in Apollo Server 3 May 2, 2021
@glasser glasser moved this from To Organize to Straightforward breaking changes in Apollo Server 3 May 2, 2021
glasser added a commit that referenced this issue May 8, 2021
AS v2.22 introduced the method `server.start()` which could be awaited
immediately after `new ApolloServer()` in order to catch startup errors
(for example, errors loading schema from gateway).

This method exists only for non-serverless framework integrations. For
`apollo-server` its semantics are integrated into `server.listen()` and
for serverless frameworks you typically must set up the handler
synchronously.

In AS2 this method was optional. This PR makes the method required. So
instead of the framework integration kicking off a `start` in the
background if you don't do it yourself, it just throws an error telling
you to call start.

Fixes #5050.
glasser added a commit that referenced this issue May 10, 2021
AS v2.22 introduced the method `server.start()` which could be awaited
immediately after `new ApolloServer()` in order to catch startup errors
(for example, errors loading schema from gateway).

This method exists only for non-serverless framework integrations. For
`apollo-server` its semantics are integrated into `server.listen()` and
for serverless frameworks you typically must set up the handler
synchronously.

In AS2 this method was optional. This PR makes the method required. So
instead of the framework integration kicking off a `start` in the
background if you don't do it yourself, it just throws an error telling
you to call start.

Fixes #5050.
glasser added a commit that referenced this issue May 10, 2021
AS v2.22 introduced the method `server.start()` which could be awaited
immediately after `new ApolloServer()` in order to catch startup errors
(for example, errors loading schema from gateway).

This method exists only for non-serverless framework integrations. For
`apollo-server` its semantics are integrated into `server.listen()` and
for serverless frameworks you typically must set up the handler
synchronously.

In AS2 this method was optional. This PR makes the method required. So
instead of the framework integration kicking off a `start` in the
background if you don't do it yourself, it just throws an error telling
you to call start.

Fixes #5050.
glasser added a commit that referenced this issue May 10, 2021
AS v2.22 introduced the method `server.start()` which could be awaited
immediately after `new ApolloServer()` in order to catch startup errors
(for example, errors loading schema from gateway).

This method exists only for non-serverless framework integrations. For
`apollo-server` its semantics are integrated into `server.listen()` and
for serverless frameworks you typically must set up the handler
synchronously.

In AS2 this method was optional. This PR makes the method required. So
instead of the framework integration kicking off a `start` in the
background if you don't do it yourself, it just throws an error telling
you to call start.

Fixes #5050.
@glasser
Copy link
Member Author

glasser commented May 10, 2021

Fixed on release-3.0.

I didn't change getMiddleware to async. I don't think it's necessary.

@glasser glasser closed this as completed May 10, 2021
Apollo Server 3 automation moved this from Straightforward breaking changes to Done May 10, 2021
@glasser glasser removed this from the Release 3.x milestone Jun 10, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

1 participant