-
-
Notifications
You must be signed in to change notification settings - Fork 431
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
TypeScript should be a peerdependency #839
Comments
I'm open to this but I'm not sure how feasible it is. We build ts-loader with a specific version of TypeScript (latest and greatest usually) so we have a high version devDependency. But typically TypeScript can be used with a whole number of different versions of TypeScript. I'm not sure we can have it listed as a peer dependency as well as a devDependency with different versions. BTW yarn is fine; I can say that as that's what I use! |
It shouldn't be an issue semantically speaking - {
"devDependencies": {
"typescript": "latest"
},
"peerDependencies": {
"typescript": "*"
}
}
Glad to hear that! I'm actually one of Yarn's maintainers 😄 We've recently unveiled the Plug'n'Play project, which will (optionally for now) enforce strict boundaries between packages (à la pnpm) and will allow the installs to become much faster and safer. It might cause some issues when packages don't list their whole set of dependency, hence my report 🙂 |
Would you like to submit a PR that adds I'll take it for a whirl. If it works without problems then I'd be happy to merge it 😄 |
We're using the same strategy for some private packages at work. It's perfectly fine to specify something in both |
#841 adds the dependency to the |
Expected Behaviour
ts-loader
shouldn't rely on undefined behaviorsActual Behaviour
ts-loader
currently relies on the hoisting in order to have access to thetypescript
packageThe fix is pretty simple:
typescript
just has to be listed as apeerDependencies
ofts-loader
(rather than just adevDependencies
). This will guarantee that it will then get the exact same instance than its parent. It can be listed as both if you want to also install it by default on dev environments.Steps to Reproduce the Problem
It causes issues with package trees that do not support hoisting, such as Yarn Plug'n'Play.
Location of a Minimal Repository that Demonstrates the Issue.
Because Plug'n'Play has a fallback mechanism, hitting this case is a bit more difficult to reproduce - but basically, if you have a workspace that depends on both
ts-loader
andtypescript
, it won't work (becausets-loader
won't be allowed to accesstypescript
as it's not one of its dependencies, and because the fallback won't kick in sincetypescript
will be in a workspace rather than the top-level module).The text was updated successfully, but these errors were encountered: