Skip to content
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

Add paths support #254

Closed
wants to merge 1 commit into from
Closed

Add paths support #254

wants to merge 1 commit into from

Conversation

jonaskello
Copy link
Contributor

Add support for executing typescript projects that uses paths in tsconfig. Discussed in #138 and #219.

It is achieved by patching node's Module._load function which is undocumented and but I found this technique used elsewhere such as in mock-require. The loader patch is only applied if there are paths specified in tsconfig so it should not alter the behaviour if there are no paths.

There is also a small example here to test it. I would like to add some tests for this PR but not sure how to do it since my functions are not exported.

@coveralls
Copy link

coveralls commented Dec 28, 2016

Coverage Status

Coverage decreased (-7.2%) to 79.231% when pulling 48c0830 on jonaskello:paths into b2e6990 on TypeStrong:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-7.2%) to 79.231% when pulling 48c0830 on jonaskello:paths into b2e6990 on TypeStrong:master.

@blakeembrey
Copy link
Member

I mentioned it in #138 (comment), but I don't have any desire to enable this in ts-node directly as it'd be promoting poor node.js practices and portability. I think it'd be better done as a separate module people can just require (e.g. ts-node -r tsconfig-paths-runtime index.ts). That way you can also work on enhancements and changes separately to the codebase here, as I probably won't be able to maintain that section of code myself.

@sanex3339
Copy link

+1 for plugin

@jonaskello
Copy link
Contributor Author

jonaskello commented Dec 28, 2016

Yes, I did read that comment but I wasn't aware of the -r option, so I thought "plugin" was refering to that it should be opt-in or a typescript plug-in. Sorry for the misunderstanding, guess I should have RTFM before trying anything :-). I considered hiding it behind a CLI flag but since it is only activated if you actually have paths in your tsconfig it is already opt-in. And if you have paths in your tsconfig it will not work today anyway so I thought using an undocumented feature in node to make it work was better than not working at all. But I do understand the reluctance to maintain undocumented features.

Now that I know the -r option I will certainly explore doing it as a plug-in. However I would say that at some point it would be nice if paths would "just work" with ts-node. The main reasons for using paths AFAIK:

  • It is the only supported way to maintain module style custom typings in your project, as discussed here and here.
  • It is the only way to inter-op with jspm (as discussed in the original paths issue in the TS repo).
  • It is the only way to structure a TS project without having a lot of "../.." references.

For these reasons I think there is a growing number of projects that use paths. I consider ts-node a great tool for the typescript community and I think it would be unfortunate if the part of the community that uses paths in their tsconfig could not use it, or had to take special steps to make it work.

Just to clarify, the only reason it cannot be included directly in ts-node is that it is using the undocumented patch of Mode._load or are there other reasons?

One question about doing it as a -r option plug-in, how do I know which tsconfig file ts-node has selected to use?

@jonaskello
Copy link
Contributor Author

After further research I found that the -r option is actually something supported by node rather than ts-node and it is also available when executing tests in mocha. Knowing that it makes more sense to make it a separate module since it could have merit outside of ts-node.

I still think that eventually having the "it just works" experience would be the nicest way in ts-node, but until we find an agreeable way to do that I'll close this and report back on my progress as a separate module loadable with -r.

@jonaskello jonaskello closed this Dec 28, 2016
@blakeembrey
Copy link
Member

blakeembrey commented Dec 28, 2016

Mostly don't want to include it in core as it's not something I can maintain myself. My time is pretty limited for OSS lately and new features like this can sometimes come with a lot of bugs. Bugs mean issues I would need to dedicate time to fixing. A better way to support a feature like this is to build the module independently (I always intended to write an abstract version, but as you know - not a priority for me) and allow it's usage programmatically. Once it's stable, I can just require it and include it in ts-node easily, but until then people can use something like a tsconfig-paths/register hook or a programmatic API to register the paths to use.

Edit: There's likely two useful levels to a module like this. 1. "low-level" paths mapping for people to use in other TS environments (Webpack, Browserify, etc.) and 2. "high-level" node-specific version.

@jonaskello
Copy link
Contributor Author

jonaskello commented Dec 28, 2016

I hear you about the lack of time, I did not get my standing christmas wish for 36h days this year either :-).

My current efforts are now in the tsconfig-paths package. I can get it to work with node but not ts-node. Specifically, I have a stand-alone example here which installs the tsconfig-pathspackage and has two scripts:

$ yarn start
$ ts-node -r tsconfig-paths main.ts 
Error: Cannot find module 'tsconfig-paths'
$ yarn start:js
$ node -r tsconfig-paths js_out/main.js
Hello1
Hello2
Hello3

I'll investigate whats going on.

@jonaskello jonaskello deleted the paths branch December 28, 2016 18:39
@jonaskello
Copy link
Contributor Author

Btw, awesome-typescript-loader already has this paths functionality for webpack so it might make sense to make a separate resolver package and share it as you mentioned.

@jonaskello
Copy link
Contributor Author

I think I found what is going on, I started #257 rather than discussing it here.

@ericmorand
Copy link

Just stumbling into this issue and I want to add my voice to this:

I don't have any desire to enable this in ts-node directly as it'd be promoting poor node.js practices and portability.

paths is bad. It defeats portability and is a hell to maintain.

@cyraid
Copy link

cyraid commented Jun 25, 2021

Just stumbling into this issue and I want to add my voice to this:

I don't have any desire to enable this in ts-node directly as it'd be promoting poor node.js practices and portability.

paths is bad. It defeats portability and is a hell to maintain.

How does it defeat portability? Used with baseUrl it can separate 'modules'. I use relative paths in my 'paths', combined with extends in my tsconfig.json.

@blakeembrey
Copy link
Member

blakeembrey commented Jun 29, 2021

How does it defeat portability?

I assume you don’t share your code on NPM, or use different bundlers, or work with other people that don’t also have paths support enabled? Portable is subjective here, maybe portable to you is just having someone else also use ts-node, but the original goal of the project was that the same code could run as if you did tsc && node, which would mean the same code works everywhere else (Deno, ESBuild, Webpack, etc). For everything else there’s plugins which is how this work got done which is another thing to consider. I probably would have never had the time to invest in paths support myself, no one pays me for this.

@cyraid
Copy link

cyraid commented Jun 29, 2021

How does it defeat portability?

I assume you don’t share your code on NPM, or use different bundlers, or work with other people that don’t also have paths support enabled? Portable is subjective here, maybe portable to you is just having someone else also use ts-node, but the original goal of the project was that the same code could run as if you did tsc && node, which would mean the same code works everywhere else (Deno, ESBuild, Webpack, etc). For everything else there’s plugins which is how this work got done which is another thing to consider. I probably would have never had the time to invest in paths support myself, no one pays me for this.

Don't worry Blake, it's all good. Thank you for your source code contributions! :)

@blakeembrey
Copy link
Member

To follow up on this, I think @jonaskello would always love help maintaining the paths support 😄 It looks like it's a project as big as ts-node on its own, especially for adding ESM support. Also happy to help expose additional hooks that people need to build cool plugins like this.

@fatihaziz
Copy link

to solve this issue, i have workaround on this issue answer : #138 (comment)

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

Successfully merging this pull request may close these issues.

7 participants