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

Idea: do not translate paths which start from some entry in node_modules #83

Closed
dko-slapdash opened this issue Dec 8, 2020 · 3 comments · Fixed by #88
Closed

Idea: do not translate paths which start from some entry in node_modules #83

dko-slapdash opened this issue Dec 8, 2020 · 3 comments · Fixed by #88

Comments

@dko-slapdash
Copy link

Hi.

I'll try to explain the issue I'm facing...

In TS 4.x, they started addressing different monorepo related feature requests and issues. For instance, they implemented "IntelliSense auto-import mapping customization" which allows people to auto-import modules from other project's dist/ folders (not from their src/ folders) and thus dramatically improve the compilation speed (because the generated *.d.ts files can be omitted in typechecking with skipLibCheck=true, and in general, they are way more lightweight than *.ts files).

Here is an example of one of the configurations:

https://github.com/microsoft/TypeScript/pull/40253/files#diff-ec1d337c15d5294001ed10a759d0be3b490c4e731588746dd30ae79eea18f01d

I'm also showing a screenshot of a sandbox project to illustrate, what's going on.

image

In a monorepo setup (e.g. lerna or yarn workspaces), for each project in packages/, lerna/yarn create a symlink in the top-level node_modules. It lets people import from other monorepo projects as if they're importing from a regular node_modules module downloaded from npmjs (although the module itself lives in packages/).

In TS 4.x, paths is also used to tell TSC, what should IntelliSense auto-import when a symbol is found. I.e. on the screenshot above, TS recognizes that there is a Dinosaur symbol in packages/shared/src/Dinosaur.ts file, and when suggesting an auto-import, it traverses it back to paths key, and finally it proposes to import it from shared/dist/Dinosaur module (which is in another monorepo project) - nice and clean. Also, TS is smart enough to prefer using .d.ts files in such cases under the hood (speed gain) and not include src/.ts (although src/ is mentioned in paths) - this is due to -b flag and composite=true.

The problem is that typescript-transform-paths also tries to resolve such a path, and in the generated *.js file, we see ../../shared/src/Dinosaur.js reference - which is wrong, the JS file should be included from node_modules/shared/dist/Dinosaur.js (i.e. we should let Node or other runtime resolve an absolute module path according to the standard node_module rules).

And here comes the proposal. If typescript-transform-paths detects a path which looks like relative to node_modules folder, it would be cool if it just didn't touch it and live it as it is. Because if someone imports from a module in node_modules, they don't want any path translation for sure.

What's challenging here is how to detect if some path is about a node_modules module or not. I don't have a good answer to this... maybe allow configuring the list of exclusions which are never translated (I would then be able to pass "shared" in this list and disable path translation for it). I.e. it's about ignoring of some of entries in paths.

Unfortunately I also can't do this:

image

Because if I do, then IntelliSense will open a wrong path (inside node_modules and not in src/) on Cmd+Click.

@nonara
Copy link
Collaborator

nonara commented Dec 8, 2020

@dko-slapdash Thanks for the excellent level of detail in this! This is an interesting one...

I think the best route here would be an exclusion list. As a bonus, I could also add an option for a jsdoc tag to skip for a single statement. For your purpose, I think the exclusion list will suffice, but I can also see where a JSDoc tag or two for overriding might be useful. I'll have a look at this soon.

@dko-slapdash
Copy link
Author

dko-slapdash commented Dec 8, 2020

Thanks!

Per-statement exclusion won't help us much, we have thousands of imports like that (from monorepo sub-projects). The key thing here is that, if we include from another monorepo project, the rules are best to be the same as if we're including from just an npmjs module, although for TS/IntelliSense/VSCode purposes, monorepo sub-projects have to be addressed differently than regular packages in tsconfig.

@nonara
Copy link
Collaborator

nonara commented Jan 4, 2021

This should now be solvable via the exclude option. See readme for details

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

Successfully merging a pull request may close this issue.

2 participants