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

feat(node-swc): Babel ast translator #1465

Merged
merged 168 commits into from
May 1, 2021
Merged

Conversation

dwoznicki
Copy link
Contributor

@dwoznicki dwoznicki commented Mar 11, 2021

This pull request is a WIP.

  • Translate babel AST definitions to rust
  • Implement convert functions for swc --> babel
  • Write tests

Let me know if there's anything missing from the todo list.

@CLAassistant
Copy link

CLAassistant commented Mar 11, 2021

CLA assistant check
All committers have signed the CLA.

@kdy1
Copy link
Member

kdy1 commented Mar 11, 2021

Wow, thank you for such enormous work!

@kdy1
Copy link
Member

kdy1 commented Mar 17, 2021

Will you implement the conversion from swc ast to babel ast?
If not, I think this can be merged :)

@dwoznicki
Copy link
Contributor Author

Sorry, yes, I am working on the conversions. It's slow going for.. well, a variety of reasons. Perhaps it would be helpful if you gave me a generic example of how you expect the Babelify trait to be implemented? Seeing how you defined the BaseNode struct was really helpful when writing the Babel AST definitions.

@kdy1
Copy link
Member

kdy1 commented Mar 17, 2021

@dwoznicki Good to hear. I'll add some examples for it tomorrow. (It's time to go sleep in my country)

@kdy1
Copy link
Member

kdy1 commented Mar 18, 2021

I did some work. Please feel free to ping me if you need ant help.

See: https://github.com/kdy1/swc/tree/d872d001bf387dd7ebe1c27ce87ab438ab42c556/native/babel-compat/src

I added some methods to convert 'common' types, and converting other ast nodes would be simillar.

@dwoznicki
Copy link
Contributor Author

Cool, thanks. I'll let you know if I have any trouble.

@dwoznicki
Copy link
Contributor Author

@kdy1 Hey, a couple questions for you.

  1. How can I get the Span for nodes that use the #[span] macro? E.g. pat::KeyValuePatProp.
  2. When I have more questions, where should I ask them? Here? Discussions?

@kdy1
Copy link
Member

kdy1 commented Mar 21, 2021

@dwoznicki

  1. You can use .span() from the trait swc_common::Spanned instead.
  2. Anywhere is ok for me.

@dwoznicki
Copy link
Contributor Author

dwoznicki commented Mar 22, 2021

@kdy1 Cool, thanks. Okay, another question.

I've found several places where the swc node does not convert cleanly into a babel node, and I need to throw/return an error if the tree is invalid. How should I handle this? Should I modify the babelify() function to return a Result?

@kdy1
Copy link
Member

kdy1 commented Mar 22, 2021

@dwoznicki I think so. Panic would also work, but it doesn't look good.

@dwoznicki
Copy link
Contributor Author

@kdy1 Hey, quick question. The swc Module and Script shebang field is a JsWord, but Babel's corresponding interpreter field is an InterpreterDirective which expects a base node. Is there any way to get a span from a JsWord?

@kdy1
Copy link
Member

kdy1 commented Mar 29, 2021

@dwoznicki Currently there's no way to do it.
I think the only way to get span without changing ast would be using span (from Module / Script) with https://docs.rs/swc_common/0.10.13/swc_common/struct.SourceMap.html#method.span_take_while.

Taking lower BytePos from span, creating a new span from lower, and extending it until newline characters would work.

@dwoznicki
Copy link
Contributor Author

Great, yeah, that appears to work. Thanks.

@kdy1
Copy link
Member

kdy1 commented Mar 31, 2021

I'll extract ast definitions from the pr so that I can use them to implement the babel plugin runner.

@dwoznicki
Copy link
Contributor Author

Oh, I've fixed a couple things since. Let me make a push right quick. I suspect there will be more problems with my definitions that'll need fixing.

@kdy1
Copy link
Member

kdy1 commented Mar 31, 2021

@dwoznicki Oh, thanks!

@dwoznicki
Copy link
Contributor Author

It's up. Are you implementing a babel plugin runner in Rust?

@kdy1
Copy link
Member

kdy1 commented Mar 31, 2021

@devongovett I'd like to, but I need to do some experiments to see if it's possible.

@kdy1
Copy link
Member

kdy1 commented Mar 31, 2021

And thanks!

@dwoznicki
Copy link
Contributor Author

dwoznicki commented Mar 31, 2021

@kdy1 If you manage to get it working, can you let me know if you solve this issue? napi-rs/napi-rs#436

I got stuck here while trying to implement Webpack plugins.

@kdy1
Copy link
Member

kdy1 commented Mar 31, 2021

Ok, I'll try workarouding it and I'll cc you when I make a success.

@dwoznicki
Copy link
Contributor Author

First pass at conversion is done. It's pretty rough, and completely untested. I'll fix it up as I write tests.

@kdy1
Copy link
Member

kdy1 commented Mar 31, 2021

Sounds great!
But I'm also blocked by napi-rs/napi-rs#436

@Brooooooklyn
Copy link
Member

I'll take a look tonight

@dwoznicki
Copy link
Contributor Author

@kdy1 Hey, I want to get your thoughts on a couple things.

  1. How rigid should the swc --> babel AST conversion be? That is, do we want to be able to babelify an swc AST and have it produce the exact same output that we would get if we parsed it with babel directly? I'm leaning towards an exact AST translation, btw, but just want to check.
  2. On a related note, there appear to be some places where the babel node definitions from here don't match the actual output of @babel/parse. For example, the parsed babel AST has an "errors" field in the File node, but the File definition does not mention errors. Should I modify our Rust babel node definition to match the actual output?

@kdy1
Copy link
Member

kdy1 commented Apr 2, 2021

@dwoznicki

For 1, I think the safety line would be babel-plugin compatibility. I expect the plugin system to work even though some ast differs (e.g. Pat::Expr in PatOrExpr::Pat), and if not, it can be fixed in the future.

For 2, I don't think so. Almost, if not all, babel plugins would not depend on some undocumented fields. And same as 1, we can fit it in the future if it causes a problem.

@dwoznicki
Copy link
Contributor Author

Cool thanks. Okay, another more specific question.

Babel wraps the top level Program in a File node, which swc of course doesn't bother with. So when I babelify an swc Program, if it starts at bytepos 0, can I assume that this is the top level Program?

@kdy1
Copy link
Member

kdy1 commented Apr 2, 2021

Yes, but not vice versa.

The swc_common::SourceMap is shared and as a result, secondly parsed ast would have non-zero starting byte position.

@dwoznicki
Copy link
Contributor Author

@kdy1 Hey, I think this PR is getting close. There are almost certainly more inconsistencies I've missed, but I'm running out of ideas for tests to write. With one big exception... TypeScript.

I've never written TypeScript, so I only understand it conceptually. Writing TS tests is going to take me a while without help. So the question is, do you want to wait for TS compatibility testing before merging?

@kdy1
Copy link
Member

kdy1 commented May 1, 2021

@dwoznicki Wow. Nice work! Thank you so much!

I don't think typescript conversion is important at the moment. It's because using type information is very complex, and to use type information, what you need the ast of tsc, not babel. So no one would try to use type ast nodes of babel.

@dwoznicki
Copy link
Contributor Author

dwoznicki commented May 1, 2021

Great! Well, I think this PR is actually ready in the case. I suspect there will be a number of bugs at first, so feel free to @ me for issues.

@kdy1
Copy link
Member

kdy1 commented May 1, 2021

Great Thanks!
I'll try implementing babel plugin runner asap :)

@kdy1 kdy1 merged commit d1415f9 into swc-project:master May 1, 2021
@kdy1 kdy1 added this to the v1.2.55 milestone May 1, 2021
@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants