-
Notifications
You must be signed in to change notification settings - Fork 47
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: initial implementation of deno_ast #1
Conversation
typescript = ["swc_ecmascript/typescript"] | ||
view = ["dprint-swc-ecma-ast-view"] | ||
visit = ["swc_ecmascript/visit"] | ||
utils = ["swc_ecmascript/utils"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These features will allow us to only use what we need in each crate.
@@ -0,0 +1,123 @@ | |||
// Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is from the CLI.
pub use swc_ecmascript::utils; | ||
#[cfg(feature = "visit")] | ||
pub use swc_ecmascript::visit; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These re-exports makes things a lot nicer because then we only have to depend on deno_ast
and not a bunch of swc crates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed on a call with David, I"m in favor of this approach as well, it"s good to contain SWC to a single crate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
|
||
pub fn parse_program_with_post_process( | ||
params: ParseParams, | ||
post_process: impl FnOnce(Program) -> Program, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was kind of annoying and necessary for deno_lint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to maybe add a comment here with that example from deno_lint
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I"m going to add documentation comments everywhere in the second pass once everything looks ok and is integrated (so I don"t waste my time documenting stuff that will change). Right now the crate is super bare, but yes I"ll do that then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore my previous comments then! 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I"m in favor of this crate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good and a great idea.
pub use swc_ecmascript::utils; | ||
#[cfg(feature = "visit")] | ||
pub use swc_ecmascript::visit; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
pub inner: TokenOrComment, | ||
} | ||
|
||
pub fn lex(source: &str, media_type: MediaType) -> Vec<LexedItem> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some sort of doc comment for public functions should be there
tokens | ||
} | ||
|
||
fn flatten_comments( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, a docblock
|
||
pub fn parse_program_with_post_process( | ||
params: ParseParams, | ||
post_process: impl FnOnce(Program) -> Program, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore my previous comments then! 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (noting that doc will be added)
This is a very rough initial implementation. There is not much documentation and barely any tests (just the bare minimum to get going).
After we"ve agreed on the implementation I"ll spend some time adding tests and do the CI.
Downstream PRs:
I will integrate this later into dprint-plugin-typescript.