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

AST Explorer #1933

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open

AST Explorer #1933

wants to merge 43 commits into from

Conversation

sujinleeme
Copy link
Contributor

@sujinleeme sujinleeme commented Jan 1, 2019

Happy new year! 🎊
It is a very basic concept of AST panel as following as AST explorer.

Except for an auto focusing option in AST explorer, on/off methods, empty keys and location data features are completed.

ast

I appreciate your review and help in advance.

Related Issue : Babel Roadmap - AST Explorer

sujinleeme and others added 30 commits August 27, 2018 20:28
The version number at the top of the repl page confused me, because I thought it referred to the version for the currently running repl, and I couldn't find a way to change it to `master` (because whenever you go to the repl page, it changes back to 6.x). I talked to @hzoo and he said it's the docs version (a docusaurus feature).

Since we already had some code that was hiding the docusaurus footer on the repl page, I repurposed it a bit and used the same pattern to also hide the docs version link when on the repl page.

![](https://puu.sh/BjnCd/9adc66337c.png)
* [email protected]

* Drop prettier since we run it from root
@babel-bot
Copy link
Contributor

babel-bot commented Jan 1, 2019

Deploy preview for babel ready!

Built with commit 00da94b

https://deploy-preview-1933--babel.netlify.com

@xtuc
Copy link
Member

xtuc commented Jan 7, 2019

Preview

That looks good. Would be cool to be able to click on the code and that it shows the location in the AST, as ASTExplorer.

Also, do you have an idea how we could remove our internal properties in the AST? (_fromTemplate, _letDone etc).

@namuwikilover

This comment has been minimized.

@sujinleeme
Copy link
Contributor Author

Preview

@xtuc

I removed _fromTemplate, _letDone props in AST objects. Anything else?

@nicolo-ribaudo
Copy link
Member

We should remove every property prefixed with _

@sujinleeme
Copy link
Contributor Author

sujinleeme commented Jan 20, 2019

Preview
@nicolo-ribaudo

All props that start with '_' prefixes are excluded.

@danez
Copy link
Member

danez commented Feb 9, 2019

replaces #1421

@nicolo-ribaudo
Copy link
Member

The output looks good (I haven't read the code yet).
Another possible improvement is to show type always as the first property since it is the most important one, or to hide it and print something like

BinaryExpression {
	"left": NumericLiteral {
		"value":1
	},
	"operator": "+",
	"right": NumericLiteral {
		"value":1
	}
}

Copy link
Member

@danez danez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, I left some inline comments.

Also it would be nice to have an interface element to disable tokens on the parser level and have it disabled by default (for performance, and tokens are used rarely anyway).
Babel-parser has an option to disable it.

Also as nicolo said type is the most important field, so I do not see the point in having an option to disable it. I would rather have an option that disables everything but the type.

@@ -137,6 +138,8 @@ export default function compile(code: string, config: CompileConfig): Return {
};

const transformed = Babel.transform(code, babelConfig);

astContext = JSON.stringify(transformed.ast);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this is the same as also with #1421. The ast is for the transformed output which is confusing imho. So for example if the input is () => {} and preset-env is enabled there is no ArrowFunction in the ast instead there is a FunctionExpression.

I wonder if there is a usecase were people want to see the transformed output, but atm I think it is more confusing. So I would prefer doing Babel.parse here when the ast panel is active (and also disabling all transform options in the interface maybe?)

But not sure what other people think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree 👍
Maybe we could move the "AST" option to the top and make it a dropdown?

schermata da 2019-02-10 00-05-33

const src = unflatten(flattenSrc);

return {
src: src,
Copy link
Member

@danez danez Feb 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the input is changed (props.src) the options here are ignored. So I disabled location but as soon as I change the input the locations are back. I have to enable and then disable the locations option again to remove locations.

@danez
Copy link
Member

danez commented Feb 9, 2019

There also seems to be a lot of unrelated changes to docs and versioned_docs, they should be reverted or the branch rebased.

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

Successfully merging this pull request may close these issues.