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

[Feature Request] Resolve alias path to relative path. #31643

Closed
5 tasks
fenying opened this issue May 29, 2019 · 3 comments
Closed
5 tasks

[Feature Request] Resolve alias path to relative path. #31643

fenying opened this issue May 29, 2019 · 3 comments
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@fenying
Copy link

fenying commented May 29, 2019

Search Terms

resolve path map

Suggestion

I think, there is two way to resolve the path:

  1. Add an option to force TS compiler transform import path in the output.
  2. Use a plugin (which described in 3.6 plan TypeScript 3.6 Iteration Plan #31639 ) to transform the path in the output.

Use Cases

I've read some rejected FEATURE REQUEST like: #10866 #9910 #5039

And there is , maybe, the official response about those request: #9910 (comment)

However that can not convince me, because of following 2 problems

  1. Like the above issues mentioned, it doesn't transform the path aliases into the real relative paths.

    This makes the output not work with Node.js because it can not find the modules by an alias path.

    Although some module like module-alias, ts-alias-loader could solve this, anyway it's not elegent, and it can not solve the second problem.

  2. If a package insides node_modules, written in TS with paths options, its alias path in *.d.ts can not be resolved. And TS compiler will throw an error because the module couldn't be resolved.

Examples

See #10866

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds labels May 29, 2019
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented May 29, 2019

There's a problem: Sometimes the paths on disk during build time are not same as the paths at runtime. What should be done about it?

There are two reasonable ways to go about solving this situation.

The first is to have you always write the path to the file on disk, and provide a way for you to specify how the compile-time path should turn into a run-time path.

The second is to have you always write the run-time path, and provide a way for you to specify how run-time paths map to disk paths during compile-time.

These are roughly equivalently good; we've chosen the second one because it's easier to "get right" and plays better with ignoring unresolved modules. In either case, you end up with an enormously complex system; the complete documentation of how to control everything here is probably tens of pages.

The third option is to offer both. This is basically taking an enormously complex system and then squaring the complexity. I think it's an understatement to call this complete insanity; no one would be able to successfully reason about this beyond the most trivial of configurations.

We're always open to more options about how to be more expressive in the space of the chosen option of "always write the runtime path"; any suggestions need to be in that realm.

@georgyfarniev
Copy link

Well, I read a lot of threads about same issue and I generally agree that good solution could be providing rich extensions api for typescript that allows such behavior.

@icopp
Copy link

icopp commented Jun 24, 2020

For me, the biggest hole with paths functionality is the complete lack of functionality around .d.ts files.

If I have a Node module written in Typescript, I can use a bundler to compile it to JS with mapped paths... but the only way I have to do the same for .d.ts files is to do some kind of awful hacky regex thing on the exported files after tsc runs.

Vadman97 pushed a commit to highlight/highlight that referenced this issue Apr 13, 2023
## Summary

<!--
Ideally, there is an attached GitHub issue that will describe the "why".

If relevant, use this section to call out any additional information
you'd like to _highlight_ to the reviewer.
-->

This is another attempt at
#4851, which had to be
[reverted](#4950) because it
broke type declarations. 😞

TypeScript isn't planning to officially support import path rewriting in
declarations (see
microsoft/TypeScript#31643 (comment)),
so to fully support arbitrary path remappings (like the ones to
`@highlight-run/client`), the most promising solution I could find
involved a combination of
[ttypescript](https://github.com/cevek/ttypescript),
[ts-transform-paths](https://github.com/zerkalica/zerollup/tree/master/packages/ts-transform-paths),
and
[rollup-plugin-typescript2](https://github.com/ezolenko/rollup-plugin-typescript2).

However in our case, we're only looking to remap a single reference, so
a dumb script that looks for the reference and replaces them feels like
a much simpler & lower risk solution. That's what I added in this PR in
addition to the previously reverted changes.

Let me know if y'all would prefer looking into the other approach
instead, which might be worthwhile if we wanted to start using arbitrary
path remappings in libraries (say, to be able use absolute imports
everywhere).

## How did you test this change?

<!--
Frontend - Leave a screencast or a screenshot to visually describe the
changes.
-->

I ran `yarn build && yarn typegen` in both main and this branch, and ran
`diff -rq` to compare both dist folders. There were no differences in
any of the .d.ts files (indicating the import replacement script gave us
the correct output), and the only differences were in `index.js`,
`indexESM.js`, and their respective importmaps, due to the changes in
the package.json file that ends up getting bundled.

![Screenshot 2023-04-12 at 4 43 37
PM](https://user-images.githubusercontent.com/6934200/231610022-ac690d7c-860b-419b-ba96-1287e296b491.png)

## Are there any deployment considerations?

<!--
 Backend - Do we need to consider migrations or backfilling data?
-->

This time I did make sure to include type declarations in the build
output comparison, so I have pretty high confidence they'll behave
identically this time.

Though one thing I couldn't confirm was whether the CI process that
publishes this module runs the `yarn typegen` command or `yarn tsc`
directly. The latter could produce the same problematic output as before
since we depend on the script specified in `yarn typegen` to rewrite
imports. Would appreciate it if someone could double check. 🙏
eightants pushed a commit to highlight/highlight that referenced this issue Apr 16, 2023
## Summary

<!--
Ideally, there is an attached GitHub issue that will describe the "why".

If relevant, use this section to call out any additional information
you'd like to _highlight_ to the reviewer.
-->

This is another attempt at
#4851, which had to be
[reverted](#4950) because it
broke type declarations. 😞

TypeScript isn't planning to officially support import path rewriting in
declarations (see
microsoft/TypeScript#31643 (comment)),
so to fully support arbitrary path remappings (like the ones to
`@highlight-run/client`), the most promising solution I could find
involved a combination of
[ttypescript](https://github.com/cevek/ttypescript),
[ts-transform-paths](https://github.com/zerkalica/zerollup/tree/master/packages/ts-transform-paths),
and
[rollup-plugin-typescript2](https://github.com/ezolenko/rollup-plugin-typescript2).

However in our case, we're only looking to remap a single reference, so
a dumb script that looks for the reference and replaces them feels like
a much simpler & lower risk solution. That's what I added in this PR in
addition to the previously reverted changes.

Let me know if y'all would prefer looking into the other approach
instead, which might be worthwhile if we wanted to start using arbitrary
path remappings in libraries (say, to be able use absolute imports
everywhere).

## How did you test this change?

<!--
Frontend - Leave a screencast or a screenshot to visually describe the
changes.
-->

I ran `yarn build && yarn typegen` in both main and this branch, and ran
`diff -rq` to compare both dist folders. There were no differences in
any of the .d.ts files (indicating the import replacement script gave us
the correct output), and the only differences were in `index.js`,
`indexESM.js`, and their respective importmaps, due to the changes in
the package.json file that ends up getting bundled.

![Screenshot 2023-04-12 at 4 43 37
PM](https://user-images.githubusercontent.com/6934200/231610022-ac690d7c-860b-419b-ba96-1287e296b491.png)

## Are there any deployment considerations?

<!--
 Backend - Do we need to consider migrations or backfilling data?
-->

This time I did make sure to include type declarations in the build
output comparison, so I have pretty high confidence they'll behave
identically this time.

Though one thing I couldn't confirm was whether the CI process that
publishes this module runs the `yarn typegen` command or `yarn tsc`
directly. The latter could produce the same problematic output as before
since we depend on the script specified in `yarn typegen` to rewrite
imports. Would appreciate it if someone could double check. 🙏
shayneo pushed a commit to shayneo/highlight that referenced this issue Apr 19, 2023
## Summary

<!--
Ideally, there is an attached GitHub issue that will describe the "why".

If relevant, use this section to call out any additional information
you'd like to _highlight_ to the reviewer.
-->

This is another attempt at
highlight#4851, which had to be
[reverted](highlight#4950) because it
broke type declarations. 😞

TypeScript isn't planning to officially support import path rewriting in
declarations (see
microsoft/TypeScript#31643 (comment)),
so to fully support arbitrary path remappings (like the ones to
`@highlight-run/client`), the most promising solution I could find
involved a combination of
[ttypescript](https://github.com/cevek/ttypescript),
[ts-transform-paths](https://github.com/zerkalica/zerollup/tree/master/packages/ts-transform-paths),
and
[rollup-plugin-typescript2](https://github.com/ezolenko/rollup-plugin-typescript2).

However in our case, we're only looking to remap a single reference, so
a dumb script that looks for the reference and replaces them feels like
a much simpler & lower risk solution. That's what I added in this PR in
addition to the previously reverted changes.

Let me know if y'all would prefer looking into the other approach
instead, which might be worthwhile if we wanted to start using arbitrary
path remappings in libraries (say, to be able use absolute imports
everywhere).

## How did you test this change?

<!--
Frontend - Leave a screencast or a screenshot to visually describe the
changes.
-->

I ran `yarn build && yarn typegen` in both main and this branch, and ran
`diff -rq` to compare both dist folders. There were no differences in
any of the .d.ts files (indicating the import replacement script gave us
the correct output), and the only differences were in `index.js`,
`indexESM.js`, and their respective importmaps, due to the changes in
the package.json file that ends up getting bundled.

![Screenshot 2023-04-12 at 4 43 37
PM](https://user-images.githubusercontent.com/6934200/231610022-ac690d7c-860b-419b-ba96-1287e296b491.png)

## Are there any deployment considerations?

<!--
 Backend - Do we need to consider migrations or backfilling data?
-->

This time I did make sure to include type declarations in the build
output comparison, so I have pretty high confidence they'll behave
identically this time.

Though one thing I couldn't confirm was whether the CI process that
publishes this module runs the `yarn typegen` command or `yarn tsc`
directly. The latter could produce the same problematic output as before
since we depend on the script specified in `yarn typegen` to rewrite
imports. Would appreciate it if someone could double check. 🙏
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Out of Scope This idea sits outside of the TypeScript language design constraints Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

5 participants