Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

Allow type to have peerDependencies instead of dependencies. #431

Open
npirotte opened this issue Feb 1, 2018 · 12 comments
Open

Allow type to have peerDependencies instead of dependencies. #431

npirotte opened this issue Feb 1, 2018 · 12 comments

Comments

@npirotte
Copy link

npirotte commented Feb 1, 2018

Currently, when a type is published, all imported types are added as dependencies.

Example: react-router v3:

dependencies": {
        "@types/history": "^3",
        "@types/react": "*"
}

But as react is a peerDependency of react-router, you can easily have an application with react@15 and react-router.

In that case definitions are broken because npm install react@16 as child of react-router.

definitions dependencies should strictly follow what is defined is the module package.json. If a dependency is a dependency, type dependency should be a type dependency, in dependency is a peerDependency, type dependency should be a peerDependency.

@freeman
Copy link

freeman commented Feb 2, 2018

Absolutely agree. This makes it a pain for example to use react components with preact when using typescript.

@chriskrycho
Copy link

It is also possible for these kinds of inherent dependencies to intersect in nasty ways with npm and yarn's resolution algorithms and preference (given a lockfile) not to update nested dependencies (cf. #360). In our own app, for example, we have a direct dependency on @types/ember which specifies we want the latest available. Another setup of types we use also depends on @types/ember but with "*" as its resolution.

Yarn tries to preserve whatever it had previously installed there, so we end up with two separate copies of @types/ember, which in turn causes TS to complain about redeclarations (as you'd expect, since that's what it sees).

@RyanCavanaugh
Copy link
Member

It's a bit of a pickle because we have no way to validate, on the DT side, that someone's PR doesn't break some combination of peerDependency satisfactors. We can't plausibly fetch every combination in the matrix of valid peer dependencies' published versions and ensure that they aren't breaking someone somewhere.

@chriskrycho
Copy link

Totally makes sense. If a type explicitly sets a dependency in its own package.json, does that override (and therefore could that be a solve)? E.g. in this case I could probably fix this by making the ember-feature-flags depend explicitly on "@types/ember": "latest" and I think that might resolve it?

@npirotte
Copy link
Author

Can't this be automated like the dependencies are?

I had a simple idea:
You can just npm info {moduleName} and compare the original module dependencies and the dependecy list extracted from typing.

If one of these extracted dep is peer in the original module, add them as peer instead of dependency :)

@RyanCavanaugh
Copy link
Member

@andy-ms opinions on the above? Seems reasonable but I haven't slept much 🙃

@ghost
Copy link

ghost commented Feb 12, 2018

"peerDependencies" would mean that people would have to remember to install those, while "dependencies" are installed automatically. Not a big deal since npm gives a warning, but we might want to update typingsInstaller to look for peerDependencies and install those too.
Not a big fan of doing npm info on the real module since it would double the number of npm requests we normally have to do -- would be better to provide a way to explicitly request peerDependencies.

@npirotte
Copy link
Author

Most of the time, when somebody install something like @types/react-redux, @types/react is already installed. That's the point of peerDependencies :) If package developer choose to have a peerDependency instead of a dependency there must be a reason a typing package should respect that...

But yeah, I agree that npm info is not the best way...

@renewooller
Copy link

renewooller commented Mar 21, 2018

This issue is causing a lot of pain at the moment. All of my definitely typed libs are currently reporting "Conflicting definitions for 'node'" because the version of node has automatically incremented to 9.*, due to the @types/node : "*", while my project is still on 8.*.

Any idea or work arounds appreciated. Current 'work around' is to copy the declaration files into the project and reference them directly and remove the @types/* dependency from package.json

UPDATE: this appears to have been primarily caused by an incorrectly configured typerRoots.

@joma74
Copy link

joma74 commented Jun 11, 2018

My case is targeting webpack 3, hence including
"@types/webpack": "~3" in my dev-dependencies.

But as soon I install "@types/webpack-dev-server": "^2.9.4" with a generated package.json of
"dependencies": { "@types/webpack": "*",
i get

node_modules/@types/webpack-dev-server/
└── node_modules
    ├── source-map
    │   ├── dist
    │   └── lib
    └── @types
        ├── tapable
        └── webpack

where @types/webpack is listed as "version": "4.4.0".

@EdwardDrapkin
Copy link

It's a bit of a pickle because we have no way to validate, on the DT side, that someone's PR doesn't break some combination of peerDependency satisfactors. We can't plausibly fetch every combination in the matrix of valid peer dependencies' published versions and ensure that they aren't breaking someone somewhere.

Right now it looks a lot, from the user's perspective, like there are two bad choices: knowingly break some projects where the "dependencies" behavior causes issues, or un-knowingly break some other projects where the "peerDependencies" behavior causes issues. I feel like the latter is a much better situation for users of TS because there are avenues for fixing the issue: don't install the peerDependencies and ignore the incorrect PD warnings from yarn/npm or PR the .d.ts file and fix the issue. Right now, we have to rely on elaborate compiler configuration changes to get our stuff in a working order... simply ignoring peer dependencies would be a lot less of a burden.

So while you're correct that you can't 100% guarantee that packages' peerDependencies don't create an impossible dependency graph, you can 100% guarantee that the current behavior does cause impossible dependency graphs, and it seems like this is a case of perfect being the enemy of good and letting bad win.

@SamB
Copy link

SamB commented Jun 7, 2019

Yeah, it sure seems like any plugin is going to need to list the host's typings in peerDependencies, or else users are going to end up with multiple versions of the host's typings such that the TypeScript compiler yells at them that import("/project/node_modules/foo").Foo is missing some properties compared to import("/project/node_modules/foo-bar/node_modules/foo").Foo when they actually try to wire up the plugin in their foofile.js / foofile.ts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants