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

consider moving expressMiddleware to a deep import #6587

Closed
glasser opened this issue Jun 16, 2022 · 1 comment
Closed

consider moving expressMiddleware to a deep import #6587

glasser opened this issue Jun 16, 2022 · 1 comment
Assignees
Milestone

Comments

@glasser
Copy link
Member

glasser commented Jun 16, 2022

@apollo/server/express or even @apollo/server/express4.
it doesn't have runtime dependencies but it especially might be nice to do the latter since express5 is imminent (sigh, the best thing about Express is that it hasn't been a moving target...)

@glasser glasser added this to the Release 4.0 milestone Jun 16, 2022
@glasser glasser self-assigned this Jun 21, 2022
glasser added a commit that referenced this issue Jun 24, 2022
While expressMiddleware has no runtime dependencies, it does rely on the
specific API of Express 4. One of the nice things about Express is that
its development had slowed and it was a very stable target, but all good
things must come to an end: Express 5 may be coming soon
expressjs/express#4920

(I kid: the changes in Express 5, like support for proper error handling
with async functions, look great.)

So let's preemptively put this under `/express4` so that if the Express
5 middleware needs to be a bit different (if nothing else, the req/res
types in the context function may vary) that we'll have an obvious place
to put it.

Fixes #6587.
glasser added a commit that referenced this issue Jun 24, 2022
While expressMiddleware has no runtime dependencies, it does rely on the
specific API of Express 4. One of the nice things about Express is that
its development had slowed and it was a very stable target, but all good
things must come to an end: Express 5 may be coming soon
expressjs/express#4920

(I kid: the changes in Express 5, like support for proper error handling
with async functions, look great.)

So let's preemptively put this under `/express4` so that if the Express
5 middleware needs to be a bit different (if nothing else, the req/res
types in the context function may vary) that we'll have an obvious place
to put it.

Fixes #6587.
@glasser
Copy link
Member Author

glasser commented Jun 24, 2022

Fixed on version-4 in #6612

@glasser glasser closed this as completed Jun 24, 2022
@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
None yet
Development

No branches or pull requests

1 participant