Skip to content

Prevent dependency hell: export dependencies from DefinitelyTyped as peerDependencies #11671

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

Closed
nicojs opened this issue Oct 16, 2016 · 16 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@nicojs
Copy link

nicojs commented Oct 16, 2016

Not sure where i can post issues on the current export of DefinitelyTyped repo to the @types npm space, please point me to the correct place.

Right now, the @types are populated using the content in the types-2.0 branche on DefinitelyTyped. An excellent choice! Open source is awesome 😄

The problem is that all dependencies marked with /// <reference types="..." /> are exported as dependencies of the package. This becomes a problem when you have multiple dependencies on one single module.

Say for example: i want type definitions for @types/express-serve-static-core and @types/glob. Both right now mark @types/node as a dependency. When using npm 2.x i immediately get these duplicate identifier errors: error TS2300: Duplicate identifier 'BufferEncoding'. When i use npm 3.x, i get some more leeway, because of the flat folder structure, but when one of the modules would require a different version of @types/node, i would again get this error. Effectively: dependency hell.

The problem would be fixed if dependencies would be marked as peerDependencies. I understand that it can be some more work for the developer who wants to use the typings, but it would effectively solve this issue. At least i would want it for modules that define variables on the global scope.

See for example of a failing build see https://travis-ci.org/stryker-mutator/stryker/jobs/168113675

@blakeembrey
Copy link
Contributor

blakeembrey commented Oct 16, 2016

This wouldn't be a very good long-term fix and only ever works if every single dependency uses the same version of peer dependency which is difficult in production. If this is a occurring, it seems like the better solution would be to improve how TypeScript is resolving dependent typings instead. For example, you're only considering the use-case of one location (DefinitelyTyped) which is centralised and can somewhat achieve this (in fact, it's relied on this poor behaviour for a very long time). What about other people that want to publish types outside of DT? What about module authors too? This would rely on every module author also using peerDependencies. However, even that breaks down because I don't want to see your peerDependencies warnings when I might not use TypeScript. Additionally, peerDependencies aren't automatically installed either. The behaviour in 2.x is to auto-install but fail hard on conflict - effectively killing anyone's ability to update a single definition and you'll land in dependency hell a lot quicker. The default 3.x behaviour is not to install them, and also not to fail as hard.

Edit: For more reference, the behaviour you propose is everything we tried to stop when I deprecated TSD in favour of Typings. Relying on behaviour like this results in tight coupling of every definition, and it becomes impossible for a development team to work properly as every dependency needs to be updated in unison and it assumes that every module will always be at the latest version. I ran into this myself numerous times when I first started using TypeScript, hence I wrote Typings 😄

@aluanhaddad
Copy link
Contributor

@blakeembrey I think you're absolutely right. peerDependencies is the opposite of the direction that we should be going in. What we need is, as I believe you hinted at, is for TypeScript to better understand and support multiple versions relative to transitive dependencies.

@nicojs
Copy link
Author

nicojs commented Oct 17, 2016

What we need is, as I believe you hinted at, is for TypeScript to better understand and support multiple versions relative to transitive dependencies.

If that fixes every usecase, i'm all for it 👍.

For the time being i would either like to see peerDependencies, or no dependencies at all. Some more clarification: I feel like dependencies should only be used if the library you are writing typings for is also dependent on a specific module. For example: when you need typings from node itself, you should just use whatever is available. In the Java world, we would call this dependency provided, because we actually rely on a runtime environment to deliver us this dependency.

The way i see it, peerDependency would come close to provided in java. I'm also for not having a dependency at all, just let the end user figure it out.

However, even that breaks down because I don't want to see your peerDependencies warnings when I might not use TypeScript.

This is why library authors should only depend on @typings using devDependencies. Only @types should ever depend on other @types imho. I have builds breaking because a library decides to depend on @typings stuff, this shouldn't be.

it only ever works if every single dependency uses the same version of peer dependency which is difficult in production.

Well, for the node example i would just prefer to use * to specify the version. Let the end user figure out which version of node it's running and use those typings. For other frameworks that do need a specific version and thus a specific version in the types, i don't have a clear solution.

Additionally, peerDependencies aren't automatically installed either

Correct. I would be in control. That's what i like about them :)

For more reference, the behaviour you propose is everything we tried to stop when I deprecated TSD in favour of Typings

Well... i was never a big fan of that decision ;).

@nicojs
Copy link
Author

nicojs commented Oct 17, 2016

BTW, this issue might be a duplicate of #8836

@blakeembrey
Copy link
Contributor

@nicojs That sounds absolutely terrible for an end-user. How do you expect any library author to publish a module if you're telling them "well, figure out the dependencies yourself". There's a reason we use package.json. You can not expect anyway, as a module author, to say "install this module and then install these type definitions to use TypeScript and good luck if my dependencies conflict with someone elses!"

If your gripe is with globals like node.d.ts, that's where I do agree - they should be peer dependencies. In fact, see my issue microsoft/types-publisher#107 which is about exactly this. This was always the behaviour of Typings too, for good reason. However, for regular dependencies, you can not expect that "figure it out yourself" will be successful.

As for your issues with Typings, I only ever saw one comment from you saying you wouldn't use it because I didn't name it TSD. The model and the need for it is really what pushes for native TypeScript support. If you're only ever developing a single application with few dependencies or a toolchain you completely control, it's easy to say the everything should be "the latest version" and it doesn't matter that it's tightly coupled. But this is the very first problem I run into using TypeScript, and that's what I knew I could fix and I'm trying to help suggest a better way to fix this too before you the proposal is just TSD over NPM.

@nicojs
Copy link
Author

nicojs commented Oct 17, 2016

That sounds absolutely terrible for an end-user. How do you expect any library author to publish a module if you're telling them "well, figure out the dependencies yourself".

Let me clarify: I think its better than having a dependency on node typings. Like i said, peerDependency would be preferable. You would still get an OK error because of the /// <reference types="node" /> for example in the typings file.

The model and the need for it is really what pushes for native TypeScript support

Isn't it true that TypeScript right now only supports one version per module? In that case i would like my transient dependencies to be flat and let me figure out more myself.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Oct 17, 2016

I don't think Typings was the right solution but I think it was heading in the right direction.

Requiring that the use your figure it out for themselves is equivalent to saying we're back to square one.

Isn't it true that TypeScript right now only supports one version per module? In that case i would like my transient dependencies to be flat and let me figure out more myself.

This is where things completely break down your proposal doesn't do anything to help with this.
There are tools out there, hint hint, that handle this problem without resorting to file system recursion.

In the Java world, we would call this dependency provided, because we actually rely on a runtime environment to deliver us this dependency.

The way i see it, peerDependency would come close to provided in java. I'm also for not having a dependency at all, just let the end user figure it out.

Java is not a good model to base any kind of dependency management system on. Need to run two versions of the Java Collections API side by side within your application to support your dependencies? Have fun doing that with weak names.

@tomitrescak
Copy link

Sooo, how can I solve problems mentioned above?

I need to use package that has "/// " in one of its .d.ts files. I am targeting ES6 so I cannot import that shim into my project as I will get myriads of duplicates. what would be the suggested way of dealing with these clashes?

@nicojs
Copy link
Author

nicojs commented Oct 17, 2016

@tomitrescak
Maybe you can take a look at the explanation on typeroots and types tsconfig properties, Hope that can solve your problem. What is the dependency with the /// in its .d.ts file?

@aluanhaddad I guess i wasn't thinking about such advanced scenario's as i haven't needed them in practice. (dep A needs B@1.0, dep C needs B@2.0). In that case, i guess there needs a change in TypeScript.

@blakeembrey happy to see you agree on environmental dependencies. That would solve i think 90% of my issues.

Do i keep this issue open in hope of a better TypeScript type resolution?

@tomitrescak
Copy link

tomitrescak commented Oct 17, 2016

@nicojs The mentioned types and typeroots do not work for me AT ALL. Even when I specify "types": ["node", "chai", "react"] in my tsconfig.json file I still get duplicates errors on compilation clashing between es6-shim and lib6.d.ts. Am using 2.1.0.dev.20161004. Here is my tsconfig.

The problematic file has a following line:

/// <reference types="es6-shim" />

@mhegazy
Copy link
Contributor

mhegazy commented Oct 17, 2016

The mentioned types and typeroots do not work for me AT ALL. Even when I specify "types": ["node", "chai", "react"] in my tsconfig.json file I still get duplicates errors on compilation clashing between es6-shim and lib6.d.ts. Am using 2.1.0.dev.20161004. Here is my tsconfig.

@tomitrescak none of the three files (node\index.d.ts, chai\index.d.ts and react\index.d.ts) reference es6-shim. what am i missing?

also definitions in your file are not only included from types. types lsit the global declarations you want to always be included, e.g. node, since you can not say import * from "node". but for anything else, the import is what brings it into your project, e.g. import .. from "typescript" will load "typescript" regardless if you have it in types or not.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 17, 2016

I should add that this line (/// <reference types="es6-shim" />) is wrong, and should be removed from the declaration file. feel free to send a PR to DT and i will merge it.

@tomitrescak
Copy link

@mhegazy the file that references es6-shim is the package file (reducer_extensions.d.ts) with following content:

/// <reference types="es6-shim" />
export declare function isQuery(action: any, name: String): boolean;
...

Yes, it is the (/// <reference types="es6-shim") that makes problems. And if removed all works as expected. So for now, I am manually removing it from the .d.ts file.

I have noticed that the "/// " is added automatically by typescript during compilation.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 21, 2016

I have noticed that the "/// " is added automatically by typescript during compilation.

where can i find the sources?

@sompylasar
Copy link

I've made a module called ts-build-tools that abstracts away TypeScript build procedures for many other TypeScript modules. It depends on @types/node, exports certain type definitions under ts-build-tools/src/index.d.ts that use @types/node, and is used to avoid installing common dependencies such as @types/node to each of the many modules that use it. The ts-build-tools also provides a common tsconfig.json for all the modules; the config uses typeRoots to pull common typings from ts-build-tools and additional ones from the modules that use it.

    "typeRoots": [
      "./node_modules/ts-build-tools/node_modules/@types",
      "./node_modules/@types"
    ]

Let some-ts-module be a module that depends on ts-build-tools to build itself with TypeScript.

If some-ts-module depends on an additional @types/some-typings-module that in turn depends on @types/node, TypeScript cannot determine which @types/node to use: the one from ts-build-tools or the one that some-ts-module's @types/some-typings-module depends on.

The resolution is ambiguous:

  • @types/node module starts resolving at some-ts-module/node_modules/ts-build-tools/src/index.d.ts and resolves to some-ts-module/node_modules/ts-build-tools/node_modules/@types/node – this prints the Module name '@types/node' was successfully resolved to message with --traceResolution.
  • but 'node' type definition starts at e.g. some-ts-module/node_modules/@types/request, so it gets to some-ts-module/node_modules/@types/node and never to some-ts-module/node_modules/ts-build-tools/node_modules/@types/node – this prints the Type reference directive 'node' was successfully resolved to message with --traceResolution.

This ambiguity leads to a ton of errors like error TS2300: Duplicate identifier 'BufferEncoding'. for every global identifier from @types/node.

I found the only workaround for now to be starting 'node' type definition resolution at some-ts-module/node_modules/ts-build-tools/node_modules/@types by ordering typeRoots so that the ./node_modules/ts-build-tools/node_modules/@types entry goes first, the ./node_modules/@types entry goes second. This means every other module will be resolved from ts-build-tools first, while the ultimate goal is to make some-ts-module override the dependencies provided by ts-build-tools.

Refs #8836
Refs unstubbable/custom-tslint-formatters#5

@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Needs Investigation This issue needs a team member to investigate its status. labels Nov 9, 2017
@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

8 participants