Skip to content

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

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

Open
5 tasks done
crazytoucan opened this issue Jul 13, 2022 · 4 comments
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Milestone

Comments

@crazytoucan
Copy link

crazytoucan commented Jul 13, 2022

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:

  • Typing the symbol name User and then invoking intellisense would include an option to star-import the module import * as apiTypes from "…";, and then to insert the fully-qualified name apiTypes.User
    • user: User| -> Ctrl+Space -> [ ] apiTypes.User path/to/apiTypes -> Enter
  • Typing the text apiTypes and then invoking intellisense would include an option to star-import the module import * as apiTypes from "…";, leaving your cursor as-is so you could then type a period . and see module members
    • function foo(user: apiTypes| -> Ctrl+Space -> [ ] apiTypes path/to/apiTypes -> Enter

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:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh
Copy link
Member

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

  • On an unresolved identifier, you can "Quickfix" to add an import to the missing thing
  • In the completion list, identifiers from other files appear, which you can "Auto-import" as part of completing them

Things I like about this:

  • We should offer the ability to namespace-import during QuickFix (let's say this issue is that)
  • You should be able to configure whether auto-import completes using destructure import vs namepsace import (TS auto import should support configuring whether a star or a qualified import is used. #19630)
  • Files should be able to specify their preferred namespace import name (though the default should be sensible too -- the default namespace import name for apiTypes.ts should be apiTypes, no action should be necessary) (please open a new issue)

Things I don't like:

  • 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. This will also lead to style fights across the ecosystem as definition file authors do/don't include this and consumers start having lint rules get violated

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Experience Enhancement Noncontroversial enhancements labels Jul 14, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jul 14, 2022
@octogonz
Copy link

  • 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:

  • a website for searching the code base probably can only do limited type analysis
  • searching for GitHub issues that mention this API
  • if I encountered a problem with combine(), maybe I want to search the support forums for mention of it

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
Copy link
Member

@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
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants