Rich-Harris / esrap

Parse in reverse
MIT License
51 stars 3 forks source link

add typescript support (poc) #13

Open manuel3108 opened 1 month ago

manuel3108 commented 1 month ago

Closes #10

Goal: Provide a tool that can properly print svelte-ast including their appropriate JS / TS

Relates https://github.com/xeho91/svelte-ast-print/issues/86 Relates https://github.com/svelte-add/svelte-add/issues/193 / https://github.com/svelte-add/svelte-add/issues/507 Previous attempt, not invoking esrap: https://github.com/xeho91/svelte-ast-print/pull/90 (tried using recast, failed)

State of this PR This is a POC. My main goal was to make the failing tests of svelte-ast-print pass. I wrote a few tests for some basic functionality, but there are 100% a lot more things we might need to think about.

Notes

Todos

xeho91 commented 1 month ago

as-types is Esprima specification compatible. Meanwhile both acorn and typescript-acorn uses EStree specification. And also Svelte with parse/compile functions.

I don't have experience with Esprima, at first glance on their repository, they don't seem to overlap with ESTree. Are you sure this is the right direction to solve typing issues?

manuel3108 commented 1 month ago

as-types is Esprima specification compatible. Meanwhile both acorn and typescript-acorn uses EStree specification. And also Svelte with parse/compile functions.

I don't have experience with Esprima, at first glance on their repository, they don't seem to overlap with ESTree. Are you sure this is the right direction to solve typing issues?

No I'm not - Will need to check. This was my first guess, as we are using ast-types in svelte-add, but indeed we are currently not using acorn there.

Rich-Harris commented 3 weeks ago

I'm also not sure that the ast-types interfaces match what acorn-typescript uses, since it doesn't use ast-types. I wonder if @typescript-eslint/typescript-estree is the answer?

Either way, this is super cool, thank you — would love to see this land!

dummdidumm commented 3 weeks ago

If you mean "replace Acorn in Svelte with TS-Estree" - it's probably going to be slower: https://github.com/typescript-eslint/typescript-eslint/issues/1003

Rich-Harris commented 3 weeks ago

I fiddled around with it a bit last night and concluded it probably wouldn't work for us because there's no equivalent of parseExpressionAt

manuel3108 commented 3 weeks ago

Regarding the ast-types thing, this @typescript-eslint/types could be the solution. Seems to be providing correct intellisense. Will give it a shot

/** @param {import('@typescript-eslint/types').TSESTree.FunctionDeclaration} test */
manuel3108 commented 3 weeks ago

Are you aware of any "full typescript feature list" so that we can see which features we are still missing? I'm currently trying to work things out with the typescript docs, but that seems pretty hard. If not, do you maybe have typescript file in mind, that we can use to determine if we are printing the same output?

From the top of my head, here is what we already have / what's missing:

I don't think this PR should necessarily implement "all" of typescript. We should probably cover the most common use-cases and add the rest later on a as-needed basis.

manuel3108 commented 3 weeks ago

As for the types based on @typescript-eslint/types: They are better than having nothing, but they have differences here and there that make things pretty annoying. I have opend an issue to asking if official types exist and where to find them.

Whatever typing library we use here, should probably also be used by all downstream projects like svelte-ast-parse, svelte-add (and it's derivates), etc. So I think it's quite important that we find / create types that match the actual implementation

vaibhav135 commented 1 week ago

Hi, I am still a noob in parsers and stuff. But my question is why not use oxc rather than acorn. I think the bundle size for oxc-parser is smaller than acorn-typescript. And also it might be faster than acorn-typescipt, since oxc is written in rust (sorry no proof, just guessing).

Here are bundle size acorn-typescript

image

oxc-parser

image
manuel3108 commented 1 week ago

Good catch, that's actually on my "to experiment" list. Didn't had the time to investigate or see which AST spec oxc-parser is using.

xeho91 commented 1 week ago

Hi, I am still a noob in parsers and stuff. But my question is why not use oxc rather than acorn. I think the bundle size for oxc-parser is smaller than acorn-typescript. And also it might be faster than acorn-typescipt, since oxc is written in rust (sorry no proof, just guessing).

Here are bundle size acorn-typescript

image

oxc-parser

image
  1. This package isn't about parsing, but rather about doing it in reverse (hence the name of this package). To put it another way: return the parsed AST object back to string.
  2. Attempt to use acorn-typescript in this PR was to extract/use the interfaces for parsed AST nodes related to TypeScript. There is no official extension of existing @types/estree which doesn't include TypeScript.
  3. Bundle size doesn't include downloaded binary (compiled with Rust) which comes with installing/adding this package to your project - oxc-parser includes a script to determine which operating system you're using.
manuel3108 commented 1 week ago

I gave oxc-parser a go, but it does not seem to be there yet. As described above, the parser was never the problem, but the types that are generated from the parser.

Edit: Additionaly they seem to mix esprima and estree specs together, which makes it painfull to work with types afterwards.


Functionality wise this PR is ready in my opinion (expect the two open conversations above). Typing wise, there are still major problems as described above. To put things into numbers: On main we currently have 4 comments with @ts-expect-error, 20 on this branch. Additionally intellisense is painfully slow, due to the new typing library.

That's why im not undrafting this PR at this point

manuel3108 commented 1 day ago

Undrafted, as this is completed functionality wise. The only remaining potential issue is in this review comment: https://github.com/Rich-Harris/esrap/pull/13#discussion_r1729148522

To sumarize a lot of the previous comments:

If we can fix the review comment above and we can agree on the following thing, I think we are good to go.

I don't think this PR should necessarily implement "all" of typescript. We should probably cover the most common use-cases and add the rest later on a as-needed basis