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

Migrate to TypeScript #60

Open
recmo opened this issue Oct 12, 2018 · 12 comments
Open

Migrate to TypeScript #60

recmo opened this issue Oct 12, 2018 · 12 comments
Labels
enhancement New feature or request

Comments

@recmo
Copy link

recmo commented Oct 12, 2018

There typescript typeings for solidity-parser-antlr and prettier.

I'm currently improving the AST typings to the point where most of the grammar rules are checked at compile time. With this you can write code transforms that don't corrupt the AST.

It also helps to track changes in the grammar over time. As the Solidity syntax evolves, the new typings will show where the printer and transforms need to be changed.

Finally it allows us to use more modern features of JS like lambda functions safely. Functional programming features are very useful for language processing.

@mattiaerre mattiaerre added the enhancement New feature or request label Oct 12, 2018
@mattiaerre
Copy link
Member

mattiaerre commented Oct 12, 2018

thanks for starting this conversation @recmo it'd be a cool idea to explore; what others think about this? how difficult would be to practically do the migration? if you have time to attempt a PR that would be awesome!

// cc @fvictorio @federicobond @j-f1

@fvictorio
Copy link
Member

I've never worked with typescript, but I can see how static typing would be very useful here.

@mattiaerre
Copy link
Member

totally down to take the ownership of this issue after the beta launch then!

@mattiaerre mattiaerre self-assigned this Oct 22, 2018
@mattiaerre mattiaerre added this to the rc launch milestone Oct 23, 2018
@mattiaerre
Copy link
Member

@federicobond
Copy link

I’ve worked with Typescript recently and I can only say good things about it. It will definitely be my default choice from now on in any Node/front-end project.

@mattiaerre mattiaerre removed this from the rc launch milestone Feb 12, 2020
@mattiaerre mattiaerre removed their assignment Sep 20, 2020
@mattiaerre
Copy link
Member

do we think doing this is still worth it? how big of a change having TS support would be? // cc @fvictorio @Janther

@fvictorio
Copy link
Member

It's probably worth it. I migrated the parser to typescript recently, but I'm not 100% happy with its output yet. Maybe when it's in a good state we can consider migrating here. (I'm also ok with keeping it in js, anyway.)

@Janther
Copy link
Contributor

Janther commented Feb 8, 2021

I believe we will need the types from the parser first as well.
I do believe that there will be an improvement in the code, although the work on this project regarding testing, coverage, and code architecture is already on a good level for understanding the project (we might need some comments here and there).

@mattiaerre
Copy link
Member

mattiaerre commented Feb 8, 2021

ok, let's keep it open for now and we'll revisit it soon

@fvictorio
Copy link
Member

Copying something I wrote on our internal chat here:


Before working on this, it would be nice to check how good prettier's support itself is for typescript. For example, right now we have functions like this:

const Block = {
  print: ({ node, options, path, print }) => {
    // ...
  }
};

That will surely not be typed automatically, but how hard is it to manually annotate the types here? What happens with things like path.call(print, 'trueExpression')? Will the trueExpression part be verified by typescript? That depends a lot on how prettier types are done.

I think the next step in a (potential) migration to typescript would be to answer those questions.

@sambacha
Copy link

fwiw airbnb has a tool for automating (YMMV) javascript to typescript: https://github.com/airbnb/ts-migrate

at worst it will show you how much effort you have ahead of you

@mattiaerre
Copy link
Member

Thanks, @sambacha if you like to give it a try and see how it goes, please by any means be our guest 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants