Improve flow typing

[MichaReiser]

Q	A
Fixed Issues?	RFC: babel/rfcs#7
Patch: Bug Fix?	No
Major: Breaking Change?	Yes
Minor: New Feature?	No
Tests Added + Pass?	No
Documentation PR Link
Any Dependency Changes?	No
License	MIT

The goal of this PR is to improve the @babel/types typing for Flow. This is a RFC because some of the changes are not backwards compatible.

Current Flow Types

The current script generates classes for each node type and specifies type refinements (%checks (node instanceof MemberExpression)) for the is* methods.

That means, the following is possible.

const {isIdentifier = require('@babel/types');

if (isIdentifier(node)) {
  console.log(node.name);
}

Shortcomings

The todays system has mainly two shortcomings.

• Exporting the node types as classes is incorrect. @babel/types does not implement the node types as classes nor does it export any value for each type. Declaring the node types as classes might misguide users to check for a certain node type using node instanceof Identifier or to create a new node using a constructor new Identifier().
• Flow only supports %checks refinements on function declarations and only if the function is called as a method. For example, isIdentifier(node) is supported but t.isIdentifier(node) (method invocation) or path.isIdentifier() are not.

Proposal

The proposal of this PR is to change the today's typing to

• Use types instead of classes to make it clear that @babel/types does not export a value for the node types.
• Use a union type for BabelNode rather than a base class. This allows to refine on node types using node.type === 'Identifier'.
• Refine on the type for is* methods

Example Usage:

import * as t from '@babel/types';
import {isStringLiteral} from '@babel/types';
import type {StringLiteral, NumericLiteral} from '@babel/types';

function asStringLiteral(ast: ?StringLiteral | NumericLiteral): BabelNodeStringLiteral | null {
  if (ast == null) {
      return t.stringLiteral("null");
  }

  if (isStringLiteral(ast)) { // or ast.type === 'StringLiteral'
    return ast;
  }

  return t.stringLiteral(String(ast.value));
}

This will also allow flow to e.g. refine Path by using path.node.type === 'Identifier' && path.node.name === 'test';

Generated types

Regression

Changing the node types to classes means that custom %checks declaration must be rewritten from e.g.

declare function isMemberX(node: BabelNode) %checks (node instanceof BabelNodeMemberExpression)

to

declare function isMemberX(node: BabelNode) %checks (node.type === 'MemberExpression');

Other Changes

• Create module scoped type declarations for each node type without the NODE_PREFIX. The goal of this is to have shorter type names and allow consumers to explicitly import the types vs using the global declarations.
• Change the special case for the super and import functions to be declared as var instead of functions to work around a parsing error in some linters. This doesn't change the semantics.
• Added %checks declaration for alias types, e.g. isLiteral

[codesandbox-ci]

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 33d7851:

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

[JLHwung]

@MichaReiser Thanks for the writings. We have an RFC repo for this specific purpose, can you open an RFC PR on https://github.com/babel/rfcs?

[babel-bot]

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

[MichaReiser]

RFC Discussion: babel/rfcs#7

[ExE-Boss]

See also: #11883

[MichaReiser]

See also: #11883

Rebased on top of #11883

[nicolo-ribaudo]

@MichaReiser Could you try running make flow without this PR and with this this PR, to see if there is any outstanding perf difference?

[MichaReiser]

@nicolo-ribaudo There's a performance difference when running a full-check on the repository. I haven't figured out how to measure the incremental performance which, in my view, is the more important performance.

Flow 0.123

~/g/babel [babel-types-flow-types] » time make flow                    
yarn flow check --strip-root
Found 0 errors

________________________________________________________
Executed in   19.09 secs   fish           external 
   usr time    7.96 secs   99.00 micros    7.96 secs 
   sys time    4.15 secs  637.00 micros    4.15 secs 
   
   
   
~/g/babel [master] » time make flow                                                                                                                                      
yarn flow check --strip-root
Found 0 errors

________________________________________________________
Executed in   14.32 secs   fish           external 
   usr time    4.02 secs   80.00 micros    4.02 secs 
   sys time    3.77 secs  449.00 micros    3.77 secs 

flow 0.137

~/g/babel [babel-types-flow-types] » time flow check --strip-root  
Executed in   13.39 secs   fish           external 
   usr time    4.34 secs  140.00 micros    4.34 secs 
   sys time    2.42 secs  849.00 micros    2.42 secs 

~/g/babel [master] » time flow check --strip-root  
________________________________________________________
Executed in   13.07 secs   fish           external 
   usr time    2.61 secs  102.00 micros    2.61 secs 
   sys time    2.63 secs  667.00 micros    2.63 secs 

I overall expected that the check performance would decrease and the results are around what I expected. Other types I tried had check times where a single file took more than 8s.