-
-
Notifications
You must be signed in to change notification settings - Fork 538
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 peer dependency #697
Comments
Please note that it’s also possible to use this package without the TypeScript compiler. It sounds like this feature will be broken in the future. I’m definitely happy to change this and kill that feature for the next major release. |
Do you have an example of the behavior expected in such a case? I think peer dependencies shouldn't affect this much - at worst a warning will be displayed when TypeScript is missing, but functionally speaking it should be the same. We're actually discussing a similar use case (optional peer dependencies) at yarnpkg/yarn#6487 |
It’s the |
So basically it's now impossible to have custom transformers using custom compilers like ttypescript? |
That does seem like a problem. What can be done to support that use case? I can certainly add the flag back for you before next release. Does your tooling wrap the existing compiler or use transforms? Maybe there’s a different feature we can expose for you? |
Basically, what Webpack loaders and others have support for it, the problem is with node. I guess it's easier to restore the flag rather than reimplementing support for custom transformers in ts-node. |
@19majkel94 Both are easy enough, I'd just look at keeping it as also a peer dependency. Hopefully that'll be ok for everyone all round. I'll reopen this to revert and make that change. |
Note that it doesn't have to be a peer dependency provided the resolution is done by the owner of the compiler. If ts-node was to call |
That’s great to know, thank you. It sounds like the optimal balance here. |
Previous discussion here: #315
The
typescript
package isn't listed as a peer dependency ofts-node
. It kinda works because the hoisting breaks boundaries between packages, allowing such require calls, but it's unsafe and relies on an undefined behavior. Most package managers start to make a move against such patterns, so it'd be best to fix it 🙂@blakeembrey mentionned that a possible problem with peer dependencies was that it would require "updating the peer dependency every single nightly release to account for prereleases in the range". It's only partially true:
The range specified in
peerDependencies
in only an hint used by package managers to warn the user when they install an incompatible version. It's not a hard requirement though, and no package manager will see it as an error to use an "incompatible" version.The problem if there's one is a global problem and should be solved at a global scale (either in the semver standard or in the way the package managers interpret the
peerDependencies
constraints with regard to prereleases). Such workarounds, while useful to solve a given problem in a short timeframe, solve it at the wrong layer and may cause problems (as is the case here).Note that we already slightly changed the semantic of the
engines
check in Yarn to also account for prereleases (starting from 0.26 iirc). I think we could do that with peer dependencies as well 🤔The text was updated successfully, but these errors were encountered: