Graphql-tools: Declaring apollo-link as a dev dependency causes complaints in typescript

[alvis]

Caused by #445, the dependency is broken.

As apollo-link is now a dev dependency cause the following typescript error since it is not installed by default.

node_modules/graphql-tools/dist/stitching/introspectSchema.d.ts(2,28): error TS2307: Cannot find module 'apollo-link'.
node_modules/graphql-tools/dist/stitching/makeRemoteExecutableSchema.d.ts(1,28): error TS2307: Cannot find module 'apollo-link'.

[tjpeden]

Adding apollo-link as a dev dependency (suggested in #424) resolves the above error, but causes the following typescript error in it's place.

node_modules/apollo-link/lib/linkUtils.d.ts (1,23): error TS2688: Cannot find type definition file for 'zen-observable'.
node_modules/apollo-link/lib/types.d.ts (1,23): error TS2688: Cannot find type definition file for 'zen-observable'.
node_modules/apollo-link/lib/link.d.ts (1,23): error TS2688: Cannot find type definition file for 'zen-observable'.

EDIT:
Adding @types/zen-observable to dev dependencies is a workaround. There's already an issue for the above error: apollographql/apollo-link#163

Full workaround: npm i --save-dev apollo-link @types/zen-observable

[alvis]

@tjpeden: The @types/zen-observable issue should be fixed in the coming version of apollo-link as apollographql/apollo-link#171 has been merged.

However, adding apollo-link as a dev dependency of graphql-tools is still not ideal. This should be fixed. But without a clear understanding of why the original change in #445 makes Launchpad work again, a PR may not be wise.

@stubailo: any thought?

[stubailo]

So basically, we had to work around with a problem in Launchpad with peer dependencies. As a result, we made this package no longer have a direct dependency on apollo-link. The dev dependency is because we use some of the types from it, as you have noticed.

I think the right solution is to fix the type issues, but not re-introduce a dependency on the library. Any ideas?

[stubailo]

If someone could jump in with a PR that would be much appreciated!

[alvis]

@stubailo do you mean that apollo-link cannot be installed alongside with `launchpad?

What is the exact problem? so that we can try to fix it?

[kvnvelasco]

Hello! (new person here, affected by this issue).
Looks like we need a @types/apollo-link.

if that makes sense i'll write a pr in a day or two.

[alvis]

But apollo-link itself is written in ts. Making an @types package doesn't sound to be a good approach.

ping @stubailo

[stubailo]

Hi! I don't think we need @types/apollo-link since the types are already in the package.

I think options are:

1. TypeScript users should install the package even though it's not a dep (99% sure it would work)
2. We can make it a direct dependency, only drawback is increased bundle size for people using it on the client

I think (2) is not so bad maybe?

[corydeppen]

Based on the guidance in the Dependencies section of the TypeScript docs, I think apollo-link should be listed in dependencies.

[alvis]

I think it should be declared as a direct dependency. After all, the coupling between apollo-link and graphql-tools is not weak. In fact, some logic even seems to be copied from apollo-link. For example, the code below
https://github.com/apollographql/graphql-tools/blob/cc69bb1556893c9828448470036d02932e3b01a8/src/stitching/linkToFetcher.ts#L26 is taken from apollo-link/src/linkUtils.ts. They should be reused by importing in my opinion.

[kbrandwijk]

There are more cases where I use the parts from graphql-tools that do not depend on apollo-link then the cases where I do. Having to add apollo-link as dependency on my own project doesn't make any sense in those cases. I also think graphql-tools should add it as a direct dependency.

[stubailo]

Can someone send a PR adding the dependency? Since this project is primarily used on the server I don't think it will be a big deal to add it.