Skip to content

chore(ts-estree): draft of estree structure (replacement for ESTreeNode) #137

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

Merged
merged 2 commits into from
Jan 26, 2019

Conversation

armano2
Copy link
Collaborator

@armano2 armano2 commented Jan 25, 2019

No description provided.

@armano2 armano2 changed the title chore(estree): draft of estree structure (replacement for ESTreeNode) chore(ts-estree): draft of estree structure (replacement for ESTreeNode) Jan 25, 2019
@armano2 armano2 requested a review from bradzacher January 25, 2019 18:39
@armano2 armano2 added the enhancement New feature or request label Jan 25, 2019
@bradzacher
Copy link
Member

bradzacher commented Jan 25, 2019

I started doing this by hand on my plugin types branch!

https://github.com/typescript-eslint/typescript-eslint/pull/120/files/e44fba51ff881b642a3a96232488c3380ffabf2b..9bfbb86125c1d7c4ebe200ef1adec6462f0673e8

I was going through each case 1-by-1 and mapping the types together, and mirroring the same names for reusable type groupings that Typescript uses:

type BindingPattern = ArrayPattern | ObjectPattern;
type BindingName = Identifier | BindingPattern;
type Expression = null; // TODO - get lists from ts source and convert to estree nodes
type ForInitialiser = Expression | VariableDeclaration;
type ObjectLiteralElementLike =
| MethodDefinition
| Property
| RestElement
| SpreadElement
| TSAbstractMethodDefinition;
type Parameter = AssignmentPattern | RestElement | TSParameterProperty;
type PropertyName = Identifier | Literal;
type Statement = null; // TODO - get lists from ts source and convert to estree nodes

It's probably better to use these generated types and do a manual pass over convert.ts to make sure things match up

@armano2
Copy link
Collaborator Author

armano2 commented Jan 25, 2019

during generation i'm able to group nodes (i have functionality for this)

@bradzacher
Copy link
Member

That's awesome! But we'd have to go through the typescript AST to find out all of the reusable type groupings they use, then map those types in ESTree nodes. It's simple to do, but it's far from a trivial thing time-wise, so it's probably better left till after.

Anyways, we'd probably want to do a manual pass over these types to triple check they aren't missing anything from our conversion logic in convert.ts.

@armano2
Copy link
Collaborator Author

armano2 commented Jan 25, 2019

this is first step, i have in plans to use types in converter itself

this PR is to your branch not to master

@bradzacher
Copy link
Member

I didn't notice that. Awesome, feel free to merge it in.
I'll reconcile the two lists, delete mine and keep plugging through the logic.
I'm about 1/3 of the way through.

@bradzacher bradzacher merged commit a47e536 into typescript-eslint:migrate-plugin-to-ts Jan 26, 2019
@armano2 armano2 deleted the es-tree-nodes branch January 26, 2019 02:11
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants