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

Offer auto-imports for all files in package.json exports wildcard directories #53116

Closed
pilaoda opened this issue Mar 3, 2023 · 27 comments · Fixed by #54831, #55815 or #56848
Closed

Offer auto-imports for all files in package.json exports wildcard directories #53116

pilaoda opened this issue Mar 3, 2023 · 27 comments · Fixed by #54831, #55815 or #56848
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@pilaoda
Copy link

pilaoda commented Mar 3, 2023

My npm lib module's package.json

  "name": "my-lib",
  "exports": {
        ".": {
            "require": "./dist/index.js",
            "types": "./dist/index.d.ts"
        },
        "./*": {
            "require": "./dist/*.js",
            "types": "./dist/*.d.ts"
        },
        "./**/*": {
            "require": "./dist/**/*.js",
            "types": "./dist/**/*.d.ts"
        }
    },

An app depends on this npm module and install it.
The vscode auto import only support types in import {A} from "my-lib" resolve to my-lib/dist/index.d.ts, and could not find the symbol and type in something like import {B} from "my-lib/utils" resolve to my-lib/dist/utils.d.ts.

@mjbvz mjbvz transferred this issue from microsoft/vscode Mar 6, 2023
@mjbvz mjbvz removed their assignment Mar 6, 2023
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Mar 6, 2023
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.1.0 milestone Mar 6, 2023
@DanielRosenwasser
Copy link
Member

I might be mistaken, but I'm fairly certain that ** is not a supported subpath pattern in Node.js.

So are you saying that the issue is that even though this package.json is incorrectly configured, TypeScript is not resilient in providing correct auto-imports?

@pilaoda
Copy link
Author

pilaoda commented Mar 7, 2023

Sorry I just learn that ./**/* is no need and only ./* can be enough for this situation.
But even thouth I delete the unnecessary ./**/* pattern and keep the ./* pattern, the auto import still not working for something like import {B} from "my-lib/xx/utils" which should resolve to ./node_modules/my-lib/dist/xx/utils.d.ts.
If I import it manually, it will be compiled successfully.
The real problem I think is that the ts language server in editor only search index.d.ts but ignore * pattern searching in a node_modules package.

@pilaoda
Copy link
Author

pilaoda commented Mar 7, 2023

More detailed example, my project has below structure:

- node_modules
   - my-lib
      - dist
         index.d.ts
         utils.d.ts
      package.json
- src
   main.ts

index.d.ts:

export declare const A: number;

utils.d.ts:

export declare const B: number;

package.json

{
  "name": "my-lib",
  "exports": {
        ".": {
            "require": "./dist/index.js",
            "types": "./dist/index.d.ts"
        },
        "./*": {
            "require": "./dist/*.js",
            "types": "./dist/*.d.ts"
        }
    }
}

During my developing in src/main.ts, If I type "A", the editor will help me auto add import {A} from "my-lib". If I type "B", the editor don't know where the symbol B is, so I need to add import {B} from "my-lib/utils" manually.

@andrewbranch
Copy link
Member

This is a known limitation for performance reasons. Originally, auto-imports were never available from files that weren’t already in the program in some way (for files in external libraries, this usually means transitively referenced by a local project file). Then, we started scraping package.json dependencies to see what else we can offer that’s not already in the program. But when we do that, we only look at the package’s entry points (main/types and exports) and files transitively referenced by those entry points, so we can avoid arbitrarily deep FS hits.

For a package that doesn’t define exports, this means anything not transitively referenced from main/types is not offered for auto-import, despite being reachable by a manual import—so this limitation applies not only to packages with wildcard exports but also to packages with no exports.

Fully lifting this limitation would effectively mean doing a recursive read on all of every project’s direct dependencies. We could make up an arbitrary rule that says we’ll do this recursive read only if an exports wildcard directs us to do so, since the intent to make modules available is explicit there, leaving packages without exports out. But I think an explicit list of exports is generally considered better practice, and I don’t mind privileging packages that tell us up front what work we’re going to do. @DanielRosenwasser thoughts?

@andrewbranch andrewbranch changed the title Typescript auto import not support ** glob pattern in "exports" field of package.json Offer auto-imports for all files in package.json exports wildcard directories Mar 7, 2023
@andrewbranch andrewbranch added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Needs Investigation This issue needs a team member to investigate its status. labels Mar 7, 2023
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Mar 7, 2023

I think we would need to be careful here - before making this change, I would want to see an expansion of the TSServer perf suite that uses the artificial test cases you've used to measure auto-import performace. But the auto-import program looking through files covered under explicit subpath patterns in exports doesn't sound like the craziest thing.

@andrewbranch
Copy link
Member

Does the TS Server perf suite report how many completion entries are returned? We will definitely do more work if we find more modules to auto-import from, but there’s a certain level of pay-to-play that’s acceptable there. What we want to avoid is paying a noticeable cost when the response is no better.

@DanielRosenwasser
Copy link
Member

I believe so - here's what it looks like today:

xstateTSServer - node (v18.10.0, x64)
Req 1 - updateOpen 3,146ms (± 0.30%) 3,037ms (± 0.44%) 🟩-109ms (- 3.48%) 3,020ms 3,054ms p=0.005 n=6
Req 2 - geterr 1,611ms (± 0.63%) 1,582ms (± 0.59%) -29ms (- 1.78%) 1,574ms 1,598ms p=0.008 n=6
Req 3 - references 103ms (± 1.46%) 107ms (± 2.02%) +3ms (+ 3.23%) 104ms 110ms p=0.019 n=6
Req 4 - navto 361ms (± 1.22%) 357ms (± 0.18%) -4ms (- 1.06%) 356ms 358ms p=0.049 n=6
Req 5 - completionInfo count 3,136 (± 0.00%) 3,136 (± 0.00%) ~ 3,136 3,136 p=1.000 n=6
Req 5 - completionInfo 427ms (± 1.10%) 422ms (± 1.46%) ~ 415ms 430ms p=0.147 n=6

@pilaoda
Copy link
Author

pilaoda commented Mar 8, 2023

If it is a performance trade-off, maybe provide some config for those user who has small codebase or good hardware to take this performance cost would be ideal? Since the auto import may improve the productivity obviously rather than this cost.

@mrmckeb
Copy link

mrmckeb commented May 18, 2023

I was debugging this issue today in a monorepo and I wanted to share a few thoughts. As a user, I had expected this to work and started by reviewing our implementation before I came across this issue.

I 100% understand the performance concerns and respect the decision by the team, but it might be helpful to allow projects to opt in/out as @pilaoda suggested.

It would also be great if TS could warn about this. I'm not sure how that should look, but it would be powerful if I - as a user or an author - received information about this issue and how to fix it. I guess this could be similar to the error for missing types on exports.

For now we'll add a script to add exports per module, which will solve this for our monorepo users.

@andrewbranch
Copy link
Member

I plan to measure how costly it is to enable this for wildcard "exports" (but not for packages with no "exports"). It’s going to miss the 5.1 release, though.

@andrewbranch andrewbranch added Rescheduled This issue was previously scheduled to an earlier milestone Needs Investigation This issue needs a team member to investigate its status. labels May 18, 2023
@typescript-bot typescript-bot added Fix Available A PR has been opened for this issue labels Jun 29, 2023
@mrmckeb
Copy link

mrmckeb commented Jul 10, 2023

Thanks @andrewbranch, amazing work. Will this be in 5.2 at this stage?

@andrewbranch andrewbranch added Bug A bug in TypeScript and removed Suggestion An idea for TypeScript In Discussion Not yet reached consensus Needs Investigation This issue needs a team member to investigate its status. Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone labels Sep 21, 2023
@typescript-bot typescript-bot added Fix Available A PR has been opened for this issue labels Sep 21, 2023
@andrewbranch
Copy link
Member

andrewbranch commented Sep 21, 2023

@delaneyb there was a bug around the .d.ts extension. A workaround until TS 5.3 is out is to refactor something like

"./components/*": {
  "types": "./lib/components/*.d.ts",
  "default": "./lib/components/*.js"
},

into

"./components/*": "./lib/components/*.js"

TypeScript doesn’t actually need the types key in this case since the .js and .d.ts files are siblings. It always knows how to make that extension substitution automatically.

jpwilliams added a commit to inngest/inngest-js that referenced this issue Sep 25, 2023
Types are automatically inferred as sibling `.d.ts` files here.
See microsoft/TypeScript#53116 for extra pre-latest info
@BarakChamo
Copy link

@andrewbranch sorry to comment here but I'm facing an issue that is very relevant and it seems this issue has been reopened previously to address other issues and bugs and the context for this problem is well explained here.
Happy to also open this as a separate issue.

To briefly explain the issue I'm facing:

project structure

I'm working with a very standard monorepo structure using bun, turbo, typescript and next.js, intended to be used as a boilerplate for several projects that are deployed on Vercel. The project is set up as a monorepo in the root package.json:

// root package.json
"workspaces": ["apps/*", "packages/*"]

Both the apps and packages are set up with "moduleResolution": "Bundler" to take advantage of ts support for the exports field while not having to specify explicit extensions, and the packages use wildcards to maintain dev flexibility inside each package like so:

// packages/ui/package.json
"exports": {
"./*": {
	"types": "./src/*/index.tsx",
	"default": "./src/*/index.tsx"
},
"./tailwind": "./tailwind.config.ts"
},

I've also tried a simpler format:

"exports": {
"./*": "./src/*/index.tsx",
"./tailwind": "./tailwind.config.ts"
},

This is following a recommended strucutre from vercel's examples on setting up turbo a monorepo

This project is set up with internal packages that are not independently compiled, so it exports the .ts and .tsx files, rather then point to a dist. This is in line with both vercel examples (as shared) and recommendations from turbo, and works quite well to speed up development cycles and internal flexiblity in the monorepo. The link between the apps and packages is managed via next.js's transpilePackages:

transpilePackages: ['@this/ui'],

Project issues

While the monorepo compiles and deploys to vercel just fine, I'm unable to take advantage of intellisense suggestions and auto imports in VS Code using the current typescript setup. It may well be that I'm messing up some other settings but did my best to follow recs and other threads on this topic and unable to get these to work in conjunction with wildcard exports. The only reference I found to the development of exports support for wildcards is on this issue which is why I'm commenting here.

Once an import is written manually, it is typed correctly, so clearly typescript is able to resolve the types once they've been coded, but while suggestions and auto imports do work inside the packages or the apps, they don't work across packages.

I may be wrong in assuming the fact that the packages export .tsx files is the cause for this issue, but following this thread made me think that's a possibility. Since these flows are endorsed by next.js and turbo and are common in lerna repos, this seems like something that should be work with typescript.

As you can see, imports and completion from an import path are working, but auto import suggestions are not:
Screenshot 2023-12-21 at 8 05 02 PM
Screenshot 2023-12-21 at 8 05 32 PM
Screenshot 2023-12-21 at 8 06 01 PM

I'm sharing a small reproduction of the issue based on the vercel example I mentioned:
issue reproduction

The project structure:

repo
- apps
- - web
- packages
- - ui

packages/ui/package.json

{
	"name": "@this/ui",
	"version": "0.0.0",
	"private": true,
	"scripts": {
		"generate:component": "turbo gen react-component",
		"story": "storybook dev -p 6006",
		"build-storybook": "storybook build"
	},
	"sideEffects": false,
	"type": "module",
	"exports": {
		"./*": {
			"types": "./src/*/index.tsx",
			"default": "./src/*/index.tsx"
		},
		"./tailwind": "./tailwind.config.ts"
	},
	"files": ["src"],
}

packages/ui/tsconfig.json

{
	"$schema": "https://json.schemastore.org/tsconfig.json",
	"compilerOptions": {
		"rootDir": ".",
		"baseUrl": "src",
		"esModuleInterop": true,
		"forceConsistentCasingInFileNames": true,
		"isolatedModules": true,
		"incremental": true,
		"composite": true,
		"module": "ESNext",
		"moduleResolution": "Bundler",
		"preserveWatchOutput": true,
		"skipLibCheck": true,
		"strict": true,
		"jsx": "preserve",
		"paths": {
			"@/*": ["./*"],
		}
	},
	"exclude": ["node_modules"],
	"include": ["src/**/*"],
}

