Move type dependencies as dev deps

[rricard]

Attempt to fix #1061

TODO:

• If this PR is a new feature, reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass
• Update CHANGELOG.md with your change
• Add your name and email to the AUTHORS file (optional)
• If this was a change that affects the external API, update the docs and post a link to the PR in the discussion

[schickling]

Seems to be also related to: #1041

[helfer]

Hm, I feel like we're going in circles with Typescript here... #733 #949. Is there really no way in Typescript to have projects just work and download the correct typings when they're installed, but also work in plain JavaScript without requiring the typings?

And is it really the case that if you use typings for graphql and other things, none of your dependencies can use different typings for those packages without getting you into a world of pain? Is it just me or is this a huge drawback of using Typescript?

Anyway, trying to be a bit more productive: if we merge this PR, will people get warnings about missing typings and be required to install them by hand? Are we really fixing anything here, or are we just choosing the less bad of two possible solutions?

[schickling]

I'm totally on board with your frustration, Jonas. This is super painful and definitely a big issue when using Typescript currently. We're having similar problems for @types/react as they are still idling at version 14.

@rricard strongly encouraged us to consider switching over to flow. What's your stand on this @helfer?

[rricard]

@schickling again, no, you seem to already master the TS compilation chain already, mastering flow will be painful for you. We need to clearly define what to expect from our package and whatnot. So my take on this would be to import less and document... Or eventually, only add to optionalDeps the things required by the typedef file. I clearly don't know at this point, @schickling can you try removing all of your custom definitions? As stated before, it works on an isolated example: https://github.com/apollostack/typed-graphql-client-example/tree/react. Maybe if you can find a minimal example breaking things as we talked yesterday...

Apart from that, I'm currently in the mountains with a weak internet connection so I doubt I can help you very much this week... I agree though we shouldn't merge that yet...

[schickling]

I think the issue as @helfer said is overlapping type definitions (possibly the graphql definitions).

Any ideas on how to ignore/workaround that?

[basarat]

And is it really the case that if you use typings for graphql and other things, none of your dependencies can use different typings for those packages without getting you into a world of pain? Is it just me or is this a huge drawback of using Typescript

Use skipLibChecks. That silences any .d.ts errors. But its bandaid use only. Suppressed errors have a way of resurficing 🌹

[schickling]

Thanks for your comment @basarat. Do you have any idea how to tackle this problem more at its core?

[basarat]

Do you have any idea how to tackle this problem more at its core?

to help me figure out, can you point out how flowtype would be different technically in handling the situation 🌹

[rricard]

@basarat Flow is still beginning in the "how to handle a library" field. For now, you pretty much control everything, which is good but requires more setup. What you do is point to directories containing typedefs in the config file. To makes things a bit easy though, the provide a DefinitelyTyped-like called flow-typed (much smaller though...). But one thing flow doesn't have to handle, since typing is optional per file (with the @flow pragma), is to be forced to type (or define as any) a library: if a file/module does not have the pragma, the exported type will pretty much be seen as any. Which makes it easier for libraries without typedefs out of the box... This avoids some dual definitions of lib we're seeing in the issue #1061. Moreover, in a project, I can choose where to get my definitions and I can choose to ignore one if needed.

Typescript on the other hand wants you to explicitly type everything, which can be good but will prevent you from compiling in some situations like this one. So I think there is no clear answer to that yet, if TS had a precedence system letting you choose which typedef (from node_modules, or from my typings file?) to use, that would help maybe... But the truth is, sometimes things clash and unfortunately, this affects libs like this one that try to be a good TS citizen but will expose too few or too much and ruin the developer experience at the same time.

[rricard]

@basarat, So my main question is more meta maybe:

How to be a good library exposing its TS typedefs?

That's what Apollo tries to be and I admit I don't have the experience to do figure that out correctly by myself. But I think there can be a set of rules/conventions, TS libraries could apply to be good TS-ready libraries, defining them in a manifesto or even just a blogpost could be useful to us and I would be really grateful if someone did that. Because even if I'm just toying with TS, knowing how to expose correctly my typedefs could be beneficial when writing libraries...

[basarat]

But one thing flow doesn't have to handle, since typing is optional per file (with the @flow pragma), is to be forced to type (or define as any) a library: if a file/module does not have the pragma, the exported type will pretty much be seen as any.

TypeScript supports the same in nightly now : microsoft/TypeScript#12358 anything that is in node_modules is automatically requireable and is inferred to be any. No need to do declare module or allowJs ;) 🌹

Because even if I'm just toying with TS, knowing how to expose correctly my typedefs could be beneficial when writing libraries

