Skip to content

chore: add more comments on ImportRecord#1640

Merged
hyf0 merged 1 commit intomainfrom
07-16-chore_add_more_comments_on_importrecord_
Jul 16, 2024
Merged

chore: add more comments on ImportRecord#1640
hyf0 merged 1 commit intomainfrom
07-16-chore_add_more_comments_on_importrecord_

Conversation

@hyf0
Copy link
Member

@hyf0 hyf0 commented Jul 16, 2024

Description

Copy link
Member Author

hyf0 commented Jul 16, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @hyf0 and the rest of your teammates on Graphite Graphite

@netlify
Copy link

netlify bot commented Jul 16, 2024

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 8856893
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/66962ca22b1cfc0008f4f96b

pub namespace_ref: SymbolRef,
/// If it is `import * as ns from '...'` or `export * as ns from '...'`
pub contains_import_star: bool,
/// If it is `import def from '...'`, `import { default as def }`, `export { default as def }` or `export { default } from '...'`
Copy link
Member

@IWANABETHATGUY IWANABETHATGUY Jul 16, 2024

Choose a reason for hiding this comment

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

what about renaming these two fields to contains_star and contains_default ?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are indeed treat as import_xxx

// We will pretend `export { [imported] as [export_name] }` to be `import `
let generated_imported_as_ref = (
self.idx,
self.symbols.create_symbol(
if export_name == "default" {
let importee_repr = self.result.import_records[record_id]
.module_request
.as_path()
.representative_file_name();
let importee_repr = legitimize_identifier_name(&importee_repr);
format!("{importee_repr}_default").into()
} else {
export_name.into()
},
self.scopes.root_scope_id(),
),
)
.into();
self.current_stmt_info.declared_symbols.push(generated_imported_as_ref);
let name_import = NamedImport {
imported: imported.into(),
imported_as: generated_imported_as_ref,
record_id,
span_imported,
};
if name_import.imported.is_default() {
self.result.import_records[record_id].contains_import_default = true;
}
self.result.named_imports.insert(generated_imported_as_ref, name_import);
self
.result
.named_exports
.insert(export_name.into(), LocalExport { referenced: generated_imported_as_ref });
}
.

contains_star might be confused with export * from '..'

@hyf0 hyf0 enabled auto-merge July 16, 2024 08:24
@hyf0 hyf0 added this pull request to the merge queue Jul 16, 2024
@github-actions
Copy link
Contributor

Benchmarks Rust

  • target: main(f790e8c)
  • pr: 07-16-chore_add_more_comments_on_importrecord_(8856893)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     49.1±1.17ms        ? ?/sec    1.00     48.9±0.63ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     58.0±1.32ms        ? ?/sec    1.01     58.7±0.53ms        ? ?/sec
bundle/bundle@rome-ts                                        1.00     93.3±0.88ms        ? ?/sec    1.02     95.0±0.97ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                              1.00    112.9±1.28ms        ? ?/sec    1.01    113.7±1.34ms        ? ?/sec
bundle/bundle@threejs                                        1.00     27.5±0.74ms        ? ?/sec    1.02     28.1±1.53ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     37.5±0.91ms        ? ?/sec    1.00     37.5±0.69ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    301.8±4.20ms        ? ?/sec    1.03    310.3±5.64ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.01    396.5±8.46ms        ? ?/sec    1.00    392.5±6.15ms        ? ?/sec
scan/scan@rome-ts                                            1.00     75.7±0.76ms        ? ?/sec    1.00     75.7±0.85ms        ? ?/sec
scan/scan@rome-ts-sourcemap                                  1.00     75.2±0.74ms        ? ?/sec    1.01     75.6±0.96ms        ? ?/sec
scan/scan@threejs                                            1.00     20.5±0.55ms        ? ?/sec    1.00     20.5±0.68ms        ? ?/sec
scan/scan@threejs-sourcemap                                  1.03     20.9±0.28ms        ? ?/sec    1.00     20.3±0.19ms        ? ?/sec
scan/scan@threejs10x                                         1.00    208.4±1.90ms        ? ?/sec    1.01    211.3±2.02ms        ? ?/sec
scan/scan@threejs10x-sourcemap                               1.00    209.5±3.08ms        ? ?/sec    1.01    210.5±1.85ms        ? ?/sec

Merged via the queue into main with commit 29431f7 Jul 16, 2024
@hyf0 hyf0 deleted the 07-16-chore_add_more_comments_on_importrecord_ branch July 16, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants