auto-import favors import from target instead of source

[chengB12]

TypeScript Version: 4.0.0-dev.20200529
Verify no issue with vs code version 1.45.1 + typescript vsrion: 3.8
Verify issue with vs code + typescript 3.9.3
still have issue with vs code + typescript 4.0.0-dev20200529

Search Terms:
import / auto import
Code
we have project configure as

|
|-- sharedlib
       | -- src
       | -- dist
|-- package1
       | -- src
       | -- dist
|-- package2
       | -- src
       | -- dist

where src is *.ts source code, dist is where compiled *.js files

sharedlib/tsconfig.json

{"compilerOptions": {
    "rootDir": "./",
    "baseUrl": ".",
    "outDir": "dist",
    "paths": {
      "*": [
        "types/*"
      ]
    },
    "composite": true
  },
  "include": [
    "src/**/*.ts",
  ]
}

package1/tsconfig.json

{"compilerOptions": {
    "baseUrl": ".",
    "outDir": "dist",
    "rootDir": "./",
    "paths": {
      "*": [
        "../sharedlib/types/*"
      ],
      "sharedlib": [
        "../sharedlib"
      ]
    },
  },
}

sharedlib/src/foo.ts

export const foo = 1

package1/src/bar.ts

console.log(foo)

Expected behavior:
When in visual studio code + typescript 3.8
auto import will prompt me to import as

import foo from 'sharedlib/src/foo'

console.log(foo)

Actual behavior:
On typescript 3.9.3+
first, before compile sharedlib, it can't recognize foo, thus no auto import
after compile, it auto import the js from

import foo from 'sharedlib/dist/src/foo'

console.log(foo)

the problem are

1. now when I add new exportable in sharedlib, I have to compile the package first before I am write reference code from other packages
2. Because code write before/after ts 3.9 are import from different path even for same module, javascript will treat they as different modules, thus breaks states/singleton, if I didn't manually remove /dist from import

Playground Link:

Related Issues:

[andrewbranch]

@chengB12 can you provide a repository with this problem? I think some important information got lost or mangled when you were distilling it down to a simple case. E.g., I think you’d need in your paths "sharedlib/*": ["../sharedlib/*"], not just the starless version, for any of this to compile under any TypeScript version.

[chengB12]

Hi, I changed in paths, still same issue

I have created a sample project,
sample.zip

[andrewbranch]

@chengB12 thanks! This is helpful. I do see that an auto-import isn’t offered for foo, but I still can’t reproduce it offering to import from sharedlib/dist/src/foo after compiling. Can you provide exact steps on how to reproduce that?

Also, in the non-example version of this, are you using Lerna, yarn workspaces, or any other method of symlinking the packages together within node_modules?

[chengB12]

Here is something interesting, I can't reproduce it with bare minimal

our original project having tons of export on ts-sharedlib, and our package1 has tons of import from ts-sharedlib. in this case vs code with ts 3.8 can infer from 'ts-sharedlib/src' and ts 3.9 can infer from 'ts-sharedlib/dist/src'

however, when I trying to strip all other files from package1, leaving only one example, and suddenly, neither ts 3.8 nor ts 3.9 can infer import anymore.

It sounds like vs code can be trained to auto import cross packages given enough success imports

either way, it sounds like a bug to me if bare minimal can't infer imports

[chengB12]

Sorry I can't provide whole 'issue-reproducible' repo, if needed, I may be able to provide a few screenshots

[andrewbranch]

When you remove all existing references from package1 to sharedlib, the lack of auto imports is currently expected behavior, tracked in #37812. The paths case is a good test case for #38923. But that’s a really well-understood problem, so the dist thing is much more concerning to me, but I can’t know whether or not it’s really a bug without seeing the whole project. For example, if you currently import sharedlib/dist/src/foo anywhere in package1, you’ll get that as a suggestion:

But I don’t think that’s a bug.

[chengB12]

I can share a screenshot,
You can see, I have existing import from 'ts-sharedlib/src...'
and after I comment correct import line,
the auto-import I got is from 'ts-sharedlib/dist/src'

[andrewbranch]

It’s not just the file that matters, though; it’s every file in the project and every file’s dependencies, those dependencies’ dependencies, and so on 😕. Would you be able to share TS Server logs?

[chengB12]

We have lint to ensure no code use import xx from 'ts-sharedlib/dist/src...'
it shouldn't comes from existing imports

tsserver.log

[andrewbranch]

Alright! I think I have a repro thanks to your logs. Hopefully last question @chengB12, when you click the lightbulb here, do you get an option for both src and dist/src like this?

[chengB12]

No, I only get hint from 'sharedlib/dist'

[andrewbranch]

Even if you add "ts-sharedlib/*": ["../ts-shardlib/*"] to paths? I see in the logs you only have the non-wildcard variant.

[chengB12]

with "ts-sharedlib/": ["../ts-shardlib/"] it display for both now, what's different between it and "ts-sharedlib": ["../ts-shardlib"]?

[chengB12]

However, the problem is instead of hint for import from 'ts-sharedlib/src', it hint for '../../ts-sharedlib/src'

which is equally undesirable since javascript will treat '../../ts-sharedlib/...' and 'ts-sharedlib/...' as two different module too

my tsconfig.json path:

  "compilerOptions": {
    "baseUrl": ".",
    "outDir": "dist",
    "rootDir": "./",
    "paths": {
      "*": [
        "../ts-sharedlib/types/*"
      ],
      "ts-sharedlib/*": [
        "../ts-sharedlib/*"
      ]
    },
  },

[andrewbranch]

Are you using Lerna or yarn workspaces or something else that will symlink these packages together through node_modules? Or do you have an installed copy of these packages in node_modules?

The * in paths is a wildcard that will capture the rest of the module specifier request and substitute it for the * in the value [docs]. I’m honestly not sure how it’s even possible for you to resolve imports from ts-sharedlib/src without having the /* in paths, unless there’s something going on in your node_modules, which there doesn’t appear to be from your server logs.

Regarding the relative import, can you check your VS Code setting for TypeScript > Preferences: Import Module Specifier:

[chengB12]

after change vs setting, it looks fine now,

regards how to resolve ts-sharedlib, I don't know, we don't use symlink. we have this line in tsconfig.json though

"references": [
    {
      "path": "../ts-sharedlib"
    }
  ]

probably this line do the trick?

[andrewbranch]

Nah, that line is correct. I think you just need to keep your path mapping with the wildcard, and I’ll fix the bug that makes the extra suggestion from dist show up. Still not sure what’s going on with getting the dist suggestion when you don’t have the wildcard path mapping. If you can provide an isolated repro for that, I can investigate it separately.

[andrewbranch]

How do imports from ts-sharedlib/src resolve at runtime? Are you publishing/packaging the dist directory of ts-sharedlib?

[chengB12]

yes, we are publishing ts-sharedlib to ts-sharedlib/dist

[chengB12]

side effect on that change:

Previously with wrong path setup, it can infer import from '../src', now it auto-import from 'src/' instead, which is not a correct path

/test is on same level of /src

probably due to TypeScript > Preferences: Import Module Specifier from relative ==> auto?

[bradennapier]

I believe this is caused by outDir -- see https://github.com/microsoft/vscode/issues/103087 -- I also have a repro to show that import will ONLY provide the dist dir if using outDir with references / paths. In my actual codee I use ts-config-paths to resolve paths properly after compile.

Repro: https://github.com/bradennapier/typescript-auto-import-bug

Note that adding "typescript.preferences.importModuleSpecifier": "non-relative", in vscode causes the following in my repro:

[dinofx]

What this issue is complaining about is exactly the behavior you would want, since the src folder won't exist outside development.

Auto import suggestions started showing up for me (maybe because of ...experimental.enableProjectDiagnostics": true) on 3.8.3. However it is inserting the wrong imports. It is pointing to the source location instead of the output location. I don't have paths set in any tsconfig files. Lerna is hoisting the packages. If I change the tsconfig for the referring package ("project"), to "disableSourceOfProjectReferenceRedirect": true, the suggested imports are correct.

I don't see why this setting should affect proposed imports. The suggested imports should always be to the transpiled output, with disableSourceOfProjectReferenceRedirect determining whether the source file is the source of truth, or the generated .d.ts.

[chengB12]

The problem is target won't exists when inside development, before compiling, and who need auto-imports outside development, I am curious.

Force compiling, otherwise, intelli-sense won't work properly doesn't sound right

[bradennapier]

Yes literally the last thing you'd ever want is auto imports to try to import the eventual dist path because you're in the dev path - it should go to the relative (or absolute if there is a paths config) of the dev source so that once compiled the compilation step can either modify properly or do nothing if it is relative and will thus remain correct.

There is not a single case where you would want to import a dist resource from your src resource. That's insane.

[andrewbranch]

@bradennapier let’s keep the conversation less shouty and incredulous. The problem is that you can invent scenarios where any of these paths do or don’t work depending on exactly what you package/deploy/run, and the compiler simply cannot know that information today. There is definitely a bug here, but anyone here saying words like “always” and “never” is making assumptions that don’t hold for all configurations and all runtime circumstances that aren’t reflected in configurations.

[bradennapier]

My bad, didn't mean to imply shouting on the caps, just emphasis. Updated accordingly.

[bradennapier]

Just a question - in what case would you want your files in the src directory to auto import to the dist directory? Should this not take paths into account to figure out where to import? Wouldn't, at the very minimum, you want it to just be relative to the current file?

This recently started affecting our larger project for some reason, before it was only on a single package of ours that this was happening. Now ALL auto imports are essentially useless because they are defaulting to the dist folder. Not sure what changed here, perhaps just a typescript version update at some point caused it... or a vscode update?

If i have a paths property in my tsconfig which indicates:

"paths": {
      "core/*": ["./core/*"],
      "services/*": ["./services/*"],
      "packages/*": ["./packages/*"]
    },

I would think it would realize that it should use packages/ (perhaps check if it exists first).

I have my editor settings to do absolute specifically but that doesn't seem to fix anything. In this case the dist folder doesn't even exist either.

I have added the dist folder into exclude as well to try to resolve - perhaps this could be how we could indicate never to use dist for auto imports?

You don't realize how much you have come to depend on a feature until it stops working :-P

---

Interestingly, in VSCode, if I look at the autofix rather than the suggestion while typing, it gives both options:

[andrewbranch]

Where importing from dist might possibly make sense is if you have a monorepo with composite projects that you intend to publish to a package registry. Suppose you have

packages/
  app/
    src/**/*.ts
    dist/**/*.js, *.d.ts
  dep/
    src/**/*.ts
    dist/**/*.js, *.d.ts

