Importing sometimes imports relative path instead of absolute when using project references

[Tyriar]

TypeScript Version: 3.5.2

Code

Auto importing CellData in https://github.com/xtermjs/xterm.js/blob/73521390d29c8a111058a6a7d08edd827ba498b7/src/browser/ColorManager.ts will import from ../../out/common instead of common/

See tsconfigs:

• ColorManager lives in https://github.com/xtermjs/xterm.js/blob/73521390d29c8a111058a6a7d08edd827ba498b7/src/browser/tsconfig.json
• CellData lives in https://github.com/xtermjs/xterm.js/blob/73521390d29c8a111058a6a7d08edd827ba498b7/src/common/tsconfig.json

Expected behavior:

import { CellData } from 'common/buffer/CellData';

Actual behavior:

import { CellData } from '../../out/common/buffer/CellData';

[andrewbranch]

TypeScript Version: 3.5.2

Hey @Tyriar, can you see if this is fixed for you now? There have been some changes around picking module specifiers in the last couple versions, and I can’t repro this in 3.6 or 3.7.

[Tyriar]

This does seem to be fixed when using TS 3.7.2, thanks!

[bpasero]

@andrewbranch there is still an issue where implementing an interface puts these ugly inline imports that are relative:

Is this known?

[andrewbranch]

@bpasero I’ve fixed a similar issue to that before so I know exactly why that’s happening, but unfortunately each of these needs to be addressed in its own separate codefix/refactor. Would you mind opening a new bug and mentioning me?

[bpasero]

@andrewbranch #34995

[Tyriar]

@andrewbranch this still seems to happen when importing from the same project:

Notice TS version in bottom and proposed auto import.

Expected is:

import { blend } from 'browser/Color';

[Tyriar]

I have also noticed important from an adjacent file importing "Constants" instead of "./Constants" which resulted in a compile error.

[andrewbranch]

@Tyriar I think the Color example is expected behavior, depending on your typescript-language-features settings:

“Auto” will still prefer relative imports if they’re shorter than the non-relative alternative. If you always want the non-relative path, you can adjust this setting accordingly.

I have also noticed important from an adjacent file importing "Constants" instead of "./Constants" which resulted in a compile error.

This sounds like a different (and worse) issue. Do you have repro steps for this one? If so, do you mind opening a new issue?

[Tyriar]

@andrewbranch thanks seem to work. I'll add that setting to workspace settings and ping back if there are further issues.

This sounds like a different (and worse) issue. Do you have repro steps for this one? If so, do you mind opening a new issue?

Repro:

1. Clone xterm.js master https://github.com/xtermjs/xterm.js
2. yarn
3. Create addons/xterm-addon-webgl/src/Constants.ts with this content:

   export const MY_CONST = 1;
   

4. Go to addons/xterm-addon-webgl/src/GlyphRenderer.ts and auto import MY_CONST

This is happening because of "baseUrl": "." here https://github.com/xtermjs/xterm.js/blob/339d4d6a7316d1c1791b7a5859ba9c8175ce3cc6/addons/xterm-addon-webgl/src/tsconfig.json#L13, but the problem is I need baseUrl in order to reference non-relative imports from paths. Maybe I'm just doing something wrong here? It compiles fine but the webpack build breaks as it doesn't know how to resolve Constants. Basically I want it to import non-relative for browser and common only, relative for everything inside addons/xterm-addon-webgl/src

[andrewbranch]

Basically I want it to import non-relative for browser and common only, relative for everything inside addons/xterm-addon-webgl/src.

Hmm, you already have explicit path mappings for browser and common, so do you need baseUrl at all there? Yes, I always forget it’s required to use paths at all

[Tyriar]

I could probably just move over to use non-relative everywhere, probably based out of the src/ dir as it doesn't feel right importing without / for anything other than node modules.

[andrewbranch]

I’m honestly not sure what the best setup for that is. I think what your asking for is super common in Webpack, but it seems like paths was designed for a slightly different scenario. I’m adding this to my list of things to investigate. Of course, you can make all your code work fine with your current settings, you’ll just get some auto-imports in a format you don’t want in some places.

[Tyriar]

@andrewbranch the setup for the addons is admittedly a little weird, TS is referenced from the core and included in the addon bundles. Typically these just reference .d.ts files but they can also include .ts files that result in additional output. The reason for this approach is that we wanted to do a renderer but not include it in the main binary as it's a lot of code to load when it's not being used, and we don't want to expose a bunch of API just for this "internal" project.