search

microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.03k stars 12.49k forks source link

Allow Intellisense autocomplete for star imports, for specific source files in your repository #49895

Open crazytoucan opened 2 years ago

crazytoucan commented 2 years ago

Problem statement

Hey TypeScript team, I've huge respect for the software you've created for the community. I'd like to submit a request for some mechanism that improves the developer experience when trying to import * as for a module in your own codebase that you control. For example, if I have a file called apiTypes.ts, for there to be a mechanism that hints to VSCode that within this codebase, you'd prefer to import this file using star-imports (import * as apiTypes) rather than a named import (import { User } from …). In other words:

// apiTypes.ts
export interface User {
  name: string;
}
// fooComponent.ts
import * as apiTypes from "./path/to/apiTypes.ts";

interface Props {
  user: apiTypes.User;
//      ^^^^^^^^^^^^^
}

To be clear, the specific request here is:

As somebody who has managed some very large codebases (1MM+ lines), the following observations are helpful for me to recognize:

  1. For a given module, it's extremely helpful for its import occurrences to be consistent (* as foo from "./givenModule" vs { Named } from "./givenModule"). It's likewise extremely helpful for the symbol used for its star-imports to be consistent (e.g. import * as api vs import * as apiTypes vs …)
    • Telling developers "oh, you should import this file this way, but other files this other way" is brittle in practice
    • While I understand there are other ways to solve this problem, like lint rules + refactorings, it really breaks the devX flow when you hit a symbol like User| to either have to know you need to go add your own import by hand or using a snippet, or to eat that you're temporarily writing "nonconformant" code and to remember to run a refactoring, wait for a lint fixer, etc.
  2. The use-case that I'm really modeling this request around is for modules that describe an API. I'd love to be able to write code that looks like resourceApi.Resource vs energyApi.Resource.
    • One alternative is to prefix all symbols in the module, like ResourceApiResource or ResourceApi_Resource, but both lead to verbose-looking symbols that may be surprising for developers unfamiliar with your team's practices to understand what's going on at a glance. In contrast, star-imports are well-understood by developers
  3. You can accomplish something almost equivalent using TypeScript namespaces, e.g. export namespace energyApi {}, although it seems we're trying to move away from namespaces in practice, and they perform poorly RE: tree-shaking for runtime symbols.
    • The runtime tree-shaking/dead-code-removal aspect is important for things like an Icons file or http endpoints files where you wouldn't necessarily want your final bundle to package all of the symbols.
    • Namespaces like this don't allow autocomplete when importing their members, like Resource| -> Ctrl+Space to pull up energyApi.Resource. You'd first have to energyApi -> Ctrl+Space -> Enter -> .R -> Enter

Suggestion

In an effort to simplify this problem and to be generative, how about a module-level @pragma directive to hint this behavior? For instance, the below top-of-file comment could be used by Intellisense to add an entry for this file, and all of its exports, to be imported as apiTypes.User instead of the named { User } import.

// apiTypes.ts
/** @preferNamespaceImport apiTypes */

export interface User {
  name: string;
}

Said in different words, for files that contain a @preferNamespaceImport directive, Intellisense would choose to import those files using star-imports (using whatever symbol is specified in the pragma), rather than as destructured named imports.

Naturally, this suggestion doesn't extend to third-party imports, like preferring lodash to be imported * as lodash or * as _, etc. Again, this proposal isn't meant to be exhaustive, but in the codebases I've worked in, something like the pragma directive would go a long way to improving devX and encouraging consistent codebases

Past work and references

I recognize many iterations of this idea have been proposed in the past, so I'd like to link them here for completeness and to promote healthy discussion

🔍 Search Terms

Star imports, namespace, intellisense, autocomplete

✅ Viability Checklist

My suggestion meets these guidelines:

RyanCavanaugh commented 2 years ago

There's a little confusion here because we actually have two features:

Things I like about this:

Things I don't like:

octogonz commented 2 years ago
  • Namespacyness should not be up to the exporter's file. This is just kind of weird IMO -- a module doesn't reasonably care how it gets used

Counterpoint, in a large code base there is a need for API names to be consistent and somewhat unique.

Some background

Consider this example:

import { combine } from './filePath.ts';

combine(x,y);

Suppose that someone needs to search for all calls to the combine() function, maybe because they fixed a bug and want to audit all the various usages, to assess what should be tested. (🏆) Or maybe because we're redesigning that API and need to understand the consequences of our proposed change.

IntelliSense can help find references to combine(), but not really in a code base with 30,000+ source files split across multiple Git repos. And not in other contexts where someone might go looking for this function:

For these requirements, "combine" is a really unfortunate string to search for. Most of the search results are likely to be accidental matches of something unrelated.

One solution

Our coding conventions strongly encourage a longer name such as FilePath.combine():

import { FilePath } from './FilePath.ts';

FilePath.combine(x, y);

We also encourage the longer name to include a container like FilePath. (Admittedly it can be awkward to impose meaningful groupings onto your functions, basically an arbitrary K-means clustering, but in practice this exercise tends to produce better names because it makes people think about the relationships between their code. In academia, a function is an isolated mathematical operation, but in real software engineering, functions are mostly characterized by the relationship to their callers. The algorithm isn't the point -- add one new caller and the algorithm may get rewritten!)

So... what about this:

import * as filePath from './filePath.ts';

filePath.combine(x, y);

It is mostly a solution to the original requirements. But only if we include an ESLint rule to enforce that the identifier filePath matches the filename filePath.ts. Otherwise someone can do this:

import * as fp from './filePath.ts';
fp.combine(x, y);

import { combine } from './filePath.ts';

Doing so would thwart our ability to search for filePath.combine(). It would undermine the idea that filePath.combine() is its name.

A request

It's great for QuickFix to provide better support for people who prefer import * as filePath. But hopefully it can ALSO provide some solution for developers facing different requirements. 😉

RyanCavanaugh commented 2 years ago

@octogonz all of that granted, the solution to the problem you're describing is a lint rule, not a nudge from auto-import/quickfix

hesxenon commented 1 year ago

@RyanCavanaugh respectfully, I disagree: You're able to write combine and have that auto imported, but you're not able to write FilePath and have that autoimported because there's this notion that the module name should not be up to the exported.

For one the reasoning behind this notion has never been clear imho and in practice this is not an issue. E.g. a package uploaded to NPM must have a unique name already, library authors already have to think about a unique "root identifier" for their code.

Up until here you're right - this can be done with a lint rule. But it still means that devs have to manually write star imports while not having to manually write qualified imports. This favors qualified imports which - in contrast to namespaced imports - is much more likely to lead to naming conflicts.

This issue is currently handled by defining relations between code bits in file structure which is largely unchecked and brittle and including the relations in the name of each exported code bit, leading to names that have long been turned to memes regarding e.g. Javas ProblemSolutionFactoryManagerStrategyManagerFacade.

So the issue at hand can be supported by lint rules but to actually turn the tides a deeper tooling integration would be needed.

I really like your two "in favor" points above, can we please have that?

Also, I'm wondering... don't you have people on the team that are deeply involved in F#? What do they have to say about this? Nesting, extending and opening modules has never been easier and safer for me than in F#, I'd really like to see some similar solutions in Typescript.