apps/web/package.json

{
	"name": "web",
	"version": "1.0.0",
	"private": true,
	"scripts": {
		"dev": "next dev",
		"build": "next build",
		"start": "next start",
		"lint": "eslint . --max-warnings 0"
	},
	"dependencies": {
		"@this/ui": "workspace:*",
	}
       .....
}

apps/web/tsconfig.json

{
	"$schema": "https://json.schemastore.org/tsconfig.json",
	"compilerOptions": {
		"baseUrl": ".",
		"rootDir": ".",
		"target": "es5",
		"lib": ["dom", "dom.iterable", "esnext"],
		"allowJs": true,
		"skipLibCheck": true,
		"strict": true,
		"forceConsistentCasingInFileNames": true,
		"allowImportingTsExtensions": true,

		"noEmit": true,
		"esModuleInterop": true,
		"module": "ESNext",
		"moduleResolution": "Bundler",
		"resolveJsonModule": true,
		"isolatedModules": true,
		"jsx": "preserve",
		"incremental": true,
		"paths": {
			"@/*": ["src/*"]
		},
		"plugins": [{ "name": "next" }]
	},
	"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", "tailwind.config.mts", ".next/types/**/*.ts"],
	"exclude": ["node_modules"],
	"references": [
		{
			"path": "../../packages/ui"
		}
	]
}

@BarakChamo
Copy link

BarakChamo commented Dec 21, 2023

Adding another simple way to recreate this issue:

Create a turbo monorepo with the following:
npx create-turbo@latest

This will create a packages/ui package with names exports in package.json.
With this setup, auto imports work just fine.

If you switch from named exports to a single wildcard:

  "exports": {
    "./*": "./src/*.tsx"
  },

All existing imports still work, the project builds fine, but suggestions stop working in VS Code.

@andrewbranch
Copy link
Member

@BarakChamo thanks for the detailed report. The previous implementation only looked for declaration files, which redirect to implementation source files only when project references are set up. Fixed at #56848.

Bit of a tangent, but it’s still weird to me that these starters and third party docs don’t set up project references just to improve type checking performance. @anthonyshew is that something you considered, or does it not play well with turborepo’s own caching mechanisms?

@Andarist
Copy link
Contributor

Bit of a tangent, but it’s still weird to me that these starters and third party docs don’t set up project references just to improve type checking performance. @anthonyshew is that something you considered, or does it not play well with turborepo’s own caching mechanisms?

Randomly chiming in... 😅 One - kinda big - DX downside of project references is that you must maintain them "manually". They are not derived from package.json etc. So you have to educate users about this. I have scripted this in the past in some of my projects but it's still quite annoying and at odds with how most of the other tools work. Usually, relationships like this are derived from package.json + some minimal-ish convention-based hints might also be required for the project but they often don't require ongoing maintenance when the dep graph naturally evolves.

Another thing is that sometimes I found the .tsbuildinfo being somewhat over-cached~. I guess that can be said about almost any caching mechanism like this but I found myself having to clear those files more often than having to clear, let's say, .next cache.

Recently-ish I also realized that within the IDE it still might not always end up speeding that much since a change in a file has to rebuild the program and all of the imported modules, including the types from other in-monorepo packages. In other words, the in-memory cache is cleared quite a bit in a somewhat surprising (for the end user) manner.

@pilaoda
Copy link
Author

pilaoda commented Dec 21, 2023

Randomly chiming in... 😅 One - kinda big - DX downside of project references is that you must maintain them "manually". They are not derived from package.json etc. So you have to educate users about this. I have scripted this in the past in some of my projects but it's still quite annoying and at odds with how most of the other tools work. Usually, relationships like this are derived from package.json + some minimal-ish convention-based hints might also be required for the project but they often don't require ongoing maintenance when the dep graph naturally evolves.

Can't agree more! Project reference should be derived from package.json to be more maintainable. BTW, project reference even report error in some monorepo with symbol link if you open it in a different linked folder level which failed to find the right relative path.

@anthonyshew
Copy link

@andrewbranch, @Andarist pretty much wrote my answer. A couple other perspectives to sprinkle on from Turborepo-land:

  1. There's already so many configuration points to handle in a TypeScript monorepo. Project References add another "yet another thing to get wrong" so we punt the problem altogether noting the perf tradeoff Mateusz described.

  2. When the over-caching by TS happens in a Turborepo, folks think it's us (since we cache stuff) but then I have to teach why it ends up being the TS cache. I, of course, don't mind helping educate, but find I want to punt the issue again because of the aforementioned perf tradeoff.

  3. Last, a personal skill issue problem: I personally don't have enough experience with Project References to confidently talk about them. I've never had type checking slowness in the repos I work in where Project References seemed to do much so I simply don't have much exposure. We have a fairly large monorepo at Vercel and, while we have some things I'd like to improve to be faster, IDE type checks aren't one of them. Having to fuddle around with References would end up being more trouble than it's worth.

Sidebar to the sidebar: I wasn't aware of #56848! Awesome! Definitely will incorporate that into our examples when it releases. Between that and package.json#imports,that's two huge improvements for questions I get frequently in TypeScript monorepos!

@puchm
Copy link

puchm commented Apr 5, 2024

Some guidance for anyone having this issue in VS Code: There is a setting called typescript.preferences.includePackageJsonAutoImports, which is set to auto by default. This apparently enables / disables the setting dependent on the estimated performance impact. From what I have tested it depends a lot on how many dependencies you have.

You can have it always enabled by setting it to on.

Edit: Also, you should make sure VS Code is using the right version of Typescript. It may be using a version that is different from the one in your project. You can check that by clicking the {} in the bottom right corner next to "Typescript" while having a TS file open. That is also where you can change it.

@EfstathiadisD
Copy link

EfstathiadisD commented May 1, 2024

Some guidance for anyone having this issue in VS Code: There is a setting called typescript.preferences.includePackageJsonAutoImports, which is set to auto by default. This apparently enables / disables the setting dependent on the estimated performance impact. From what I have tested it depends a lot on how many dependencies you have.

You can have it always enabled by setting it to on.

Edit: Also, you should make sure VS Code is using the right version of Typescript. It may be using a version that is different from the one in your project. You can check that by clicking the {} in the bottom right corner next to "Typescript" while having a TS file open. That is also where you can change it.

That kind of fixed it, but it imports it from build dir not the exports dir. Any ideas? Below is the package.json of the imported package and after that the autoimport. The correct one should be @silverstone/utils/cn.

Screenshot 2024-05-01 at 12 04 42 Screenshot 2024-05-01 at 12 05 50

@jonahallibone
Copy link

Some guidance for anyone having this issue in VS Code: There is a setting called typescript.preferences.includePackageJsonAutoImports, which is set to auto by default. This apparently enables / disables the setting dependent on the estimated performance impact. From what I have tested it depends a lot on how many dependencies you have.

You can have it always enabled by setting it to on.

Edit: Also, you should make sure VS Code is using the right version of Typescript. It may be using a version that is different from the one in your project. You can check that by clicking the {} in the bottom right corner next to "Typescript" while having a TS file open. That is also where you can change it.

This actually solved a multi-day problem for me, so thanks!

I am wondering how auto works though. Below is my package exports, which yes, does resolve a ton of icons. But it also turned off the importing for /dynamic which is a single file. This lead me to believe it just turns off autoimport for the entire package. But, if I used "./*: "./dist/icons*.js" i could get the icon imports work at the root of the package, but would once again break any other imports for the package. So clearly it's not entirely turned off but does seem to turn off any subpath imports entirely.

Image

This leads to my questions: What is the behavior of auto? Does it just look at depth of the import path mapping? If so, that's kind of a strange heuristic to use and lead to (an understandably) frustrating experience. Is it possible to address how this setting works such that imports from packages using an export field are more predictable?

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