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

babel-types: Add TypeScript definitions #5856

Merged
merged 6 commits into from Jul 25, 2017

Conversation

@andy-ms
Copy link
Contributor

andy-ms commented Jun 13, 2017

Q A
Patch: Bug Fix? no
Major: Breaking Change? Yes in a few places, but only where it was wrong before.
Minor: New Feature? yes
Deprecations? no
Spec Compliancy? yes
Tests Added/Pass? n/a (see below)
Fixed Tickets
License MIT
Doc PR
Dependency Changes

Companion to babel/babylon#523. This adds definitions for all of the TypeScript nodes.
I tested this by recursively validating TypeScript trees parsed from DefinitelyTyped. That script also tests the TS plugin, so I'll wait to include the script in the PR that adds the plugin.

CC @JamesHenry @soda0289 This defines the spec for TypeScript nodes, and duplicates the information found in types.js in babel/babylon#523.

optional: true,
},
};

defineType("CallExpression", {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

This now requires a builder property.

@@ -279,41 +302,44 @@ defineType("FunctionExpression", {
inherits: "FunctionDeclaration",
aliases: ["Scopable", "Function", "BlockParent", "FunctionParent", "Expression", "Pureish"],
fields: {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

Is this necessary? It already inherits.

This comment has been minimized.

Copy link
@andy-ms

andy-ms Jun 14, 2017

Author Contributor

It doesn't look like "inherits" merges things? opts.fields = opts.fields || inherits.fields || {};

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

But all the fields are the same, right?

This comment has been minimized.

Copy link
@andy-ms

andy-ms Jun 15, 2017

Author Contributor

A FunctionExpression can't have declare. id and body are the same but I don't think it would be worth extracting those out separately.

@@ -441,22 +467,18 @@ defineType("MemberExpression", {
});

defineType("NewExpression", {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

This now requires a builder property.

@@ -494,8 +517,8 @@ defineType("ObjectMethod", {
},
key: {
validate: (function () {
const normal = assertNodeType("Expression");
const computed = assertNodeType("Identifier", "StringLiteral", "NumericLiteral");
const normal = assertNodeType("Identifier", "StringLiteral", "NumericLiteral");

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

Shit, did I get these backwards? 😳

@@ -558,14 +575,12 @@ defineType("ObjectProperty", {

defineType("RestElement", {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

This now requires a builder property.

@@ -7,11 +7,13 @@ import defineType, {
assertEach,
assertOneOf,
} from "./index";
import { functionCommon, patternLikeCommon } from "./core";

defineType("AssignmentPattern", {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

This now requires a builder property.

@@ -26,10 +28,11 @@ defineType("AssignmentPattern", {

defineType("ArrayPattern", {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

This now requires a builder property.

params: {
validate: chain(assertValueType("array"), assertEach(assertNodeType("LVal"))),
// Unlike other functions, arrow functions are never generators.
...(() => { const x = functionCommon; delete x.generator; return x; })(),

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jun 14, 2017

Member

This is icky.

This comment has been minimized.

Copy link
@hzoo

hzoo Jun 14, 2017

Member

There is a proposal for arrow function generators at Stage 1 (not implemented) https://github.com/tc39/proposals

@JamesHenry

This comment has been minimized.

Copy link
Member

JamesHenry commented Jun 14, 2017

Thanks for sharing @andy-ms 👍 I'll use this to build out a separate, shared test suite now that we are much closer to convergence

@andy-ms

This comment has been minimized.

Copy link
Contributor Author

andy-ms commented Jun 14, 2017

@codecov

This comment has been minimized.

Copy link

codecov bot commented Jun 14, 2017

Codecov Report

Merging #5856 into 7.0 will increase coverage by 0.08%.
The diff coverage is 98.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##              7.0    #5856      +/-   ##
==========================================
+ Coverage   85.25%   85.34%   +0.08%     
==========================================
  Files         284      286       +2     
  Lines        9963    10030      +67     
  Branches     2781     2781              
==========================================
+ Hits         8494     8560      +66     
  Misses        969      969              
- Partials      500      501       +1
Impacted Files Coverage Δ
packages/babel-types/src/definitions/flow.js 100% <ø> (ø) ⬆️
packages/babel-types/src/definitions/typescript.js 100% <100%> (ø)
packages/babel-types/src/definitions/core.js 98.61% <100%> (+0.08%) ⬆️
...ckages/babel-types/src/definitions/tsFlowCommon.js 100% <100%> (ø)
packages/babel-types/src/definitions/es2015.js 96.96% <88.88%> (+0.3%) ⬆️
...bel-plugin-transform-es2015-parameters/src/rest.js 98.14% <0%> (-1.86%) ⬇️
...bel-plugin-transform-es2015-classes/src/vanilla.js 91.06% <0%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5387d9f...3b4f31a. Read the comment docs.

defineType("TSDeclareMethod", {
visitor: ["decorators", "key", "typeParameters", "params", "returnType"],
fields: classMethodOrDeclareMethodCommon,
});

This comment has been minimized.

Copy link
@Jessidhia

Jessidhia Jun 28, 2017

Member

Are these for overload declarations, or for things that have the declare keyword?

This comment has been minimized.

Copy link
@andy-ms

andy-ms Jun 28, 2017

Author Contributor

Could be either. It's any method with no body.

declare class C1 { // ClassDeclaration with declare: true
    m(): void; // TSDeclareMethod
}
class C2 {
  m(x: number): number; // TSDeclareMethod
  m(x: string): string; // TSDeclareMethod
  m(x: any) { return x; } // ClassMethod
}
@@ -264,30 +271,47 @@ defineType("ForStatement", {
},
});

export const functionCommon = {
params: {

This comment has been minimized.

Copy link
@hzoo

hzoo Jul 12, 2017

Member

Looks like generator got removed?

generator: {
  default: false,
  validate: assertValueType("boolean"),
}
@@ -118,8 +118,10 @@ defineType("BreakStatement", {
aliases: ["Statement", "Terminatorless", "CompletionStatement"],
});

defineType("CallExpression", {
visitor: ["callee", "arguments"],
const callOrNew = {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jul 14, 2017

Member

Could we just have NewExpression inherit from CallExpression?

},
id: {
validate: assertNodeType("Identifier"),
optional: true, // May be null for `export default function`

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jul 14, 2017

Member

Who's bright idea was this?!

This comment has been minimized.

Copy link
@Jessidhia

Jessidhia Jul 14, 2017

Member

export default function () {} should be named *default*, which you can't really represent as a valid Identifier :)

This comment has been minimized.

Copy link
@jridgewell
assertEach(assertNodeType("LVal")),
),
...functionCommon,
expression: {

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jul 14, 2017

Member

Instead of adding this, we should be removing it from babylon.

},
id: {
validate: assertNodeType("Identifier"),
optional: true, // Missing if this is the child of an ExportDefaultDeclaration.

This comment has been minimized.

Copy link
@jridgewell

jridgewell Jul 14, 2017

Member

UGH. 🙄

This comment has been minimized.

Copy link
@Jessidhia

Jessidhia Jul 14, 2017

Member

Same reason as function. They're both special-cased in the spec to allow omitting the name specifically in this case.

@hzoo
hzoo approved these changes Jul 25, 2017
Copy link
Member

hzoo left a comment

awesome work again andy, sorry for the wait

@hzoo hzoo merged commit 248743e into babel:7.0 Jul 25, 2017
3 checks passed
3 checks passed
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 85.77% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@andy-ms andy-ms deleted the andy-ms:ts-definitions branch Jul 26, 2017
@lock lock bot added the outdated label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants
You can’t perform that action at this time.