-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Offer option to resolve module path maps in emitted code #26722
Comments
I think you want #16577. |
Just to elaborate a bit here, we haven't moved on that issue given that there are so many moving parts in the modules space that anything we do would be irrelevant or deprecated in 2 years. For ts-node, can you use tsconfig-paths, as per this documentation? |
Hey Daniel, thanks for the quick response. I don't think this is a duplicate of #16577 -- this issue deals with aliases not being resolved to their relative paths on build. I should have been clearer in the description rather than linking to an old bug. E.g, given this directory structure:
Where compilerOptions: {
...
"paths": {
"@common/*": [
"./common/*"
]
}
} And this code in # server/index.ts
import { foo } from '@common/helper'; When TSC outputs this, it keeps the alias rather than resolve it to the relative path. And node doesn't know how to resolve this. Unfortunately, I don't think you can use
But this does not work:
My request is that the TypeScript compiler offer an option to resolve the aliases to relative paths so that node can properly run the outputted code. Otherwise, the "paths" feature in |
Check out this gist: |
A simple use case that
Right now, the only way to do this is to have a Babel compile step happen with |
@DanielRosenwasser provided my previous comment makes sense, can we remove the duplicate flag or discuss further? |
@DanielRosenwasser pinging again. Any chance this could go on the roadmap for 3.2? |
Until officially supported, I've created a solution that does not require any dependencies nor babel. It also fixes .d.ts files (currently only if they are in the same directory as output. declarationDir support can be added too. PRs are welcomed!). |
@joonhocho there are existing third party solutions, the idea is to avoid them entirely. @DanielRosenwasser pinging again -- this is still marked as a duplicate and it shouldn't be. can we re-evaluate? |
@DanielRosenwasser please reevaluate this issue. The proposed solution is simple, and has nothing to do with the reasons you mentioned for rejection - this doesn’t align with any 3rd party tool, it just makes the TS internal paths option work like the vast majority of people expect. Please consider this. Every week an issue is opened about this, and quite a few have very long and very strong comment sections that all echo the same 2 things:
|
@andy-ms could I get your eyes on this issue? I think Daniel errantly resolved this as a duplicate a while ago, but per my explanation above (#26722 (comment)), it's not. I think given the reactions to this FR and the original issue (#10866), it would be a well received addition to TS. |
I think it's unlikely that we would change the module specifier you wrote when emitting -- #16577 is an existing issue that asks for that kind of functionality, and if we ever did add that then maybe we would take a look at this too. |
@andy-ms ah my mistake I read through the issue more and understand how this is a dupe. Thanks for taking a look. |
It's so different from 16577, and more important!While this issue and #16577 look similar in terms of "customizing emitted code on module paths", I think there is a subtle difference between the two and I'd like to ask for reopening this issue as its own. Let's say I have the following import in my typescript code import helper from "@utils/helper";` and in "paths": {
"@utils/*": [ "src/interal/tools/utils/*" ],
} The problem with this one is, there is an actual piece of information, (rather than just a const helper_1 = __importDefault(require("@utils/helper")) and the piece of information that Exesting Third Party SolutionsTo solve this issue, I need to either re-enter the same mapping manually for the consumption of my JS Loader via something like module-alias (funny, I'm already doing it once for ESLint!) which requires having multiple entries in the codebase for the same concept, a terrible practice; or I should use a tool like tsconfig-paths that reads It's interesting to me that for this very problem, tsconfig-paths has got to 500K downloads/week! In the mean time, all the tickets asking for this simple functionality have been closed here: #16640, #18951, #18972, #19453, and so on! (PS. Just found #10866 from years ago with tons of emojis!) ProposalIt's really simple.
So my "compilerOptions": {
"module": "commonjs",
"esModuleInterop": true,
"target": "es6",
"noImplicitAny": true,
"moduleResolution": "node",
resolveMappedPaths: "true"
"..." It can also be a property on an overloaded per-path-basis config, e.g.: "paths": {
"@utils/*": {
"resolveOnCompile": true,
"target": [ "src/interal/tools/utils/*" ]
},
} |
+1 |
+10086 |
For production apps after a |
@joseluisq Thanks! But @DanielRosenwasser I don't get it why we don't get a compiler flag to resolve the path. Just to make the path-feature work we have to usw webpack or another bundler. It does not work with tsc. |
Path mappings are meant to reflect the behavior of whatever is actually resolving
Because a compiler flag doesn't actually relieve us of the burden of the complexity of a feature - it just makes it harder for us to think about. |
@DanielRosenwasser Thanks for your answer!
My main problem is that I really like what tsc gives me - multiple output files! Every TS-file produces a JS counterpart. That's cool. Readable, understandable and nice. That's what I use for all of my libs. I use webpack for the main application - OK.
I can choose between two options.
[Update] |
But when you use that in webpack, the resulting build actually works. When you use path mapping in TS and use the TS compiler, the resulting build doesn't work. |
+1 for this, same issue here. We're using TSC only for type-check and generating d.ts files. The rest is Babel. In Babel we're resolving our aliases fine, but the generated d.ts files will still have the alieses in the copiled code. I also don't understand why this issue have to be closed. |
+1 |
+1..... |
@wintercounter >I also don't understand why this issue have to be closed. |
Also wondering - this is really making a theoretically nice feature (having nice imports) completely useless. |
Wouldn't it also conceptually conflict with import maps?
Another one I'd like to add is handling of dynamic import syntax. Should they be transformed as well? If yes, that might present a very surprising behavior: import('path'); // works
const path = 'path';
import(path); // fails If no, that might be against the specification, since both static and dynamic imports use the same abstract operation for specifier resolution. |
As the |
As I mentioned before the solution would be an overhaul to the plugin
system. Another pain point which the community has complaining about for
years now. It would save a lot of headaches fro you guys if we would be
able to customize the compiler on our own through plugins and it would
easily tackle such cases as this one.
…On Fri, Sep 20, 2019, 04:04 Kitson Kelly ***@***.***> wrote:
Wouldn't it also conceptually conflict with import maps?
As the paths feature stands now, it reasonably models import maps. It is
how the feature of SystemJS and AMD was standardised in a browser context.
It doesn't conflict conceptually any more because of it, but it underlines
that TypeScript doesn't want to get into the business of re-writing module
specifiers, because that is effectively a runtime only concern.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26722?email_source=notifications&email_token=AAHLJQGE4HWI2OCOPAL6HU3QKQVUPA5CNFSM4FSCEI7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7FKBWY#issuecomment-533373147>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHLJQEH4Q6YKFQ5RXLVEC3QKQVUPANCNFSM4FSCEI7A>
.
|
+1 |
I think this is really important, as TypeScript declarations for libraries are essentially broken if you use absolute paths. We then need to manually fix them, or use a tool to go through and correct paths. If my library internally imports But I can't do the same for For the JavaScript itself, I understand the argument for not changing output. |
I spend all day trying to figure out what wrong with my setup and why paths aren’t resolved in outputted js files. Makes this feature almost useless and very confusing, since it processes NOT WORKING output for NodeJS. Please add some option to resolve paths that tsc already knows internally and bring the end to this struggles. It wouldn’t break anything if it will require option to enable resolution. |
I also ended up here after searching for why my emitted code doesn't have the paths I expected. After reading through this thread, I think I agree with what seems to be the unpopular position of not changing the paths in the output. There's just too much to consider, and I don't see how tsc could possibly account for every variation of build process out there, not to mention things like automated tests, etc. What I would suggest to the tsc devs is to make the documentation a little clearer here: https://www.typescriptlang.org/docs/handbook/module-resolution.html#path-mapping Right now, that section could be interpreted to mean that emitted code will have the paths resolved, but really the setting just makes it so that we can use other tools for module resolution and allow TypeScript to understand what those imports mean. The documentation not being clear about that seems to have led to some confusion and expectations among a lot of users, myself included. |
@mrobrian what is suggested here multiple times (and has received many upvotes and support from other developers) is to add an optional flag that supports the simple absolute paths to relative conversation in the output, just as it's already doing in the compile time. The two bold words here are:
Thanks. |
+1 |
Just gave up and change 72 file in one of my packages. Moved back from module path to relative path imports. |
@RyanCavanaugh I think what people ask for could be a rather simple solution and has nothing to do with the existing compile-time module resolve process. I think what people want is:
{
"compilerOptions": {
"pathAlias": {
"a/b/c" : "../x/y/z",
"@e/d/f/": "../../e/d/f",
"../../u/v/w/": "../dist/"
}
}
} The compliler should complete all existing compilation logic and do the followings before emitting the result:
Moreover:
I think the simple logic above should avoid most of the issues you mentioned in your post as this |
It also achieves what we ask for using
{
"compact": false,
"retainLines": true,
"minified": false,
"inputSourceMap": false,
"sourceMaps": false,
"plugins": [
[
"module-resolver",
{
"root": ["./dist"],
"alias": {
"abc/src": "@a/bc/dist"
}
}
]
]
}
Unfortunately, babel won't touch It will be good that this feature can be implemented by |
Curious if anyone has found a solution for rewriting import alias in |
Yes, https://www.npmjs.com/package/tscpaths |
@wintercounter Hm. Doesn't seem to work if your |
The author did a limited implementation just to support the basics, but I
think it'd be easy to add support for extends. Make a PR if you have time.
…On Wed, Jan 8, 2020 at 5:00 PM Tom ***@***.***> wrote:
@wintercounter <https://github.com/wintercounter> Hm. Doesn't seem to
work if your tsconfig.json extends from another config. In my project, we
extend from a dependency config and tscpaths blows up.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#26722?email_source=notifications&email_token=AAHLJQBLU7OCXGYZOC6C2ITQ4X2BDA5CNFSM4FSCEI7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEINBRAA#issuecomment-572135552>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAHLJQHWFLAQHJPBTQQK4RLQ4X2BDANCNFSM4FSCEI7A>
.
|
After reading a heavy portion of this thread I can say that everyone is right, from their perspective. What I (again. I) think the problem in this discussion is? TS users come from a working mode of, hey, i'm running my node app with TS in development (ts-node/webpack/whatever) and it works!! Now, when I build it does not! While (I guess) TS authors look at it differently, TS is just a platform to take on form of source code and output another form of source code and having it run as intended. This came to bloom mainly because of the gaining popularity of monorepos and how easy it is to build libraries that way, alongside a working application. Now, there are 2 scenarios:
With (1), users expect TS to take those paths mappings and convert them to relative paths so they will work cause they are not in With (2) user's will not expect that to happen because it will cause the imports to point to nothing. I understand TS's team take on this, it is something that will work with (1) but not (2) and that's enough to rule it out. Maybe i'm wrong, but it seems to me it's one of the reasons for the mixup |
Optional flag = Developer will have the option to enable it for #1 of your case; which is the majority of the cases as can be seen by the number of upvotes here. |
If path maps are not resolved into emitted code, then what are they for? |
@winseros this heavily downvoted comment I wrote a few months ago answers that question. #26722 (comment) |
We took this to several design meetings and discussed this to death, and we're sticking with the "JS is JS" design philosophy that has driven TypeScript since its origination. I'm going to lock this because it's a long thread and our typical experience is that these threads go in circles when it takes more than a few minutes to read the thread. As usual, I'd implore anyone to actually read all 81 comments here, as we ourselves have done multiple times, before filing new issues related to the matter. |
Search Terms
paths
tsconfig.json
compilerOptions
ts-node
node
cannot find module
Suggestion
Per #10866, module path maps are not resolved in emitted code. This means that if you attempt to run the emitted code with node, it will be unable to resolve those paths and the server will not start.
Current solutions:
browserify
orwebpack
to bundle the output -- this is an unfortunate solution given that it'd be the only reason for me to bring in one of these tools, as I've managed to build and deploy apps using just the typescript compiler. This blog post from the TypeScript team even strongly recommends using a TypeScript toolchain:https://blogs.msdn.microsoft.com/typescript/2018/08/27/typescript-and-babel-7/
module-alias
. This is a bit gross as it requires duplicating the path maps, and inserting code at the root of your server.Proposed solution:
Offer a
compilerOption
, something likeresolvePaths
, that resolves aliased paths to their relative paths.Use Cases
So that I can continue to meaningfully use the
paths
property intsconfig.json
.Examples
Please see the example in #10866.
Checklist
My suggestion meets these guidelines:
The text was updated successfully, but these errors were encountered: