Support TypeScript 4.5 type-only import/export specifiers by sosukesuzuki · Pull Request #13802 · babel/babel

sosukesuzuki · 2021-09-30T17:04:30Z

Q	A
Fixed Issues?	Fixes #13799
Minor: New Feature?	Y
Tests Added + Pass?	Yes
License	MIT

Support type-only import/export specifiers will be landed in TS 4.5.

import { type Foo } from "foo";
export { type Foo } from "foo";

Todo:

babel-parser
babel-generator
babel-types
babel-plugin-transform-typescript

AST change:

/cc @bradzacher What do you think?

Add importKind: "value" | "type" to ImportSpecifier.
Add exportKind: "value" | "type" to ExportSpecifier

References:

Type-only import specifiers microsoft/TypeScript#45998

codesandbox-ci · 2021-09-30T17:07:06Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1f933af:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

babel-bot · 2021-09-30T17:09:52Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/49458/

bradzacher · 2021-09-30T19:17:45Z

I'd personally have preferred if the AST was just kind to match the top-level names - but considering we're matching flow here - I think it's okay for us to align with them

JLHwung · 2021-10-02T03:17:29Z

packages/babel-plugin-transform-typescript/src/index.ts

@@ -229,9 +229,23 @@ export default declare((api, opts) => {
                continue;


Can you rebase on latest main? We should also register global type for type-only import specifiers so they can be removed in the following case:

import { type A } from "module"; export { A }

nicolo-ribaudo

Can you add a test for import { type A } from "x" with onlyRemoveTypeImports? import "x" should not be removed.

nicolo-ribaudo · 2021-10-10T10:42:44Z

packages/babel-generator/src/generators/modules.ts


 export function ExportSpecifier(this: Printer, node: t.ExportSpecifier) {
+  if (node.exportKind === "type") {
+    this.word(node.exportKind);


Suggested change

this.word(node.exportKind);

this.word("type");

d337f69 👍

nicolo-ribaudo · 2021-10-10T10:52:40Z

packages/babel-plugin-transform-typescript/src/index.ts

-                  } else {
-                    allElided = false;
-                    NEEDS_EXPLICIT_ESM.set(path.node, false);
+                  if (!importsToRemove.includes(binding.path)) {


We can use a Set for importsToRemove, since .has() is O(1) while .includes is O(n).

8cb6dc0 👍

sosukesuzuki · 2021-10-11T10:40:13Z

@nicolo-ribaudo Thank you for your review! I've addressed your comments. 4d1cde6

JLHwung · 2021-10-12T13:03:13Z

packages/babel-plugin-transform-typescript/src/index.ts

              }

-              if (isAllSpecifiersElided()) {
+              if (!onlyRemoveTypeImports && isAllSpecifiersElided()) {


Why should we keep import statements when onlyRemoveTypeImports is true?

TS removes them anyway.

@nicolo-ribaudo You said #13802 (review). What do you think?

Whoops, I assumed that import { type X } from "x" would have been equivalent to import {} from "x" and thus not removed. I'm sorry!

Hmm, interesting. TS removes import {} from "x", too. It preserves only import "x".

According to behavior of TypeScript, it is better to remove the Import Statement in such cases. 6b79fd1

packages/babel-parser/src/plugins/typescript/index.js

packages/babel-parser/src/parser/statement.js

packages/babel-parser/src/plugins/typescript/index.js

nicolo-ribaudo · 2021-10-24T15:09:13Z

packages/babel-parser/src/parser/statement.js

-      const local = this.parseModuleExportName();
-      node.local = local;
-      if (this.eatContextual(tt._as)) {
+      node.local = this.parseModuleExportName();


I'd like to avoid mixing TS/flow-specific code in the main parser, when possible.

Would it be possible to move type handling in parseModuleExportName()? Or maybe we reorganize it like this:

parseExportSpecifiers() { while () { // loop to parse all the specifiers const local = this.parseModuleExportName(); nodes.push(this.parseExportSpecifier(local)); } } parseExportSpecifier(local) { // ... }

and in typescript/index.js

parseExportSpecifier(local) { if (local.type === "Identifier" && local.name === "type") { const node = this.startNodeAtNode(local); // if this is a type export, parse it as such return node; } return super.parseExprtSpecifier(local); }

This is similar to what we do for imports.

It's a little different from the interface you suggested, but I modified it based on that. Please review again.

commits:

(Sorry for many commits..)

d459c8b

519f977

cb0f943

91918b7

d074010

JLHwung · 2021-10-25T18:57:43Z

packages/babel-parser/src/plugins/typescript/index.js

+      node[rightOfAsKey] = rightOfAs;
+
+      const kindKey = isImport ? "importKind" : "exportKind";
+      node[kindKey] = hasTypeSpecifier ? "type" : "value";


Can you also update packages/babel-parser/src/types.js?

c44171a 👍

JLHwung · 2021-10-25T19:27:40Z

packages/babel-parser/src/plugins/typescript/index.js

+              // { type as as something }
+              hasTypeSpecifier = true;
+              leftOfAs = firstAs;
+              rightOfAs = this.parseIdentifier();


When we are parsing export, the right-of-as is a liberal identifier (should be parsed by parseIdentifier(true)), otherwise it should throw unexpected reserved word:

export { type a as if } from "x"; // valid import { type a as if } from "x"; // invalid

We can use parseIdentifier(true/false) for those specifiers. But Babel doesn't check reserved word for TypeScript. Should we still fix for that?

babel/packages/babel-parser/src/plugins/typescript/index.js

Lines 2237 to 2247 in 248aa9d

checkReservedWord(

word: string, // eslint-disable-line no-unused-vars

startLoc: number, // eslint-disable-line no-unused-vars

checkKeywords: boolean, // eslint-disable-line no-unused-vars

// eslint-disable-next-line no-unused-vars

isBinding: boolean,

): void {

// Don't bother checking for TypeScript code.

// Strict mode words may be allowed as in `declare namespace N { const static: number; }`.

// And we have a type checker anyway, so don't bother having the parser do it.

}

Hmm, let's fix that in another PR, then.

JLHwung

Awesome work!

nicolo-ribaudo

Thanks!

sosukesuzuki added area: typescript pkg: generator pkg: types PR: New Feature 🚀 A type of pull request used for our changelog categories labels Sep 30, 2021

sosukesuzuki force-pushed the type-only-import-specifier branch from 3cc6c3d to 15392c3 Compare September 30, 2021 22:40

sosukesuzuki marked this pull request as ready for review September 30, 2021 22:46

fedeci added this to the v7.16.0 milestone Sep 30, 2021

fedeci self-requested a review September 30, 2021 22:50

JLHwung reviewed Oct 2, 2021

View reviewed changes

sosukesuzuki added 8 commits October 2, 2021 12:33

Implement parser

9b91e84

Add parser tests

1b20596

Implement generator

8daf3bc

Add generator tests

16b4bc1

Implement types

a2edbe3

Implement plugin-transform-typescript

6421008

Add plugin-transform-typescript tests

6a596b9

Remove imported-type from exports

4929209

sosukesuzuki force-pushed the type-only-import-specifier branch from 15392c3 to 4929209 Compare October 2, 2021 07:26

bradzacher mentioned this pull request Oct 3, 2021

TypeScript 4.5 Support typescript-eslint/typescript-eslint#3950

Closed

8 tasks

sosukesuzuki mentioned this pull request Oct 4, 2021

Support TypeScript 4.5 prettier/prettier#11600

Closed

nicolo-ribaudo reviewed Oct 10, 2021

View reviewed changes

sosukesuzuki added 3 commits October 11, 2021 19:22

Use string literal instead of variable

d337f69

Use set instead of array

8cb6dc0

Keep import statement if all of specifiers are type-only one

4d1cde6

sosukesuzuki added the pkg: parser label Oct 11, 2021

JLHwung reviewed Oct 12, 2021

View reviewed changes

packages/babel-parser/src/plugins/typescript/index.js Outdated Show resolved Hide resolved

JLHwung reviewed Oct 12, 2021

View reviewed changes

packages/babel-parser/src/parser/statement.js Outdated Show resolved Hide resolved

JLHwung reviewed Oct 12, 2021

View reviewed changes

packages/babel-parser/src/plugins/typescript/index.js Outdated Show resolved Hide resolved

sosukesuzuki added 6 commits October 13, 2021 16:09

Rename isTypeOnly to hasTypeSpecifier

05d5183

Use isContextual instead of string comparison

31b273c

Use tokenIsKeywordOrIdentifier instead of match

ec6398e

Remove import statement if all of specifiers are type-only one

6b79fd1

Merge branch 'main' into type-only-import-specifier

72b3c3b

Fix type errors

ebf0a58

nicolo-ribaudo reviewed Oct 24, 2021

View reviewed changes

nicolo-ribaudo mentioned this pull request Oct 24, 2021

Regenerate parser fixtures #13882

Merged

Merge branch 'main' into type-only-import-specifier

d5c69eb

JLHwung reviewed Oct 25, 2021

View reviewed changes

JLHwung approved these changes Oct 25, 2021

View reviewed changes

sosukesuzuki added 9 commits October 26, 2021 21:20

Extract parseExportSpecifier

d459c8b

Extract parameters

519f977

Move the logic for error

cb0f943

Refactor exports

91918b7

Refactor imports

d074010

Fix lint comments

ab1f5c2

Fix babel-parser/types.js

c44171a

Throw error for string local for import

bfd0f9d

check lval

1f933af

nicolo-ribaudo approved these changes Oct 28, 2021

View reviewed changes

nicolo-ribaudo merged commit d5ba355 into babel:main Oct 28, 2021

sosukesuzuki deleted the type-only-import-specifier branch October 29, 2021 01:58

snyk-bot mentioned this pull request Dec 14, 2021

[Snyk] Upgrade @babel/core from 7.9.6 to 7.16.0 aml-org/playground#69

Closed

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 28, 2022

github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2022

		@@ -229,9 +229,23 @@ export default declare((api, opts) => {
		continue;

	checkReservedWord(
	word: string, // eslint-disable-line no-unused-vars
	startLoc: number, // eslint-disable-line no-unused-vars
	checkKeywords: boolean, // eslint-disable-line no-unused-vars
	// eslint-disable-next-line no-unused-vars
	isBinding: boolean,
	): void {
	// Don't bother checking for TypeScript code.
	// Strict mode words may be allowed as in `declare namespace N { const static: number; }`.
	// And we have a type checker anyway, so don't bother having the parser do it.
	}

Uh oh!

Conversation

sosukesuzuki commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bradzacher commented Sep 30, 2021

Uh oh!

JLHwung Oct 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sosukesuzuki commented Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicolo-ribaudo Oct 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLHwung left a comment

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sosukesuzuki commented Sep 30, 2021 •

edited

Loading

codesandbox-ci bot commented Sep 30, 2021 •

edited

Loading

babel-bot commented Sep 30, 2021 •

edited

Loading

JLHwung Oct 2, 2021 •

edited

Loading

sosukesuzuki commented Oct 11, 2021 •

edited

Loading

nicolo-ribaudo Oct 24, 2021 •

edited

Loading