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

Add syntax hint to allow reexports of types in isolatedModules mode #34750

Closed
5 tasks done
BPScott opened this issue Oct 26, 2019 · 6 comments · Fixed by #35200
Closed
5 tasks done

Add syntax hint to allow reexports of types in isolatedModules mode #34750

BPScott opened this issue Oct 26, 2019 · 6 comments · Fixed by #35200
Assignees
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@BPScott
Copy link

BPScott commented Oct 26, 2019

Search Terms

isolatedModules, type export, type re-export, babel

Context

In --isolatedModules mode you get an error when reexporting types, the reason why this breaks is documented in the new handbook. In the following example lines 2 and 5 both raise give "Cannot re-export a type when the '--isolatedModules' flag is provided." warnings.

import { SomeType } from "someModule";
export { SomeType };
let x: SomeType | undefined;

export { SomeOtherType } from "someModule";

This leaves programme authors who wish to compile using a per-file compiler with some choices:

Use export * from "someModule", which will deal with all exports, but won't work if you need to cherry pick exports. It's usually possibly to rearchitect your folder structure to work around this but not always.

Or use a temporary renaming:

import { SomeType as SomeType1, SomeOtherType as SomeOtherType1 } from "someModule";
export type SomeType = SomeType1;
let x: SomeType | undefined;

export type SomeOtherType = SomeOtherType1;

Prior to #31231 (to be released in TS 3.7) authors could export a type that matches the same name as an import, but that loophole has been closed:

import { SomeType, SomeOtherType } from "someModule";
export type SomeType = SomeType;
let x: SomeType | undefined;

export type SomeOtherType = SomeOtherType;

Authors should not have to go through such renaming dances in order to get type reexports working in isolatedModules mode, instead it should be possible to hint that an import / reexport is of a type.

Suggestion

Add some kind of syntax hint to imports and reexports that denote that a particular identifier is a type and thus can be elided when compiling in isolatedModules mode.

Here I'm proposing the strawman of a type keyword before the ImportSpecifier / ExportSpecifier, as suggested by @Jessidhia in #31231 (comment). This is the same syntax Flow uses though I must admit I'm not familiar with Flow.

import { type SomeType } from "someModule";
export { SomeType };
let x: SomeType | undefined;

export { type SomeOtherType } from "someModule";

Placing the hint on a per-specifier basis will allow mixing of type and value import/reexports in a single import/export statement:

import { SomeValue, type SomeType, SomeOtherValue, type SomeOtherType } from "someModule";

export { AnotherValue, type AnotherType, YetAnotherValue, type YetAnotherType } from "someOtherModule";

Regarding unhappy paths:

  • Adding a type hint to a value should raise a type error "Value export must not be tagged as a type"
  • When isolatedModules is enabled type exports without a type hint should continue to raise a type error, though the message will likely need to be updated to something like "Cannot rexport type without a type hint"
  • When isolatedModules is disabled then there is no value to adding the type hint. We could disable adding the hints by throwing a TypeError "Type exports with type hint are only required in isolatedModules mode" but that feels pretty heavy handed. I think a better path is allowing the type hint from the compiler's perspective, while having a linting rule to that can can say if these type hints should be present or not (defaulting to present for isolatedModules is enabled and absent if disabled). This will help projects who wish to migrate from using tsc to a per-file compiler.

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, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Oct 30, 2019
@Bnaya
Copy link

Bnaya commented Nov 1, 2019

You can still re-export like so:

export type CreateObjectBufferOptions = import("./internal/api").CreateObjectBufferOptions;

in flowtype you have a special keyword for importing types:
https://flow.org/en/docs/types/modules/#toc-importing-and-exporting-types

@andrewbranch
Copy link
Member

andrewbranch commented Nov 4, 2019

Thanks for writing that up @BPScott!

I have a few questions/comments:

  1. Is there actually any reason other than symmetry to support type-only imports? It seems the only pattern that causes trouble is re-exporting a type from another module.
  2. This is the same syntax Flow uses though I must admit I'm not familiar with Flow.

Flow places the type keyword at the import/export declaration level, and it applies to all imports/exports in the declaration. From their docs:

// @flow
import type Foo, {MyObject, MyInterface} from './exports';

In other words, they explicitly don’t do this:

Placing the hint on a per-specifier basis will allow mixing of type and value import/reexports in a single import/export statement

I think I would lean toward mimicking Flow on this, because I think it would be misleading to write

import type ThisOneIsTypeOnly, { ButThisOneHasValue } from './mod';

especially for users who are familiar with Flow. The restriction of splitting up type-only and regular imports/exports into separate declarations seems reasonable (maybe even desirable for grokability) to me.

  1. It’s not 100% clear whether this would be purely a syntactic hint applicable only to type-only symbols, or would be capable of filtering the meanings of symbols that occupy both type and value spaces, like classes:
// a.ts
class C {}
export { C as RegularClassExport };
export type { C as TypeOnlyClassExport }; // Is this allowed?

// b.ts
import type { RegularClassExport } from './a'; // Is this allowed?
import { TypeOnlyClassExport } from './a';

// Ok to use as type
declare let a: TypeOnlyClassExport;
declare let b: RegularClassExport;

// Can’t use as value
const c = new TypeOnlyClassExport();
//            ^^^^^^^^^^^^^^^^^^^ 'TypeOnlyClassExport' only refers to a type,
//                                 but is being used as a value here. ts(2693)
const d = new RegularClassExport();
//            ^^^^^^^^^^^^^^^^^^ 'RegularClassExport' only refers to a type,
//                                but is being used as a value here. ts(2693)

On one hand, I think this is kind of intuitive, but on the other hand, it’s not needed to solve the re-export issue in isolatedModules, since a class can safely be re-exported (since the export will resolve to something at runtime in the JavaScript emit).

  1. There are also a few other open questions around what would be supported for namespace and default imports/exports. I’m not sure if any of these are compelling, since, again, they’re unrelated for the only real problem scenario I’ve identified:

I think the summarized top-priority questions that need to be answered are

  1. Are there any real scenarios that aren’t solved by export type { T } or export type { T } from './mod'?
  2. Is it confusing to implement export type without import type?

@BPScott
Copy link
Author

BPScott commented Nov 5, 2019

Heya @andrewbranch, thanks for extra info on Flow usage.


As for your questions:

Is there actually any reason other than symmetry to support type-only imports? It seems the only pattern that causes trouble is re-exporting a type from another module.

Excellent point. I figured it'd make eliding the imports easier from the compiler's perspective, in the case where a type was both used and reexported but it seems babel is already smart enough to remove imports of types (see this babel repl). In that case I don't think there's any value in keeping the import changes. As you mentioned in point 3 they might be useful for type-only imports but that sounds like a separate feature and out-of scope for solving reexports in isolatedModules

Flow places the type keyword at the import/export declaration level, and it applies to all imports/exports in the declaration. From their docs: [snip]

Digging a little deeper it seems that both of the syntaxes we talk about are supported by Flow. The "on the declaration level" you talked about was there first, and then the "on the specifier" import A, {type B, C} from './x' format got added in v0.38.0 in facebook/flow#2890 as sugar to allow for import/exports that contain both values and types. Annoyingly the "on the specifier" style doesn't seem to be mentioned in their docs.

I agree that mimicing Flow is best though the fact that it does both styles muddies the water somewhat. Them adding the "on the specifier level" as sugar suggests that they've found the "on the declaration" style a little restrictive. Personally I have no strong opinion either way - I'd be happy to do the "on the declaration" style, though it would mean some repetition when you reexport values and types from the same file (this crops up when we reexport react components and their props quite a bit)

export {SomeValue} from './x';
export type {SomeType} from './x';

// vs

export {SomeValue, type SomeType} from './x';

Also of interest regarding which syntax is chosen is this babel issue that talks about how these two syntaxes are handled differently, and that only the "on the declaration" level style gets stripped when compiling to JS: babel/babel#6300. But I don't think this is that relevant if we drop the import syntax stuff.

It’s not 100% clear whether this would be purely a syntactic hint applicable only to type-only symbols, or would be capable of filtering the meanings of symbols that occupy both type and value spaces, like classes: [snip]
On one hand, I think this is kind of intuitive, but on the other hand, it’s not needed to solve the re-export issue in isolatedModules, since a class can safely be re-exported (since the export will resolve to something at runtime in the JavaScript emit).

I was thinking this would be only applicable to type-only symbols. The meaning filtering stuff is nice but as you say it isn't needed right now. I think we could say class C {}; export type { C as TypeOnlyClassExport }; is an error for now, and that would leave the door open to adding this functionality at a later point.

here are also a few other open questions around what would be supported for namespace and default imports/exports.

I don't think any of those are particulary compelling if we punt on the meaning filtering stuff, leave them as type errors?


Are there any real scenarios that aren’t solved by export type { T } or export type { T } from './mod'?

Nothing that springs to mind right now, though I've not thought about this much since the original issue. It's a bit more verbose than the "per specifier" style but both enable the same things, and as both are implemented by Flow the "lets follow flow" approach doesn't really push us towards one style over the other.

Is it confusing to implement export type without import type?

import type doesn't seem to be needed to enable type reexports. I think a bit of asymmetry is acceptable if the part we skip doesn't add anything useful for this feature. The door is still open to fill that gap with meaning filtering if we want to do that in the future. I don't think these two features need to be tied together.

@andrewbranch
Copy link
Member

Did some digging and found that there is another scenario that a lot of people have been asking about for a long time, which is basically the exact opposite of the motivation for this issue: people saying “please don’t remove my imports even if I only use them as a type, because those modules have side effects.” This seems to be most common in Angular, where one file exports a service class and registers it with Angular as a side effect, then other files import that class but only use it as a type, since Angular will inject an instance of that service where needed. The first real proposal (#2812) to address this proposed type-only imports and exports (with the same syntax proposed here) that could always be elided from emit, whereas any other imports would never be elided.

Related:

Make sure this import/export is elided

Make sure this import is not elided

The latter group is a compelling scenario purely by the number of people feeling confused or frustrated over such a long period of time. There are workarounds, but they’re generally regarded as verbose and non-obvious, and sometimes they break lint rules.

The former group has collectively very low engagement. There are fewer 👍s on all those combined than on this issue—Webpack 5 is definitely the strongest argument for type-only anything.

Thoughts so far:

Type-only imports and exports (similar to the original proposal in #2812), probably combined with a compiler flag, could be used to solve all these issues at the same time. Maybe that makes sense, since the two requests are essentially complements. But on the other hand, it seems like the numerous people in the “stop eliding my imports” camp probably don’t care about having always-elided type-only imports. So I’m not yet fully convinced that type-only imports is appropriate as an umbrella fix for both scenarios, but it’s worth exploring. I’ll try to bring this to our next design meeting.

@BPScott
Copy link
Author

BPScott commented Nov 6, 2019

Excellent spelunking!

Looking at babel/babel#6300 (comment) it sounds like babel handles the two flow syntaxes in the same way as #2812 proposes so there's a bit of prior-art there:

  • import type { SomeType } from "./x"; will compile to [NOTHING] - removing the import statement entirely when compiling
  • import { type IFoo } from "mod1"; will compile to import from "mod1"; - removing all type specifiers but leaving the import intact for the sake of side effects.

@Bnaya
Copy link

Bnaya commented Nov 7, 2019

I personally like the import type { SomeType } from "./x";
Possible issues:
In flow you can't import types without it, so it's easy to know what's es-exportable.
In typescript, if you will still be able to import without it, also when isolatedModules is on, you might have types imported with type and some without.
So typescript will need to add additional metadata per imported type symbol about how it was imported, with or without type, so it can know if its safe to export the symbol, and warn the developer about it (+ quick fix?)

Another option is to disallow types imports without type keyword when isolatedModules is on, to behave more as flow

code example:

import type { SomeType } from "./x";
import { IFoo, func } from "./b";

export { 
    // this is ok
    SomeType,
    // this is not
    IFoo 
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants