Avoid barrel re-exports in auto-imports

[TheColorRed]

Bug Report

🔎 Search Terms

• autoImportFileExcludePatterns

Bug

When using the setting typescript.preferences.autoImportFileExcludePatterns, it works, however, in a mono-repo I cannot import from another package's root without using the full path to the file when excluding index.ts files.

{
  "typescript.preferences.autoImportFileExcludePatterns": [
    "./packages/*/src/index.ts"
  ],
}

I have a workspace package.json that looks like this:

{
  "name": "workspaces",
  "version": "1.0.0",
  "workspaces": [
    "packages/core",
    "packages/a"
  ]
}

In the package.json of packages/core I have this:

{
  "name": "@org/core",
  "main": "./src/index"
}

Then in the packages/a, I try to import a file from packages/core, but autoimport imports something like this even though it is exported through the main file:

import { MyClass } from '@org/core/src/classes/my-class';

The package.json for packages/a

{
  "name": "@org/a",
  "main": "./src/index"
}

The tsconfig.json for both:

{
  "extends": "../tsconfig.base.json",
  "files": [
    "./src/index.ts"
  ]
}

Expected Result

This might be a feature request, but I think it would be great if barrel files could be ignored in the auto-import if within the same module/package, as this can often lead to circular dependencies that are hard to track down.

[RyanCavanaugh]

Can you set up a sample repo, and also clarify the "expected result" ? Are you expecting a different import path to be generated, or not to be shown that import suggestion at all?

[TheColorRed]

@RyanCavanaugh I have created the repo: https://github.com/TheColorRed/vscode-autoImportFileExcludePatterns

Steps:

1. Clone the repo
2. npm i
3. In packages/helper/src/index.ts type readF and select the one that is in @org/core from the Intellisense popup.

You will then notice the auto import imports the full path to the file:

import { readFile } from '@org/core/src/files';
readFile

Now if you turn off the setting typescript.preferences.autoImportFileExcludePatterns in .vscode/settings.json and use Intellisense, the import works the way I intended:

import { readFile } from '@org/core';
readFile

Having this setting works the way I want when referencing a file in the current mono-repo project (aka when @org/core or @org/helper uses one of its own files). But when @org/helper uses a file from @org/core the wrong import is used.

Here is what I am looking for:

@org/core     -> import { readFile } from './files';
@org/helper   -> import { readFile } from '@org/core';

Here is what is happening:

@org/core     -> import { readFile } from './files';
@org/helper   -> import { readFile } from '@org/core/src/files';

[ssalbdivad]

I ran into a similar issue with this. It would be great if there were a way to exclude "index.ts" from being suggested directly while still allowing other specifiers that resolve to a file called "index.ts".

[ssalbdivad]

@andrewbranch Any chance this could fit in with some of the ongoing auto-import improvements?

Would make a huge difference to me as currently I either get imports from my main.ts entrypoints within a single package, causing circularities, or cannot get auto imports from other packages in the monorepo.

[andrewbranch]

This is super working-as-intended, and the desired behavior I think clearly doesn’t map onto autoImportFileExcludePatterns, unless you had a multi-root workspace and had different VS Code settings for each TS project, which would let you exclude your own index.ts files in each workspace root without excluding any other package’s index.ts files.

Auto-imports has a preference toward barrel files because they usually indicate the most public export of something and result in the shortest path. I would like to move away from this preference since barrel files often make build tools work poorly, but there needs to be a stronger proposal for how to make this shift, and it needs to be motivated by objective reasons, not code style aesthetics.

In the example project here, if you don’t want to import from '@org/core/src/files', then that package should declare an "exports" that prevents such a path from being resolvable. After that, if we had a setting for “avoid barrel re-exports,” it would do the job, because in the local project, you might have many ways to access some export, including a barrel file, but there would only be one way to access everything from '@org/core'.

[andrewbranch]

I feel instinctively opposed to a setting like “avoid barrel re-exports in my own files but not from my dependencies.” There are many reasons barrels are bad; the circularity problem is just one. If auto-imports is making a circularity that could have been detected, or at least looks suspicious, we should try to fix that universally, not behind a setting. I did some work like that in #47516. But given all the other barrel problems, it begs two questions:

1. Why would you want to import from barrels in node_modules? Your bundler and other tools are probably going to work better and faster if you use the deepest / most specific imports the package allows.
2. Why write them in your own code?

To me, this makes it feel like we kind of don’t need any additional settings. If you want short import paths from node_modules but you don’t want to import from your own barrels, maybe you should stop writing barrels in your code. On the other hand, if we changed settings for everyone to avoid barrels everywhere, and people started complaining about getting unwieldy auto-import paths for their dependencies, then maybe those dependencies should define "exports" that declare what subpaths are allowed.

[ssalbdivad]

@andrewbranch Maybe I should have described my use case more clearly but I don't use barrel files (or at least in the sense I normally hear them described). I only have a single main.ts file for each package which constitutes its public API- there are no nested barrel files I ever want to use internally. Is there an alternative to this for a package that wants to publish a top-level API?

I have a paths config like this that allows me to avoid having to build to see my runtime or type changes reflected e.g. in tests:

		"paths": {
			"arktype": ["./ark/type/main.ts"],
			"@arktype/util": ["./ark/util/main.ts"],
			"@arktype/fs": ["./ark/fs/main.ts"],
			"@arktype/schema": ["./ark/schema/main.ts"],
			"@arktype/attest": ["./ark/attest/main.ts"]
		}

If you have another suggestion as to how to achieve this that doesn't result in the problematic suggestions but does allow importing from a path like @arktype/schema that resolves to the entrypoint of another package in the monorepo, I'd definitely be open to it!

[ssalbdivad]

@andrewbranch Rereading your second comment, I understand more what you're suggesting now, but top-level discoverability for the API of a library feels valuable.

I would definitely not want to give that up just to avoid some obnoxious auto-imports that worst-case scenario I can manually fix. Granted it is grating given how often these kinds of imports occur, but I don't know if I can recall a library that doesn't provide some top-level export and I think there are good reasons for that.

[andrewbranch]

IMO you should never use paths. Use package manager workspaces and set up project references. That’s enough to not have to build to see type changes in the editor—references to project reference output files get transparently redirected to input files in the IDE so you get instant IntelliSense updates across projects. For fast test/dev runtime workflows, I add conditional "exports" on top of this:

{
  "exports": {
    "ts": "./src/main.ts",
    "default": "./dist/main.js"
  }
}

then set your test runner / whatever to use the ts condition.

paths goes through some pretty different auto-import code paths so I’m not sure how it might be affecting your experience.

[ssalbdivad]

@andrewbranch I actually had this kind of setup originally but it felt like a lot of extra ceremony for no extra benefit compared to using paths, at least within my own repo.

I also had the same problem with autoimports with that set up.

[ssalbdivad]

@andrewbranch Any new ideas on this one?

I've been talking with @colinhacks about strategies for developing in a TS monorepo with no build step as documented in his blog post here.

This is one of the biggest downsides to either of these approaches. I don't see any way around having at least one "barrel file" for your API entry point that doesn't involve freezing your file structure in order to have a stable release.

[andrewbranch]

I’m open to trying a preferDeepImports preference that applies universally, no carve-outs for local files vs. node_modules.

[ssalbdivad]

@andrewbranch I'm sure that could be useful in some situations but I expect it would result in imports resolving to arbitrary source files across packages in a setup like the one I'm describing.

I'm far from committed to tsconfig paths if there's a better way to achieve this that avoids the autoimports issue. What I'm really wondering is do you...

1. Agree a config setup that avoids having to build in dev is a big win?
2. Know of a better method than those mentioned by Colin to achieve this (specifically tsconfig paths paralleling your package names or customConditions)?
3. Have any suggestions for how to autoimport from a package entry point when outside the package itself in a setup like this?

My first intuition was that perhaps typescript.preferences.autoImportFileExcludePatterns or another setting could support a pattern that would be checked against the import specifier rather than the resolution, but maybe there is a technical reason that is not possible or desirable?

[andrewbranch]

I’ve been using customConditions in the arethetypeswrong monorepo from the beginning to get no-build updates in the Vite dev server. I just don’t agree that configuring that is unreasonably burdensome. The entire JavaScript ecosystem has been given a huge gift in that all major tools adopted conditional package.json "exports" almost to spec. It’s not a perfect system, but it’s pretty good, and this is a perfect application for it.

a pattern that would be checked against the import specifier rather than the resolution, but maybe there is a technical reason that is not possible or desirable

No, it’s both possible and desirable, just nitpicky design questions remain: #55092

[ssalbdivad]

Yeah customConditions is fine with me. It essentially serves the same purpose in my case and I can see how in other scenarios it would be less likely to cause problems getting out of sync, so I can see why you'd recommend it.

I think for now it would fall victim to the same underlying problem, I will keep an eye on the issue and add some context there.

[andrewbranch]

I think for now it would fall victim to the same underlying problem

If I’m understanding you correctly, it just depends on how you set up the "exports" API. You can choose whether you expose deep imports or not. If you just want to have a single package entrypoint that exports everything, you can do that, and then auto-imports within that package will work normally, importing from local files, and imports outside that package will only ever be able to refer to the top-level package specifier. If you choose to expose deep imports on the package by defining many subpaths or a wildcard subpath, along with "." re-exporting everything, then yes, auto-imports will have to make a decision whether to import from the package’s "." or other subpaths, which would be affected by the preference I’m proposing. But I still assert that if you want “deep imports” in some places, you have reasons to want them everywhere 😄 Am I missing something? I don’t really see what the problem is.

[ssalbdivad]

@andrewbranch What you described does explain how preferDeepImports could solve this problem with either setup. That sounds like it could work perfectly as long as it doesn't try to cross package boundaries directly (which I assume it wouldn't since I don't that occurs currently).

What I meant about the same underlying problem though is that given that such a setting doesn't exist currently, both tsconfig paths and package.json exports would be affected by this issue, correct?

[andrewbranch]

One major difference between exports and paths is that exports block the resolution of paths not explicitly listed, whereas paths is purely additive. So already, you can use customConditions and "exports" to limit how a package can be imported externally, whereas paths does not really afford you that ability.

[ssalbdivad]

@andrewbranch Sure but I'm already using exports for that reason and I mostly just have one entry point per package so this doesn't really impact me.

[andrewbranch]

Then I don’t understand what problem you’re currently having 😅 The OP complained about auto-imports like '@org/core/src/classes/my-class' which shouldn’t be possible if your equivalent of @org/core/package.json has an "exports" (assuming you’re using a modern moduleResolution setting)

[ssalbdivad]

I have a package arktype that depends on @arktype/schema. @arktype/schema has a primary API entrypoint at ark/schema/api.ts.

To avoid importing from the API entrypoint from within @arktype/schema, which would often create cyclic imports, I can add the following config:

	"typescript.preferences.autoImportFileExcludePatterns": [
		"./ark/schema/api.ts"
	]

However, if I do this, I no longer get autoimports from @arktype/schema in arktype as @arktype/schema, but rather from the nested file, even though it is not exported in @arktype/schema's package.json.

Maybe it is because I only have a single root-level tsconfig that is shared by both packages during dev?

EDIT: it seems like this happens even if I create a tsconfig.json in each individual package.

[andrewbranch]

If you rename api.ts to index.ts and delete the autoImportFileExcludePatterns you should be good to go 😬

[ssalbdivad]

@andrewbranch Is index.ts hardcoded? 😅

I wanted to avoid it because of the association with directory imports but I would definitely switch back if it is the only way to get that behavior for now.

[andrewbranch]

TypeScript/src/services/codefixes/importFixes.ts

Lines 1344 to 1358 in 4ada270

	// This is a simple heuristic to try to avoid creating an import cycle with a barrel re-export.
	// E.g., do not `import { Foo } from ".."` when you could `import { Foo } from "../Foo"`.
	// This can produce false positives or negatives if re-exports cross into sibling directories
	// (e.g. `export * from "../whatever"`) or are not named "index".
	function isFixPossiblyReExportingImportingFile(fix: ImportFixWithModuleSpecifier, importingFilePath: Path, toPath: (fileName: string) => Path): boolean {
	if (
	fix.isReExport &&
	fix.exportInfo?.moduleFileName &&
	isIndexFileName(fix.exportInfo.moduleFileName)
	) {
	const reExportDir = toPath(getDirectoryPath(fix.exportInfo.moduleFileName));
	return startsWith(importingFilePath, reExportDir);
	}
	return false;
	}

It was originally intended primarily to avoid module specifiers like "../.." because they look code smelly, but it was later expanded slightly to deal with circularities in more cases. It might make sense to remove the index filename part of this now.

[ssalbdivad]

@andrewbranch I think that works! 🎉 Thank you so much- that has been plaguing me since long before I commented on this a year ago 😅

Would definitely be interested if other file names are supported so I can switch back, but I care 95% less about that than being able to import across packages easily.

[andrewbranch]

It actually might be possible to do this cheaply without heuristics, as long as the file you’re currently editing exports something... the data structure central to auto imports can answer whether an arbitrary symbol exported from your current file can also be imported from the file we’re generating an import to, detecting a circularity. We’re well set up to do that for individual exports, just not whole modules, so we’d do well for export * but could fail to detect cases like export { JustOneThing } from "./foo"...

[ssalbdivad]

I'm generally exporting a specific subset of symbols in a package entry point to make the API as explicit as possible.

The index.ts has such marginal downside I'm unlikely to think about this again unless I specifically read somewhere that other files can be handled the same way by autoimports.

I guess the biggest downside is discoverability as I never would have guessed changing the file name would fix this.