Skip to content

TypeScript should be a peer dependency #697

Closed

Description

@arcanis

Previous discussion here: #315

The typescript package isn't listed as a peer dependency of ts-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 🤔

Activity

  1. blakeembrey commented on Oct 7, 2018

    @blakeembrey
    Member

    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.

  2. arcanis commented on Oct 7, 2018

    @arcanis
    Author

    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

  3. blakeembrey commented on Oct 7, 2018

    @blakeembrey
    Member

    It’s the --compiler option. It defaults to typescript, but can be anything. In general this feature is probably unused by the majority (especially since TypeScript has native nightly support), which is why I’m happy to remove it. It seems like this package being able to resolve any other will be going away, so best to remove this before there’s too much confusion.

  4. MichalLytek commented on Oct 11, 2018

    @MichalLytek

    So basically it's now impossible to have custom transformers using custom compilers like ttypescript?
    https://github.com/cevek/ttypescript#ts-node

  5. blakeembrey commented on Oct 11, 2018

    @blakeembrey
    Member

    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?

  6. MichalLytek commented on Oct 12, 2018

    @MichalLytek

    Basically, what ttypescript does is wrapping the tsc compiler to apply custom plugins/transformers that are supported by TypeScript only programmatically, like ts-transform-graphql-tag or ts-nameof.
    See microsoft/TypeScript#14419 for more info why.

    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.

  7. blakeembrey commented on Oct 13, 2018

    @blakeembrey
    Member

    @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.

  8. arcanis commented on Oct 13, 2018

    @arcanis
    Author

    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 require.resolve with a paths option that would only contain the cwd, it should then be able to pass the result to require without issues (the resolution would then be done against the dependencies of the package owning the cwd, not the dependencies of ts-node itself).

  9. blakeembrey commented on Oct 13, 2018

    @blakeembrey
    Member

    That’s great to know, thank you. It sounds like the optimal balance here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions