Add TS support to @babel/parser's Scope #9766

nicolo-ribaudo · 2019-03-25T20:59:58Z

Q	A
Fixed Issues?	Fixes #9763, fixes #9480
Patch: Bug Fix?	Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

babel-bot · 2019-03-25T21:04:35Z

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

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

packages/babel-parser/src/util/scope.js

nicolo-ribaudo · 2019-03-26T00:00:30Z

packages/babel-parser/src/util/scope.js

-      const scope = this.currentScope();
-      scope.lexical.push(name);
-    } else if (bindingType === BIND_FUNCTION) {
+    if (


The changes in this file are mostly refactoring things in different functions, to make it possible to overwrite them.

Jessidhia · 2019-03-26T02:14:01Z

packages/babel-parser/src/util/scopeflags.js

@@ -41,12 +41,14 @@ export const BIND_NONE = 0, // Not a binding
  BIND_LEXICAL = 2, // Let- or const-style binding
  BIND_FUNCTION = 3, // Function declaration
  BIND_SIMPLE_CATCH = 4, // Simple (identifier pattern) catch binding
-  BIND_OUTSIDE = 5; // Special case for function names as bound inside the function
+  BIND_OUTSIDE = 5, // Special case for function names as bound inside the function
+  BIND_TS_ENUM = 6; // TypeScript enums (block-scoped, but can be re-declared using var/function)


A complete fix for #9763 would also need to keep track of other typescript declarations.

Also, this is not quite how TS Enums work. enum can be declared multiple times, but only with enum or namespace. It might be useful to keep track of which types are in "type space" or "value space".

For example, type A = {}; function A() {} are completely different and unrelated -- they are two separate bindings (and both are hoisted!); they just share the same name.

Here's an attempt to explain how the declarations (are supposed to) work. The "type" part of declarations is always hoisted.

\ value type

namespace maybe yes, can hold other declarations

enum yes, let-like yes, namespace-like

class yes, let-like yes (refers to the instance)

interface no yes

type no yes

var yes no

let yes no

const yes no

function yes no

Generic arguments aren't here but they are type-like.

namespace and enum (besides an ES module itself) are the only kinds of type that can hold other types or declarations inside them. class "looks like" it would behave like that but they are interface-like instead of namespace-like -- but class can merge with a namespace, which affects the value portion class (i.e. they're part of the constructor, not the instance). class can also merge with interface, which affects the type portion of class (the instance type).

✅ = merges
🔴 = invalid
⚪️ = no merging and no conflict; they are separate entities
⚠️ = namespaces are weird (as long as they do not contain values they don't conflict; if they do they can't merge)
❗️ = no conflict if and only if the type annotations are identical

Merging namespace enum class interface type var let const function

namespace ✅ ✅ ✅ ✅ 🔴 ⚠ ⚠ ⚠️ ✅

enum ✅ ✅ 🔴 🔴 🔴 🔴 🔴 🔴 🔴

class ✅ 🔴 🔴 ✅ 🔴 🔴 🔴 🔴 🔴

interface ✅ 🔴 ✅ ✅ 🔴 ⚪️ ⚪️ ⚪️ ⚪️

type 🔴 🔴 🔴 🔴 🔴 ⚪️ ⚪️ ⚪️ ⚪️

var ⚠ 🔴 🔴 ⚪️ ⚪️ ❗️ 🔴 🔴 🔴

let ⚠ 🔴 🔴 ⚪️ ⚪️ 🔴 🔴 🔴 🔴

const ⚠ 🔴 🔴 ⚪️ ⚪️ 🔴 🔴 🔴 🔴

function ✅ 🔴 🔴 ⚪️ ⚪️ 🔴 🔴 🔴 *

*: multiple declarations of a function / method on the same scope are only valid if the declarations are overloads; only one of the declarations is allowed to have an implementation (the curly braces block), and it must be the last one.

type can maybe be understood as the type-only const equivalent, as it is block-scoped and it can't merge with anything or be reassigned.

Thanks you for the detailed table!
I think that we for the (var...functon)x(var...functon) cells, we should follow JavaScript's behavior, since we can't rely on type info in Babel.

As for ⚠️, I think that I won't implement redeclaration checks for them in this PR since they are only needed when transpiling namespaces to IIFEs, but Babel doesn't support doing so. It could be added later but it will be more difficult.

The "type" part of declarations is always hoisted.

Always to the block, not to the function, right?

airato · 2019-04-02T22:45:38Z

today it seems we have 3 issues with named exports in typescript:

parser fails with undefined exports when we try to export TS types within named exports. this happens because TS type declarations are not counted as a declaration when checking exports (New @babel/parser early errors break by-name typescript exports #9763)
the scope for bodyless TS function declarations is getting opened, but not getting closed. This causes issues with function overloads
the typescript transform does not erase named type exports.

this PR should fix 1) and maybe 2). Issue 3 is hidden today since parser fails before transform.

I have a branch attempting to fix these, but I'm doing so with existing scope module, and it's pretty dirty.
Since this PR is doing a refactor of the scope module for TS, I'll wait for this to land and then will create a PR to fix issue 3.

Here is one of the tests from my branch
https://github.com/airato/babel/blob/2b6fe025ace1038b5d2e77420f50692a0b1bd22d/packages/babel-plugin-transform-typescript/test/fixtures/declarations/erased/input.mjs

The parser should be able to parse this with no errors

danez · 2019-04-03T03:39:47Z

This example now fails although it shouldn't, and may be because the SIMPLE_CATCH was removed?:

try {} catch (a) { 
  if(1) function a(){} 
}

https://tc39.github.io/ecma262/#sec-variablestatements-in-catch-blocks

Although I just noticed we already have a testcase for this which was untouched. So not sure why it is failing in the repl.

nicolo-ribaudo · 2019-04-04T19:33:23Z

@danez It is already failing on master: https://babeljs.io/repl/build/master#?babili=false&browsers=IE%207&build=&builtIns=false&spec=false&loose=false&code_lz=C4JwngBA3gvhDGBDY8AWEAUiCU0ICgIIBLAMwwEZdSBXAO3mGIHs6JENtYCYg&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=script&lineWrap=false&presets=es2015%2Creact%2Cstage-2&prettier=true&targets=&version=7.4.3&externalPlugins=

I'd prefer to fix it in a separate PR since it seems unrelated.

Although I just noticed we already have a testcase for this which was untouched. So not sure why it is failing in the repl.

Maybe it's @babel/traverse?

danez · 2019-04-04T19:35:59Z

Sure, but I just wonder why the test works. https://github.com/babel/babel/blob/master/packages/babel-parser/test/fixtures/core/scope/dupl-bind-catch-hang-func/input.js

But in the repl it doesn't.

nicolo-ribaudo · 2019-04-04T19:37:26Z

Because that tests doesn't run @babel/traverse, only @babel/parser.

danez · 2019-04-04T19:40:25Z

🤦‍♂️I thought the error comes from the parser.

nicolo-ribaudo · 2019-04-04T19:55:12Z

@danez I will add a few more commits to address the link in #9766 (comment)

nicolo-ribaudo · 2019-04-04T20:54:09Z

@airato Your test is now correctly parsed.

henderea · 2019-04-17T17:34:46Z

Hey, is this PR going anywhere? It doesn't seem to have much activity in the past couple of weeks. What all is it waiting on?

Jessidhia

Looks good 👍

Sorry for taking so long to review this 🙇‍♀️

It could be good to have const enum tests as well, just in case. TypeScript requires the constness to be the same (but the error message it emits when they differ is nonsense).

Jessidhia · 2019-04-22T10:20:17Z

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

+  ): boolean {
+    if (scope.enums.indexOf(name) > -1) {
+      // Enums can be merged with other enums
+      return !(bindingType & BIND_FLAGS_TS_ENUM);


Enums can merge with namespaces too; I guess the error is avoided by not calling super.declareName for namespaces?

Yeah exactly.

SCOPE_SIMPLE_CATCH was used instead

1) Move this.scope.exit() for functions from parseFunctionBody to the callers. Otherwise the scope of body-less functions was never closed. Also, it is easier to track scope.exit() if it is near to scope.enter() 2) Register namespace ids for export

babel/babel#9766 was really needed.

nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: parser area: typescript i: regression labels Mar 25, 2019

danez reviewed Mar 25, 2019

View reviewed changes

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

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

nicolo-ribaudo marked this pull request as ready for review March 25, 2019 22:25

nicolo-ribaudo force-pushed the parser-typescript-scope-enum branch 3 times, most recently from 0f7a245 to ed7b6d6 Compare March 25, 2019 23:55

nicolo-ribaudo commented Mar 26, 2019

View reviewed changes

Jessidhia reviewed Mar 26, 2019

View reviewed changes

nicolo-ribaudo requested a review from Jessidhia March 26, 2019 21:10

nicolo-ribaudo changed the title ~~Add TS enums support to @babel/parser's Scope~~ Add TS support to @babel/parser's Scope Mar 26, 2019

rodogir mentioned this pull request Mar 28, 2019

Webpack upgrade breaks TypeScript interface exports vercel/next.js#6668

Closed

danez approved these changes Apr 4, 2019

View reviewed changes

nicolo-ribaudo mentioned this pull request Apr 10, 2019

[plugin-transform-typescript] Default or named exported interface result in runtime error #7640

Closed

nicolo-ribaudo added this to In progress in Parser: Spec Compliance, Refactoring and Performance via automation Apr 16, 2019

Jessidhia approved these changes Apr 22, 2019

View reviewed changes

Parser: Spec Compliance, Refactoring and Performance automation moved this from In progress to Reviewed Apr 22, 2019

nicolo-ribaudo added 15 commits April 26, 2019 13:52

[parser] Allow plugins to extend ScopeHandler

3cd1814

Directly extend Scope

1fd612c

Don't use new.target to get the ScopeHandler

93638f6

[parser] Add TS enum support to the Scope

8c4e379

Remove duplicated options in tests

627bfa3

Fix

feca04a

Fix flow

02bbd18

Rename tests

0f16595

Add tests

6b87d61

Full typescript support in scope

cb5f104

Remove BIND_SIMPLE_CATCH

69dede6

SCOPE_SIMPLE_CATCH was used instead

Export TS types

035904e

Register function declarations

59fcd6a

Disallow redeclaration of enum with const enum

be8e163

nicolo-ribaudo force-pushed the parser-typescript-scope-enum branch from 89fc433 to be8e163 Compare April 26, 2019 12:04

nicolo-ribaudo merged commit 30d507c into babel:master Apr 26, 2019

Parser: Spec Compliance, Refactoring and Performance automation moved this from Reviewed to Done Apr 26, 2019

nicolo-ribaudo deleted the parser-typescript-scope-enum branch April 26, 2019 12:19

nicolo-ribaudo mentioned this pull request Apr 26, 2019

Export 'IValues' is not defined #9895

Closed

airato mentioned this pull request May 5, 2019

fix(typescript): erase type exports #9944

Merged

Klaster1 pushed a commit to gridgain/gridgain that referenced this pull request May 8, 2019

WC-1041 Update Babel dependencies again.

a80c2a6

babel/babel#9766 was really needed.

This was referenced Aug 17, 2019

[Snyk] Upgrade @babel/polyfill from 7.0.0 to 7.4.4 O-Mutt/fields-of-arle#9

Open

[Snyk] Upgrade @babel/runtime from 7.0.0 to 7.5.5 Watemlifts/storybook#7

Closed

This was referenced Sep 17, 2019

[Snyk] Upgrade @babel/polyfill from 7.4.4 to 7.6.0 cds-snc/report-a-cybercrime#744

Closed

[Snyk] Upgrade @babel/runtime from 7.4.4 to 7.6.0 Matthelonianxl/drizzle#2

Open

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 3, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TS support to @babel/parser's Scope #9766

Add TS support to @babel/parser's Scope #9766

nicolo-ribaudo commented Mar 25, 2019 •

edited

babel-bot commented Mar 25, 2019 •

edited

nicolo-ribaudo Mar 26, 2019

Jessidhia Mar 26, 2019 •

edited

nicolo-ribaudo Mar 26, 2019

nicolo-ribaudo Mar 26, 2019

airato commented Apr 2, 2019

danez commented Apr 3, 2019 •

edited

nicolo-ribaudo commented Apr 4, 2019 •

edited

danez commented Apr 4, 2019

nicolo-ribaudo commented Apr 4, 2019 •

edited

danez commented Apr 4, 2019

nicolo-ribaudo commented Apr 4, 2019

nicolo-ribaudo commented Apr 4, 2019

henderea commented Apr 17, 2019

Jessidhia left a comment

Jessidhia Apr 22, 2019

nicolo-ribaudo Apr 26, 2019

\	value	type
`namespace`	maybe	yes, can hold other declarations
`enum`	yes, `let`-like	yes, `namespace`-like
`class`	yes, `let`-like	yes (refers to the instance)
`interface`	no	yes
`type`	no	yes
`var`	yes	no
`let`	yes	no
`const`	yes	no
`function`	yes	no

Merging	`namespace`	`enum`	`class`	`interface`	`type`	`var`	`let`	`const`	`function`
`namespace`	✅	✅	✅	✅	🔴	⚠	⚠	⚠️	✅
`enum`	✅	✅	🔴	🔴	🔴	🔴	🔴	🔴	🔴
`class`	✅	🔴	🔴	✅	🔴	🔴	🔴	🔴	🔴
`interface`	✅	🔴	✅	✅	🔴	⚪️	⚪️	⚪️	⚪️
`type`	🔴	🔴	🔴	🔴	🔴	⚪️	⚪️	⚪️	⚪️
`var`	⚠	🔴	🔴	⚪️	⚪️	❗️	🔴	🔴	🔴
`let`	⚠	🔴	🔴	⚪️	⚪️	🔴	🔴	🔴	🔴
`const`	⚠	🔴	🔴	⚪️	⚪️	🔴	🔴	🔴	🔴
`function`	✅	🔴	🔴	⚪️	⚪️	🔴	🔴	🔴	`*`

Add TS support to @babel/parser's Scope #9766

Add TS support to @babel/parser's Scope #9766

Conversation

nicolo-ribaudo commented Mar 25, 2019 • edited

babel-bot commented Mar 25, 2019 • edited

nicolo-ribaudo Mar 26, 2019

Choose a reason for hiding this comment

Jessidhia Mar 26, 2019 • edited

Choose a reason for hiding this comment

nicolo-ribaudo Mar 26, 2019

Choose a reason for hiding this comment

nicolo-ribaudo Mar 26, 2019

Choose a reason for hiding this comment

airato commented Apr 2, 2019

danez commented Apr 3, 2019 • edited

nicolo-ribaudo commented Apr 4, 2019 • edited

danez commented Apr 4, 2019

nicolo-ribaudo commented Apr 4, 2019 • edited

danez commented Apr 4, 2019

nicolo-ribaudo commented Apr 4, 2019

nicolo-ribaudo commented Apr 4, 2019

henderea commented Apr 17, 2019

Jessidhia left a comment

Choose a reason for hiding this comment

Jessidhia Apr 22, 2019

Choose a reason for hiding this comment

nicolo-ribaudo Apr 26, 2019

Choose a reason for hiding this comment

nicolo-ribaudo commented Mar 25, 2019 •

edited

babel-bot commented Mar 25, 2019 •

edited

Jessidhia Mar 26, 2019 •

edited

danez commented Apr 3, 2019 •

edited

nicolo-ribaudo commented Apr 4, 2019 •

edited

nicolo-ribaudo commented Apr 4, 2019 •

edited