-
Notifications
You must be signed in to change notification settings - Fork 413
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(tree_shaking): pure annotation #1225
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1225 +/- ##
==========================================
+ Coverage 87.07% 87.15% +0.07%
==========================================
Files 128 128
Lines 6376 6467 +91
==========================================
+ Hits 5552 5636 +84
- Misses 824 831 +7 ☔ View full report in Codecov by Sentry. |
Benchmarks Rust
|
CodSpeed Performance ReportMerging #1225 will not alter performanceComparing Summary
|
01db5bc
to
b3479aa
Compare
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.
Good work.
// the result Variant should be `Err(usize)` | ||
let insert_index = self | ||
.attached_comment_vecmap | ||
.binary_search_by(|probe| probe.0.end.cmp(&expr.span.start)); |
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 fine with this implementation.
However, this feel not right about how to get comments of ast nodes. I was expecting we could get comment of ast node by simply using API like trivias.get_comments(expr.span)
.
Once oxc support emit comments in codegen, we could see how oxc get comments from oxc ast nodes.
@Boshen cc
@@ -112,6 +112,7 @@ impl RuntimeNormalModuleTask { | |||
); | |||
let mut symbol_for_module = AstSymbols::from_symbol_table(symbol_table); | |||
let facade_path = ResourceId::new("runtime"); | |||
let trivias = ast.with_mut(|fields| std::mem::take(fields.trivias)); |
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.
Probably, it's better to use it directly rather take out trivias
field, it might be needed in the future. But this doesn't block merging PR.
Description