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

rewrite imports #60723

Closed
wants to merge 2 commits into from
Closed

Conversation

adrianstephens
Copy link

Fixes #58657 (?)

This PR adds a compiler option 'rewriteImports' which, when enabled, changes the path of local import statements to the resolved path relative to the source file. e.g.

import * as xyz from '@stuff/xyz';

is transpiled to

const xyz = require("../shared/src/xyz.js");

with

tsconfig.json:
  "compilerOptions": {
      ...
    "rewriteImports": true,
    "paths": {
      "@stuff/*": ["shared/src/*"]
    },
  },

So this doesn't fix an explicit issue marked as 'help wanted' (that I could find), and it seems there are a number of workarounds for dealing with the path discrepancy, so maybe it's deemed unnecessary or wanted, but:

  1. A lot of the approaches involved additional build steps which I wanted to avoid.
  2. Hooking require or _resolveFilename had global repercussions inside vscode extensions (my use case).
  3. Ultimately none of the increasingly contrived approaches fully worked.
  4. I couldn't think of a reason anyone wouldn't want this to happen.

If this change is ok in principal, but I am doing this wrong, please advise me of a course of action.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 9, 2024
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #58657. If you can get it accepted, this PR will have a better chance of being reviewed.

@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@MichaelMitchell-at
Copy link

I couldn't think of a reason anyone wouldn't want this to happen.

If you move the file to a different directory you may have to update all the relative import paths, creating extra noise in diffs and git blame.

@adrianstephens
Copy link
Author

adrianstephens commented Dec 9, 2024

If you move the file to a different directory you may have to update all the relative import paths, creating extra noise in diffs and git blame.

The changes would only be in the output, which wouldn't typically be committed. Also, if you map the moved file through the "paths" setting in tsconfig, you wouldn't even need to change the typescript import statements

@RyanCavanaugh
Copy link
Member

This isn't OK in principle (and doesn't seem to be what the linked issue describes). Path rewriting like this is very fraught and is intentionally not part of TypeScript.

@adrianstephens
Copy link
Author

@RyanCavanaugh I feel like I've gone about this the wrong way; if you don't object I'd like to open an issue explaining why I think this is needed so it can be discussed (or shot down!) properly.

I'm just trying to finish what the 'paths' option started, and this very small, optional change finally allowed me to share code among my projects easily.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Dec 10, 2024

This has of course been discussed before

#49083 (comment)
#16577 (comment)
#15479 (comment)
#49083 (comment)
#15479 (comment)

If you have something not covered in those threads (which would be impressive! 😅) feel free to log a new issue but this is extremely trodden ground at this point

@adrianstephens
Copy link
Author

adrianstephens commented Dec 10, 2024

@RyanCavanaugh I do have something not covered in those threads! I think you've made the philosophy quite clear though, so I won't belabor this.

In my scenario I am relying on ts to transpile only the files that are reached through import statements. In this case ts uses tsconfig's "paths" and "rootDir" to locate the source files and transpile them into "outDir". It is only for these files, that ts has just created and certainly knows the relative location of, that I wanted the import statement to be fixed.

(I've written some vscode extensions that I want to share code between, and my options are limitied)

Just going to add that since you already have tryRenameExternalModule, adding one wafer-thin tryRenameInternalModule can't do any harm...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Improve support for internal packages by resolving path aliases
4 participants