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

Type-only imports and exports #35200

Merged
merged 50 commits into from Jan 3, 2020

Conversation

@andrewbranch
Copy link
Member

andrewbranch commented Nov 19, 2019

TL;DR:

  • import type { A } from './mod', export type { A } from './mod'
  • Add flag to stop eliding import declarations from emit

To do:

  • parsing
  • checking: default import
  • checking: named imports
  • checking: named exports
  • checking: namespace import
  • update isolatedModules error message
  • test with enums
  • isolatedModules code fix
  • auto imports behavior
  • compiler flag, emit behavior
  • checker API (getTypeAtLocation)
  • grammar error for mixing default and named bindings
  • code fix for splitting a default + named bindings into two import declarations
  • error for usage in JS
  • check/improve parsing diagnostics for common mistakes
  • test more quick info, completions, rename, etc.
  • TextMate grammar
  • auto-import/codefix for name in value space that’s already type-only imported?
  • code fix for --importsNotUsedAsValue=error error

Background

TypeScript elides import declarations from emit where, in the source, an import clause exists but all imports are used only in a type position [playground]. This sometimes creates confusion and frustration for users who write side-effects into their modules, as the side effects won’t be run if other modules import only types from the side-effect-containing module (#9191).

At the same time, users who transpile their code file by file (as in Babel, ts-loader in transpileOnly mode) sometimes have the opposite problem, where a re-export of a type should be elided, but the compiler can’t tell that the re-export is only a type during single-file transpilation (#34750, TypeStrong/ts-loader#751) [playground].

Prior art

In early 2015, Flow introduced type-only imports which would not be emitted to JS. (Their default behavior, in contrast to TypeScript’s, was never to elide imports, so type-only imports for them were intended to help users cut down on bundle size by removing unused imports at runtime.)

Two months later, #2812 proposed a similar syntax and similar emit behavior for TypeScript: the compiler would stop eliding import declarations from emit unless those imports were explicitly marked as type-only. This would give users who needed their imports preserved for side effects exactly what they wanted, and also give single-file transpilation users a syntactic hint to indicate that a re-export was type-only and could be elided: export type { T } from './mod' would re-export the type T, but have no effect on the JavaScript emit.

#2812 was ultimately declined in favor of introducing the --isolatedModules flag, under which re-exporting a type is an error, allowing single-file transpilation users to catch ambiguities at compile time and write them a different way.

Since then

Over the last four years after #2812 was declined, TypeScript users wanting side effects have been consistently confused and/or frustrated. They have workarounds (read #9191 in full for tons of background and discussion), but they’re unappealing to most people.

For single-file transpilation users, though, two recent events have made their lives harder:

  1. In TypeScript 3.7, we sort of took away --isolatedModules users’ best workaround for reexporting a type in #31231. Previously, you could replace export { JustAType } from './a' with

    import { JustAType } from './a';
    export type JustAType = JustAType;

    But as of TypeScript 3.7, we disallow the name collision of the locally declared JustAType with the imported name JustAType.

  2. If a Webpack user was left with an erroneous export { JustAType } from './a' in their output JavaScript, Webpack 4 would warn, but compilation would succeed. Many users simply ignored this warning (or even filtered it out of Webpack’s output). But in Webpack 5 beta, @sokra has expressed some desire to make these warnings errors.

Proposal

  • Add type-only imports and exports similar to #2812 and Flow
  • Change the default emit behavior of the compiler to stop eliding regular imports even if the imported names are only used in type positions
  • Always elide imports and re-exports explicitly marked as type-only
  • Add a (temporary?) compiler flag that restores the current behavior of eliding imports that are used only for types to help users with back-compat
  • Updated: Leave the default emit as-is, adding the flag --importsNotUsedAsValue <"remove" | "preserve" | "error"> to control the behavior
    • remove is default; maintains today’s behavior
    • preserve keeps imports used only for types in the emit as a side-effect import
    • error acts as preserve but also adds an error whenever an import could be written as an import type

Syntax

Supported forms are:

import type T from './mod';
import type { A, B } from './mod';
import type * as Types from './mod';

export type { T };
export type { T } from './mod';

Possible additions but I think not terribly important:

export type * from './mod';
export type * as Types from './mod'; // pending #4813

We notably do not plan to support at this time:

  • type modifier on import/export specifiers: import { type A } from './mod', export { A, type B }
  • Mixing a type-only default import with named or namespace imports: import type T, { A } from './mod', import type T, * as ns from './mod'

The forms in the former bullet will be syntax errors; the forms in the latter will be grammar errors. We want to start with productions that can be read unambiguously, and it’s not immediately clear (especially in the absence of Flow’s implementation), what the semantics of import type A, { B } from './mod' should be. Does type apply only to the default import A, or to the whole import clause? We prefer no one need wonder.

Type semantics

Any symbol with a type side may be imported or exported as type-only. If that symbol has no value side (i.e., is only a type), name resolution for that symbol is unaffected. If the symbol does have a value side, name resolution for that symbol will see only the type side. The typical example is a class:

// @Filename: /a.ts
export default class A {}

// @Filename: /b.ts
import type A from './a';
new A();
//  ^ 'A' only refers to a type, but is being used as a value here.

function f(obj: A) {} // ok

If the symbol is a namespace, resolution will see a mirror of that namespace recursively filtered down to just its types and namespaces:

// @Filename: /ns.ts
namespace ns {
  export type Type = string;
  export class Class {}
  export const Value = "";
  export namespace nested {
    export class NestedClass {}
  }
}
export default ns;

// @Filename: /index.ts
import type ns from './ns';
const x = ns.Value;
//        ^^ Cannot use namespace 'ns' as a value.

let c: ns.nested.NestedClass;

Emit

Updated: When the importsNotUsedAsValue flag is set to 'preserve', type-only import declarations will be elided. Regular imports where all imports are unused or used only for types will not be elided (only the import clause will be elided):

// @importsNotUsedAsValue: preserve

// @Filename: /a.ts
import { T } from './mod';
let x: T;

// @Filename: /a.js
import "./mod";
let x;

Back-compat flag

There’s a new flag removeUnusedImports. Its name is not perfect because it really means “remove imports that have imported names that never get used in a value position.” Open to suggestions.

Updated: this PR is backward-compatible by default.

Auto-imports behavior

  • Symbols without a value side will be imported as type-only if there’s not already a regular import from the containing module. If there’s an existing import from the containing module, it will be added to that import (as happens today).
  • There will be a configuration option to disable type-only auto imports entirely (since some people use VS Code’s TypeScript version for editor services but compile with an older version).

I’m not yet confident what other changes, if any, will the right move, but the main scenarios to consider are:

  • User auto imports a class, enum, or namespace in a type position. Should we do a type-only import?
  • User has a type-only import of a class, enum, or namespace, then later tries to use the same symbol in a value position. Do we convert the type-only import to a regular import? (Is that even possible with a completions code action?)

Successor of #2812
Closes #9191
Closes #34750

Would close if they were still open:

src/compiler/checker.ts Outdated Show resolved Hide resolved
@andrewbranch andrewbranch force-pushed the andrewbranch:feature/type-only branch 2 times, most recently from f26046a to d5e3ebb Nov 20, 2019
@ajafff

This comment has been minimized.

Copy link
Contributor

ajafff commented Nov 21, 2019

@andrewbranch what about imported values that are only used for their types via typeof in the file? Are these imports still elided?

@andrewbranch

This comment has been minimized.

Copy link
Member Author

andrewbranch commented Nov 21, 2019

@ajafff I think ideally the plan would be no, imports not marked with type are never elided. Flow has an import typeof form for this use case. I think that’s probably a reasonable follow-up feature. I had initially thought of import typeof as syntactic sugar for something already possible, but as you bring up, if you care about eliding imports that are unnecessary at runtime but you need the typeof a value, the original proposal doesn’t allow for that. /cc @DanielRosenwasser thoughts?

Of course, a workaround is to export a type alias from the file where the value was exported and import that instead, but you can’t do that if the value in question comes from a third party library.


Edit: a surefire workaround is typeof import('./mod').SomeClass

andrewbranch added 17 commits Nov 12, 2019
…type alias
@andrewbranch

This comment has been minimized.

Copy link
Member Author

andrewbranch commented Dec 31, 2019

Updated the PR description with changes from the decisions made in the last two design meetings (#35806, #35807). Summary:

  1. Problem: concern that a change to default emit behavior will break people’s apps in subtle, hard-to-diagnose ways. Solution: don’t change the default emit. Opt in with a compiler flag.
  2. Problem: if I want to opt into the new emit behavior and use a type-only import everywhere that I can, how do I track down my existing imports that should become import type? Solution: make the compiler flag have a setting where import declarations that only get used as types are errors, forcing you to use import type where possible.

The default emit has been reverted back to the current behavior in master, and the flag --importsNotUsedAsValue <"remove" | "preserve" | "error"> has been added.

@andrewbranch

This comment has been minimized.

Copy link
Member Author

andrewbranch commented Dec 31, 2019

@typescript-bot pack this

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

typescript-bot commented Dec 31, 2019

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at f8333d0. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot

This comment has been minimized.

Copy link
Collaborator

typescript-bot commented Dec 31, 2019

Hey @andrewbranch, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/58985/artifacts?artifactName=tgz&fileId=7313687032FC56B61CC0B0C80C2BB0F7E070038BC4951BA65BD206D680E4FF6702&fileName=/typescript-3.8.0-insiders.20191231.tgz"
    }
}

and then running npm install.


There is also a playground for this build.

@DanielRosenwasser

This comment has been minimized.

Copy link
Member

DanielRosenwasser commented Jan 3, 2020

Added a couple of changes to phrasing. I still am not a fan of the option name, but I think we'd like to at least get this in for the beta.

@DanielRosenwasser

This comment has been minimized.

Copy link
Member

DanielRosenwasser commented Jan 3, 2020

@SeaRyanC if I don't get the chance to in the next 30, feel free to merge this when the tests pass.

@DanielRosenwasser DanielRosenwasser merged commit 3b396e6 into microsoft:master Jan 3, 2020
8 checks passed
8 checks passed
build (8.x)
Details
build (10.x)
Details
build (12.x)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
node10 Build #59261 succeeded
Details
node12 Build #59259 succeeded
Details
node8 Build #59260 succeeded
Details
@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Jan 3, 2020

I just noticed this PR and I wanted to say I think this is a great addition!

I wanted to share a use case that's particularly close to my heart; you may be aware of fork-ts-checker-webpack-plugin. Amongst other things, it powers the TypeScript checking experience in create-react-app.

I worked on amending the plugin so that it could be shipped with create-react-app. Facebook had a hard requirement that the plugin not have a dependency upon TypeScript. We were able to support this by rewriting the plugin somewhat. However, we still needed a bunch of types at compile time. As a consequence there's a whole bunch of imports followed by // Imported for types alone comments:

https://github.com/TypeStrong/fork-ts-checker-webpack-plugin/blob/f07881e80e522c6233568e9f5f7b9574dbae750c/src/ApiIncrementalChecker.ts#L2

I got this wrong a bunch of times a created unwitting dependencies. As I understand this feature, I think "importsNotUsedAsValue": "error" would have made that journey easier and could prevent future regressions. Rejoice!

@nicolo-ribaudo

This comment has been minimized.

Copy link

nicolo-ribaudo commented Jan 3, 2020

❤️❤️ from Babel for this PR!

@stevefan1999-personal

This comment has been minimized.

Copy link

stevefan1999-personal commented Jan 4, 2020

Ah thank you sir

@elektronik2k5

This comment has been minimized.

Copy link

elektronik2k5 commented Jan 5, 2020

This is an awesome and long awaited feature! 🍾

@johnnyreilly and anyone else who needs this feature today without waiting for typescript@3.8: type only imports (albeit with a confusing and awkward syntax) have been part of the language since typescript@2.9, for almost two years now.

type SomeType = import('path-or-package').SomeExportedType
// or inline:
function(foo: import('path-or-package').SomeExportedType) { /* function body */ }
@johnnyreilly

This comment has been minimized.

Copy link

johnnyreilly commented Jan 5, 2020

@elektronik2k5 I saw your thread on twitter! Thanks for sharing; somehow that had passed me by!

@ExE-Boss

This comment has been minimized.

Copy link
Contributor

ExE-Boss commented Jan 17, 2020

Is it possible to import types from @types/* packages directly?

Example:

import type Foo from "@types/foo";
import type { Bar } from "@types/bar";

This would be particularly useful for importing types from non‑npm @types/* packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.