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

[apollo-express-server] Missing dependency on express #3238

Closed
mwaldstein opened this issue Aug 31, 2019 · 0 comments · Fixed by #3239
Closed

[apollo-express-server] Missing dependency on express #3238

mwaldstein opened this issue Aug 31, 2019 · 0 comments · Fixed by #3239
Milestone

Comments

@mwaldstein
Copy link

apollo-express-server has a dependency on express to run (see

) but it isn't listed in the dependencies of its package.json -

Typically, this shouldn't cause issues as you'd only be using it in an express-based project, but if you aren't directly including express but instead using a private wrapper package which adds boilerplate, npm won't know to make express available to apollo-express-server, throwing the error:

internal/modules/cjs/loader.js:584                                                                                                                                                     throw err;
    ^

Error: Cannot find module 'express'

The solution is to add express to either the dependencies or peerDependencies of apollo-express-server.

abernix added a commit that referenced this issue Aug 31, 2019
The [literal `import`-ing of `express` in `ApolloServer.ts`][1] isn't new
but, prior to #3047, it had only been a typing dependency — not an actual
runtime dependency — since the TypeScript compiler doesn't emit `require`s
when only types are utilized.

To see this first hand, see [the emitted `dist/ApolloServer.js` in
`apollo-server-express@2.8.2`][2] compared to [the same file in
`apollo-server-express@2.9.0`][3].  (Hard to decipher, but check
around like 15 and search for `"express"` in the second link.)

Now that we actually utilize `express.Router` to build the composition of
middleware in #3047 , it's true that `express` is no longer just a type
dependency, but does warrant being a regular dependency as well!

Since this has never been the case before during the entirety of the v2
line, I'm a bit concerned about introducing it now, but it seems other
integrations like `apollo-server-koa` already have similar requirements
going back to its introduction in [#1282][4]

https://github.com/apollographql/apollo-server/blob/92ea402a90bf9817c9b887707abbd77dcf5edcb4/packages/apollo-server-koa/package.json#L41

Furthermore, we already have similar direct dependencies on other packages
which we use directly, like [`cors`][5] and [`body-parser`][6]:

https://github.com/apollographql/apollo-server/blob/ff3af66a0f3c63bfb056feca82fc7e7b7592b4a5/packages/apollo-server-express/package.json

If this turns out to be problematic, we could consider declaring it only in
`peerDependencies`, but those come with their own [share of confusion][7].

[1]: https://github.com/apollographql/apollo-server/blob/6d9c3b8c97/packages/apollo-server-express/src/ApolloServer.ts#L1
[2]: https://unpkg.com/apollo-server-express@2.8.2/dist/ApolloServer.js
[3]: https://unpkg.com/apollo-server-express@2.9.0/dist/ApolloServer.js
[4]: #1282:
[5]: https://npm.im/cors
[6]: https://npm.im/body-parser
[7]: npm/rfcs#43
Fixes: #3238
abernix added a commit that referenced this issue Aug 31, 2019
The [literal `import`-ing of `express` in `ApolloServer.ts`][1] isn't new
but, prior to #3047, it had only been a typing dependency — not an actual
runtime dependency — since the TypeScript compiler doesn't emit `require`s
when only types are utilized.

To see this first hand, see [the emitted `dist/ApolloServer.js` in
`apollo-server-express@2.8.2`][2] compared to [the same file in
`apollo-server-express@2.9.0`][3].  (Hard to decipher, but check
around like 15 and search for `"express"` in the second link.)

Now that we actually utilize `express.Router` to build the composition of
middleware in #3047 , it's true that `express` is no longer just a type
dependency, but does warrant being a regular dependency as well!

Since this has never been the case before during the entirety of the v2
line, I'm a bit concerned about introducing it now, but it seems other
integrations like `apollo-server-koa` already have similar requirements
going back to its introduction in [#1282][4]

https://github.com/apollographql/apollo-server/blob/92ea402a90bf9817c9b887707abbd77dcf5edcb4/packages/apollo-server-koa/package.json#L41

Furthermore, we already have similar direct dependencies on other packages
which we use directly, like [`cors`][5] and [`body-parser`][6]:

https://github.com/apollographql/apollo-server/blob/ff3af66a0f3c63bfb056feca82fc7e7b7592b4a5/packages/apollo-server-express/package.json

If this turns out to be problematic, we could consider declaring it only in
`peerDependencies`, but those come with their own [share of confusion][7].

[1]: https://github.com/apollographql/apollo-server/blob/6d9c3b8c97/packages/apollo-server-express/src/ApolloServer.ts#L1
[2]: https://unpkg.com/apollo-server-express@2.8.2/dist/ApolloServer.js
[3]: https://unpkg.com/apollo-server-express@2.9.0/dist/ApolloServer.js
[4]: #1282:
[5]: https://npm.im/cors
[6]: https://npm.im/body-parser
[7]: npm/rfcs#43
Fixes: #3238
@abernix abernix added this to the Release 2.9.x milestone Sep 1, 2019
abernix added a commit that referenced this issue Sep 1, 2019
@abernix abernix modified the milestones: Release 2.9.x, Release 2.9.3 Sep 1, 2019
@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

Successfully merging a pull request may close this issue.

2 participants