If the project is in typescript it should just use typings field in package.json pointing to the generated declaration files (declaration:true in tsconfig.json). See examples :

• TypeStyle : https://github.com/typestyle/typestyle/blob/c2bf868e907a74b89349f3a0d8a7301b799c9412/package.json#L6 which contains both the .js and .d.ts generated by the tsconfig.json : https://github.com/typestyle/typestyle/blob/c2bf868e907a74b89349f3a0d8a7301b799c9412/tsconfig.json#L8-L10

• FreeStyle https://github.com/blakeembrey/free-style/blob/f6ca5630ff73f22cf6bb5f12fccd7b33209b710e/package.json#L6 + https://github.com/blakeembrey/free-style/blob/f6ca5630ff73f22cf6bb5f12fccd7b33209b710e/tsconfig.json#L4-L6

If your project depends on external global things like node, you put them in devDependencies and mention that typescript users should install type definitions for node their project e.g. npm install @types/node. This is similar to how you would guide them to setup other TypeScript options that you require for them to be able to work with your code e.g. experimentalDecorations: true or lib option etc : https://basarat.gitbooks.io/typescript/content/docs/types/lib.d.ts.html#lib-option

Hope this helps ❤️

[DanielRosenwasser]

In general the advice we've given to people is to put their declaration files into dependencies and devDependencies. Here's the way I think about it:

1. If your library doesn't have an API (i.e. it's just a simple executable or web app), then all your declaration files should be devDependencies.
2. If your library does have an API, then for any declaration file it consumes:
   1. If it's a global dependency (e.g. Node), put it in devDependencies - this depends on the user's runtime environment and must be self-selected.
   2. If the public-facing API uses/exposes types from that declaration file, it should be under dependencies.
   3. Otherwise, it should be in devDependencies.

One potential option is to list your declarations as devDependencies and give special instructions to your TypeScript consumers to install the appropriate packages. This kind of makes it a a pain for TS users.

Finally, you can also just publish your declarations to DefinitelyTyped and thus @types itself.

Note: One thing that we'll be doing down the road in DefinitelyTyped is to have "redirects" (which is probably just going to be a submodule within DefinitelyTyped). You might be able to take advantage of that down the road.

---

While I understand that there is a little bit of hesitance to have @types packages ship to JS consumers,

1. These packages are relatively small, and package managers like Yarn make managing them even faster.
2. They don't affect bundle-sizes anyway because they don't export values.
3. JavaScript users win anyway because editors like VS Code, VS, and WebStorm can all provide useful experiences based on declaration files.

Requiring an out-of-band package manager is one of the most exhausting, frustrating, confusing, and time-consuming things you can do for new TypeScript users. Our community has gone through two package managers, and the overhead for new users is surprisingly staggering. What we've found is that overlaying TypeScript on top of existing/familiar concepts in the JavaScript world has been the most user-friendly option.

[schickling]

Thanks a lot for all these constructive and insightful comments. I think apollo-client has a lot of potential to improve in that regards in order to make usage for Typescript developers more enjoyable.

I've pinged @blakeembrey about this and got some helpful pointers how to resolve the underlying problem here:

anyway, in general, that pr is not needed - stick with what you had and just improve the definitions you’re using

for instance, https://github.com/nitintutlani/typed-graphql/blob/master/graphql.d.ts#L1-L2 is written in global mode (declare module "") and does conflict with other usages - so avoid that definition and tidy up to use all external module definitions from @types (making prs to DefinitelyTyped, if needed)

So if I'm not mistaken then #1041 might be able to resolve this? (/cc @stubailo)

[rricard]

@basarat & @DanielRosenwasser thank you for this information. I'll let @stubailo & @helfer tell me what they prefer in the solutions you gave me and I'll get to work. But I think, seeing what's happening here that sending the typedefs to DefinitelyTyped may be the best option, I would just wait though for now to see if typed-graphql changed to @types/graphql fixes the issue @schickling is having...

[helfer]

Thanks for the advice @DanielRosenwasser!

@rricard let's first make sure we use @types/graphql instead of typed-graphql, and then move global dependencies like @types/node and anything that we don't transitively export to devDependencies and see how that works.

@schickling do you have a simple setup that could help us test whether we've solved the problems for typescript developers as we implement these changes?

[schickling]

The dev branch of our homepage repo should do the trick: https://github.com/graphcool/homepage (However it's not necessarily simple.)

[helfer]

@rricard @schickling most of this should be implemented with this commit. Once I release 0.7 (in a day or two) we can test it!