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

types: Use explicit return type of express.Router on getMiddleware. #3230

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

abernix
Copy link
Member

@abernix abernix commented Aug 29, 2019

The TypeScript compiler is inferring the return type of this method
accurately, but emitting import("express-serve-static-core") directly into
the apollo-server-express type definitions.

Looking at the package-lock.json for the Apollo Server repository, I have
a working theory that there are multple @types/express's within this project
and that, when resolving the imports in the delcarations, a copy of
express-serve-static-core is not quite in the right resolution path —
potentially due to some package hoisting and module resolution.

To be honest, it's just a theory and I only spent about 3 or 4 minutes
looking at this, but I hope that this will address #3222 (intentionally not
using the keyword "Fixes" here because we'll wait for validation!)

cc @CallMeLaNN

The TypeScript compiler is inferring the return type of this method
accurately, but emitting `import("express-serve-static-core")` directly into
the `apollo-server-express` type definitions.

Looking at the `package-lock.json` for the Apollo Server repository, I have
a working theory that there are multple `@types/express`'s within this project
and that, when resolving the imports in the delcarations, a copy of
`express-serve-static-core` is not quite in the right resolution path —
potentially due to some package hoisting and module resolution.

To be honest, it's just a theory and I only spent about 3 or 4 minutes
looking at this, but I hope that this will address #3222 (intentionally not
using the keyword "Fixes" here because we'll wait for validation!)
@abernix abernix force-pushed the abernix/express-types-fix-maybe-3222 branch from 2d159b9 to 3f7b328 Compare August 29, 2019 11:52
@abernix abernix merged commit c758ebd into master Aug 29, 2019
@abernix abernix deleted the abernix/express-types-fix-maybe-3222 branch August 29, 2019 12:31
@abernix abernix added this to the Release 2.9.2 milestone Aug 29, 2019
@CallMeLaNN
Copy link

@abernix Let me try this soon.

@CallMeLaNN
Copy link

@abernix it works on yarn v2. thanks.

@abernix abernix requested a review from trevor-scheer August 31, 2019 21:27
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 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

Successfully merging this pull request may close these issues.

2 participants