feat: basic commonjs support #38
Conversation
|
!bench |
Benchmark Results |
| @@ -25,7 +25,8 @@ pub struct ScanResult { | |||
| pub import_records: IndexVec<ImportRecordId, ImportRecord>, | |||
| pub star_exports: Vec<ImportRecordId>, | |||
| pub export_default_symbol_id: Option<SymbolId>, | |||
| pub dynamic_imports: FxHashMap<Span, ImportRecordId>, | |||
| pub imports: FxHashMap<Span, ImportRecordId>, | |||
| pub module_resolution: Option<ModuleResolution>, | |||
There was a problem hiding this comment.
What's ModuleResolution?
Looking into the code, I thought it was marking a module whether is cjs or esm?
There was a problem hiding this comment.
I think it could be replaced with module_type that you gonna to add in #56.
| fn set_module_resolution(&mut self, module_resolution: ModuleResolution) { | ||
| if let Some(resolution) = &self.result.module_resolution { | ||
| if resolution != &module_resolution { | ||
| // TODO shouldn't mix esm syntax and cjs syntax |
There was a problem hiding this comment.
Is mix esm syntax and cjs syntax not allowed or not implemented?
There was a problem hiding this comment.
I remember it shouldn't support rolldown at previous rolldown meetings
There was a problem hiding this comment.
As I remember correctly, the meeting said it should be supported, but it could be enable or disable in module level. https://github.com/rolldown-rs/meeting-notes/blob/3fe04946dd858b4f5844763e0f476318b17d7854/09-18-2023.md?plain=1#L30
| .get_symbol_final_name(symbol) | ||
| .cloned() | ||
| .unwrap_or_else(|| name.into()) | ||
| // .expect(&format!("runtime symbol {name} not found")) |
There was a problem hiding this comment.
Why this was comment out? panic should be expected here.
There was a problem hiding this comment.
Because runtime symbol deconfict is not implemented.
There was a problem hiding this comment.
It's better to add a TODO comment here or raise a issue, otherwise, this is easily to be forgotten.
There was a problem hiding this comment.
I already added to runtime task.
There was a problem hiding this comment.
Have you also add that person who implementing the runtime symbol deconflicting should un-comment out this line of code?
If you haven't, I think it's just like I said above that this is easily to be forgotten.
Description
related #36
Test Plan
Add basic.