Please move @types references to devDependencies

[paralin]

I'm getting the following conflict right now:

Error at myproject/node_modules/@types/express-serve-static-core/index.d.ts:32:10: Duplicate identifier 'PathParams'.
Error at myproject/node_modules/@types/express-serve-static-core/index.d.ts:34:10: Duplicate identifier 'RequestHandlerParams'.
Error at myproject/node_modules/apollo-server/node_modules/@types/express-serve-static-core/index.d.ts:32:10: Duplicate identifier 'PathParams'.
Error at myproject/node_modules/apollo-server/node_modules/@types/express-serve-static-core/index.d.ts:34:10: Duplicate identifier 'RequestHandlerParams'.
Error at myproject/node_modules/apollo-server/node_modules/@types/express-serve-static-core/index.d.ts:158:9: Subsequent variable declarations must have the same type.  Variable 'expires' must be of type 'Date', but here has type 'boolean | Date'.
Error at myproject/node_modules/apollo-server/node_modules/@types/express-serve-static-core/index.d.ts:162:9: Subsequent variable declarations must have the same type.  Variable 'secure' must be of type 'boolean', but here has type 'boolean | "auto"'.

Please make the @types references be devDependencies so that they do not get installed in node_modules when installing graphql-server.

[paralin]

Nevermind, after upgrading to the graphql-server- naming scheme it seems OK.

[stubailo]

Thanks for reporting and resolving!

[despairblue]

This is still an issue. Would you be open to a PR that moves the @types dependencies to dev dependencies?

The version of @types/express-serve-static-core that apollo-server-express depends on is incompatible with the version of @types/express I have installed:

../../node_modules/@types/express/index.d.ts:99:42 - error TS2344: Type 'P' does not satisfy the constraint 'Params'.
  Type 'P' is not assignable to type 'ParamsArray'.

99         extends core.ErrorRequestHandler<P, ResBody, ReqBody, ReqQuery> { }
                                            ~

../../node_modules/@types/express/index.d.ts:108:124 - error TS2344: Type 'P' does not satisfy the constraint 'Params'.
  Type 'P' is not assignable to type 'ParamsArray'.

108     interface Request<P = core.ParamsDictionary, ResBody = any, ReqBody = any, ReqQuery = core.Query> extends core.Request<P, ResBody, ReqBody, ReqQuery> { }
                                                                                                                               ~

../../node_modules/@types/express/index.d.ts:109:138 - error TS2344: Type 'P' does not satisfy the constraint 'Params'.
  Type 'P' is not assignable to type 'ParamsArray'.

109     interface RequestHandler<P = core.ParamsDictionary, ResBody = any, ReqBody = any, ReqQuery = core.Query> extends core.RequestHandler<P, ResBody, ReqBody, ReqQuery> { }
                                                                                                                                             ~


Found 3 errors.

Using a resolution fixes this. But @types shouldn't be in the dependencies to be honest. Especially not with a fixed version number. It prevents me from using a newer version as well as clashing if I use another package that depends on the same types package but with another version.

  "resolutions": {
    "@types/express-serve-static-core": "4.17.13"
  },

[abernix]

@despairblue:

There have been problems with these particular types, generally speaking (DefinitelyTyped/DefinitelyTyped#40905). If this problem persists for you, please let us know, though we have tried multiple times to get this right as you'll note in the apollo-server-related commits on that PR and #4493 as well).

Generally speaking though, it is simply not sufficient to merely say that @types/ should not be in dependencies. If a generated type declaration file itself uses import type { Type } from 'module', the module must be in dependencies so that it will be installed (by package managers) when the package is installed, allowing TypeScript consumers to use it without having to top-level install every @types/ package that are used in their project's dependencies. If generated types do not import other @types/ packages in the emitted declaration files, and are instead only used for local development on the library, then they can be safely in devDependencies as we do with most of the @types/ dependencies in this repository.

[despairblue]

@abernix Right. If the types are reexported they need to be in the dependencies. But doesn't that sound like peerDeps? Like if I use different react components they won't depend on react but instead have react as a peer dep so I can choose what version of react I'm running. Same with packages that use singletons or share classes from a library like packages depending on prom client, mongoose or mongodb.

That way the user of apollo still has to install them manually, but npm and yarn will let them know. And the concept of peerDeps is well understood on the ecosystem (hopefully).

What do you think?

[abernix]

Perhaps! We won't change patterns right now, but once the peer dependencies are (again) automatically installed in npm 7 (which is not yet latest, but is coming along nicely) we can re-consider. I also believe it would be substantially disruptive to projects to merely change this right now (without a major version bump), so this is probably an Apollo Server 3.x consideration.

(Worth noting that, with the exception of @types/express-serve-static-core, this pattern has worked pretty darn well, all things considered — though I do acknowledge it can be better, and might be in the future!)