In each, the rootDir is src and the outDir is dist. In app, you have either paths or node_modules symlinks set up to be able to reference ../dep/* as dep/*, because in production, app will install dep into its node_modules.

Now, suppose you’re trying to import something from dep into app during development. What should the module specifier be? The answer is pretty clear if the exported symbol is exported from dep’s main entrypoint in its package.json: the module specifier can just be "dep". But what if the symbol is from dep/src/some/sub/dir.ts / dep/dist/some/sub/dir.js, and that symbol isn’t available from just "dep"? Of the two paths, it seems likely that dist is better, since the JS and TS are not colocated. But that answer only really makes sense because I told you that dep is going to be published to a package registry and ultimately installed in app’s node_modules, which is something the compiler doesn’t know.

Your screenshot shows a relative import path to dist, which I do think is harder to justify. The OP here had a non-rooted import path to dist, like my example above. Either way, this particular behavior was an unintended side effect of fixing a related auto-import issue for yarn workspace / lerna users.

I have added the dist folder into exclude as well to try to resolve - perhaps this could be how we could indicate never to use dist for auto imports?

It just doesn’t work like that, in this particular case. The reason we look in dist even when you exclude it is so that if you are wanting to import a symbol from, say, ../dep/src/index.ts, which happens to be symlinked to ./node_modules/dep/src/index.ts, we want to see if the output file ./node_modules/dep/dist/index.js happens to match the "main" field in ./node_modules/dep/package.json, because if it does, we can just have you import from "dep". The bug is that when that process doesn’t yield the really nice module specifier "dep", we’re left with a random file in dist that we don’t know how to generate a module specifier for other than writing a path with dist in it, and we prematurely gave up on the other non-dist paths because we were hoping that it would yield just "dep".

It seems like you’re concerned that we’re not going to fix this or something, but that’s not the case; I’m working on it 🤷

What would be tremendously enlightening to me, if not directly helpful with this issue, is if you could explain why you’re using paths in the first place. In your example tsconfig snippet earlier, how do you suppose that an import from "core/*" will resolve at runtime? Is core going to be published to a package registry and installed in the other packages’ node_modules? If so, how do these things resolve during local runs and testing? I have always used lerna or yarn workspaces for this kind of thing, which means you don’t have to use paths.

[bradennapier]

Thanks @andrewbranch not worried it won't be fixed, was just curious for real cause I was trying to think of a case :-P, thanks for the descriptive response though!

The paths are used to remove relative imports, which are extremely painful since our repo is quite massive, and is a fairly common request from people (there are lots of issues around this). It is a common pattern with webpack for example to add a resolve.alias to handle this so that your src dir is always allowed to imported as an absolute.

I accomplish this in typescript by adding https://github.com/zerkalica/zerollup as a plugin, but this issue would occur for anyone using webpack or rollup as example as well, or https://github.com/dividab/tsconfig-paths -- but there are probably around 10-12 packages all to deal with these cases.

eslint plugins also exist specifically for this https://github.com/alexgorbatchev/eslint-import-resolver-typescript

the transformer plugin is the ideal because it just rewrites the paths to relative at compile time so it all matches up and there is no runtime hit.

lerna would be nice here... unfortunately i was not apart of the original decision when the repo was made and that is quite difficult change atm. we use lerna in a few of the other repos that were made more recently ;-)

---

vscode also does this - they just do it manually at build time, but they enforce absolute imports across the board IIRC ;-)

[AlCalzone]

@andrewbranch I believe I'm still running into this, even with the current TS version:

Note that this is an import from within the same project. I would expect the import path to be "../message/Constants".

Other suggestions are all kinds of weird:

From top to bottom:

1. index.d.ts in the current project's root dir, which re-exports from ./Values (this file is copied during the build)
2. index.ts in the current project's src dir, which re-exports from ./src/Values/index.ts (source file for the 1st suggestion)
3. index file for the referenced project @zwave-js/core (preferred import, because its the referenced project's main export)
4. Values.d.ts in the current project's root dir, which re-exports from @zwave-js/core (this file is copied during the build)
5. Values.ts in the current project's src dir, which re-exports from @zwave-js/core (source file for the 4th suggestion)
6. Absolute path (relative to baseUrl) to the original source (where suggestion 3 exports from). Using this path would be completely invalid.

Tbh, I haven't figured out a good way to import sources from the current project with a relative path and from referenced projects using their absolute path. Sometimes it works, sometimes it doesn't 🤷‍♂️

[chengB12]

@AlCalzone for me, I solve this issue by

1. use nightly typescript build
2. specify in paths in tsconfig.json

{
   paths: {
        "zwave-js/src": [ "../zwave-js/src/*" ]
   }
}

[dinofx]

Imports that refer to the target location are working for me, even when the source has not been built and the target location does not exist. And of course such imports work as expected at runtime. This works with just project references, without any need to configure a "paths" entry in tsconfigs.

So a source file can import shared/dist/foo, and TS uses node resolution, finds the hoisted shared package, doesn't find dist/foo, but is able to resolve to the .ts source file (in its original, non-hoisted location) that will eventually produce it.

[AlCalzone]

I do have paths configured, because the directory structure is a bit different than the package names that are released later on. My first problem should is not affected by paths though, since it is an "internal" import.

The 2nd one does go away once there is one import in the current file that points to the correct location. When it is not, I think this heuristic makes sense:

1. Prefer symbols that are exported from another project's main entry point
2. In the current project, prefer symbols that are just exported (i.e. at the definition site) over ones that are re-exported

[chengB12]

from comment of #40253

this is fixed by checking paths first to look up matches, so changing paths to make what you want it to be recommended in auto-import works for me

[andrewbranch]

I believe I'm still running into this, even with the current TS version:

@AlCalzone can you confirm which nightly you’re using?

My first problem should is not affected by paths though, since it is an "internal" import.

It’s affected by baseUrl. You may want to remove that setting once #40101 is merged. I also plan to explore adding a setting that makes it so that imports within a project prefer relative imports.

That said, I want to keep this thread focused on the original problem—there’s a lot of other auto-import work to be done in this release, and it will get harder for me to track if all the different issues become about auto-imports broadly. The only thing relevant to this issue is the fact that you got the Constants import both from build and from src. If you can definitely reproduce this with a recent nightly, is this an open source repo I could clone to reproduce? That would be very helpful. Thanks!

[AlCalzone]

can you confirm which nightly you’re using?

20200909

I also plan to explore adding a setting that makes it so that imports within a project prefer relative imports.

That would be lovely!

is this an open source repo I could clone

Yep :)

1. clone https://github.com/AlCalzone/node-zwave-js/tree/c48522b5c9365428a50493f723f99111271f8dca
2. npm install
3. open packages/zwave-js/src/lib/commandclass/AlarmSensorCC.ts
4. delete line 14: import { MessagePriority } from "../message/Constants"; - you'll get an error in line 163
5. toggle quick fixes on MessagePriority
   

[andrewbranch]

@AlCalzone something in your build produces a bunch of .d.ts files in the root level of packages/zwave-js that import directly from build. These root-level declaration files are not excluded by your tsconfig, so these outputs become future inputs. So, this behavior is expected, although we have #40195 to track adding an error/warning for this, since a lot of people end up in this situation accidentally. I didn’t dig into your build process to understand whether it’s intentional or not in your case.

[AlCalzone]

Thanks, that explains it. In my case it is intentional, because it simulates Node.js's sub-module exports.

[andrewbranch]

In that case, you probably just need to tweak your tsconfig.json include/exclude to make sure the editor doesn’t see those files.

[AlCalzone]

@andrewbranch That seems to work :)