@types/mysql should be in devDepenencies

[gamliela]

@types inclusion should be optional. Use case: in our project we share code for client and server, and server types conflict with client types. Consider setInterval, which has different return type on Node and DOM.

Also, I think that this is the convention for many libraries, since TypeScript in general is optional.

[CodeFoodPixels]

Microsoft/types-publisher#81 states that they should be installed as a dependency so that it's installed without people having to mess around. To me this makes sense.

I don't see how mysql types will have any effect on your frontend types.

/cc @RetroCraft @move-zig because they added/changed the TypeScript stuff.

[move-zig]

There was a bit of a discussion about this on https://gitter.im/DefinitelyTyped/DefinitelyTyped from Nov 06 19:47 to Nov 07 08:23 (ET, I think). It seems like the consensus was a standard dependency, not devDependency. I think that makes sense.

[RetroCraft]

The idea is that @types dependencies should be installed when you install the package. When it was setup as just a devDep as before, you don't know you need to install it until you try and run/build/lint/whatever and it errors out saying that it can't find a definition for the mysql module.

[gamliela]

I understand the rationale for this change. Still, there is a reason why many libraries put it in devDependencies after all, and some of these reasons are given in the links you presented.

Here are my arguments:

1. TypeScript is not mandatory. By putting types in the dependencies section you signal that the types are essential and required part of the project, which is not the case.

2. It is easy to add @types for those who want it, but it is much harder to remove it for those who don't want it (like me). For me, it involves to write a specific postinstall script to deal with this problem.

3. There is a technical problem that affect (probably minority) of users. Some types are inferred incorrectly. See discussion at the end of this thread, and note that eventually they chose to put @types/node at devDependencies.

I will try to explain # 3 from our perspective. A variation of our problem is demonstrated also here.

We have sever code (Node) and client code (ES5) on the same project tree. Both share the same tsconfig file and both code parts get compiled together. Let's put aside the question whether it's a good setup or not; that's the way we work now and I assume we are not the only ones that work this way. In our client code we have expressions like setTimeout(...). We actually mean window.setTimeout(...) (doc) and therefore expect to get a number type as result. But because of the inclusion of @types/node (as side effect of @types/mysql inclusion) the TypeScript compiler automatically picks the node version of setTimeout, which returns a Timer object instead of number. So we ends up with TS2323: Type 'Timer' is not assignable to type 'number' error.

[CodeFoodPixels]

1. Typescript isn't mandatory and unless you actually try and use typescript, it has no real effect. Someone can still use ES5/ES6.
2. As @types/mysql is a dependency of the types in this library, they absolutely should live in dependencies. It defeats the point of having a dependency when users then have to install a separate package to make yours work, especially when it's not a dependency of their project, but a dependency of mine.
3. As mentioned, this is a minority of users. The dependencies they moved were dev-only dependencies, as can be seen in the changelog file in this commit: https://github.com/Polymer/dom5/commit/634d5bb69d7ce4d00ed93a06ab0e4023760036f2

Let's put aside the question whether it's a good setup or not

That's pretty key to this issue actually. If my library didn't uncover this problem, another one would have. The fact your workflow is broken because of a bad setup should have no bearing on the correct implementation of my node library.

Also, because there's an xkcd for everything:

[gamliela]

I never said our workflow is broken. It is, I agree, questionable. It's way too opinionated to disallow it completely. I also didn't claim that types are not dependencies. They are indeed. But there is a vague line between dependencies and devDependencies, and since types information are definitely not runtime dependencies one can argue that they are devDependencies.

[CodeFoodPixels]

As the maintainer of this library, I can take the liberty of being opinionated in working against questionable workflows. If I had to cater for every workflow ever, nothing would get done.

There isn't a vague line between devDependencies and dependencies, devDependencies are installed when you're doing development on the project itself (as in if you were to clone this repo and run npm install) and are not installed when you install a package into your project. As a library, this is an important differentiation.

On Tue, 14 Nov 2017, 15:04 Alon Gamliel, notifications@github.com wrote:

I never said our workflow is broken. It is, I agree, questionable. It's way too opinionated to disallow it completely. I also didn't claim that types are not dependencies. They are indeed. But there is vague line between dependencies and devDependencies, and since they are not runtime dependencies one can argue that they are devDependencies.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/lukeb-uk/node-promise-mysql/issues/72#issuecomment-344286907, or mute the thread https://github.com/notifications/unsubscribe-auth/AEjy1Gw9l_FmXKpcVsW_PMrC9y-NwiNmks5s2awCgaJpZM4Qb-JL .

[RetroCraft]

@gamliela Is there any reason why you can't write window.setTimeout? If you have Node code you should be including @types/node anyways, yes?

[gamliela]

@RetroCraft No specific reason. It's just a lot of places to change, and also the problem occurs in other variations (e.g. require function). And yes, we do have Node code, but since it sits together with non-Node code we are very picky about the types we throw in.