Skip to content

Comments

refactor(import_record): make resolved_module optional#7907

Merged
graphite-app[bot] merged 1 commit intomainfrom
01-15-refactor_import_record_make_resolved_module_optional
Jan 20, 2026
Merged

refactor(import_record): make resolved_module optional#7907
graphite-app[bot] merged 1 commit intomainfrom
01-15-refactor_import_record_make_resolved_module_optional

Conversation

@shulaoda
Copy link
Member

@shulaoda shulaoda commented Jan 15, 2026

No description provided.

@shulaoda Graphite App
Copy link
Member Author

shulaoda commented Jan 15, 2026 •
edited by sapphi-red
Loading

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

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

@netlify
Copy link

netlify bot commented Jan 15, 2026 •
edited
Loading

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit e88bacb
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/696f3baac92b73000837aa0f

@shulaoda shulaoda mentioned this pull request Jan 15, 2026
Merged
@shulaoda shulaoda force-pushed the 01-15-refactor_import_record_make_resolved_module_optional branch from f019b14 to a0ce790 Compare January 16, 2026 10:26
@shulaoda shulaoda mentioned this pull request Jan 16, 2026
Merged
@shulaoda shulaoda force-pushed the 01-15-refactor_import_record_make_resolved_module_optional branch 2 times, most recently from f2a9110 to 8215b7d Compare January 18, 2026 14:20
@shulaoda shulaoda marked this pull request as ready for review January 18, 2026 20:41
Copilot AI review requested due to automatic review settings January 18, 2026 20:41
Copilot AI reviewed Jan 18, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the ImportRecord data structure to make the resolved_module field optional (Option<ModuleIdx>) instead of always present. This is a significant change that affects import record handling throughout the codebase.

Changes:

  • Modified ImportRecordStateResolved to have resolved_module: Option<ModuleIdx> instead of just ModuleIdx
  • Added into_resolved_module() helper method that unwraps the Option with an expectation (for cases where the module is guaranteed to be resolved)
  • Updated all import record access patterns throughout the codebase to properly handle the optional field using patterns like filter_map, and_then, is_some_and, and early returns with let Some(...) else { ... }
  • Changed module loader to wrap resolved module indices with Some(idx) when calling into_resolved

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
crates/rolldown_common/src/types/import_record.rs Core change: made resolved_module optional and added helper method
crates/rolldown_common/src/types/symbol_ref.rs Updated to use and_then for optional module resolution
crates/rolldown_common/src/module/normal_module.rs Updated iterators to use filter_map and ? operator for optional handling
crates/rolldown/src/module_loader/module_loader.rs Wrapped resolved indices with Some() when converting to resolved records
crates/rolldown/src/stages/link_stage/*.rs Multiple files updated with proper None handling patterns
crates/rolldown/src/stages/generate_stage/*.rs Multiple files updated to handle optional resolved modules
crates/rolldown/src/module_finalizers/*.rs Updated to use and_then and proper None handling
crates/rolldown/src/hmr/*.rs Updated HMR-related code to handle optional modules
crates/rolldown/src/utils/chunk/*.rs Updated chunk-related utilities for optional handling
crates/rolldown/src/bundle/bundle.rs Updated bundle generation to filter out unresolved modules

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2026 •
edited
Loading

Benchmarks Rust

  • target: main(4b58006)
  • pr: 01-15-refactor_import_record_make_resolved_module_optional(f04ad6d)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     68.1±1.42ms        ? ?/sec    1.03     70.3±2.09ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     74.5±1.57ms        ? ?/sec    1.04     77.1±3.27ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    100.0±1.51ms        ? ?/sec    1.04    104.0±1.85ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    112.0±1.81ms        ? ?/sec    1.02    114.2±1.57ms        ? ?/sec
bundle/bundle@threejs                                        1.00     35.6±1.40ms        ? ?/sec    1.04     37.0±2.16ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     40.2±1.26ms        ? ?/sec    1.05     42.1±0.76ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    367.4±4.45ms        ? ?/sec    1.01    370.4±3.75ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    427.6±4.87ms        ? ?/sec    1.00    428.5±5.21ms        ? ?/sec
scan/scan@rome_ts                                            1.02     81.5±2.37ms        ? ?/sec    1.00     80.1±2.14ms        ? ?/sec
scan/scan@threejs                                            1.03     28.3±1.80ms        ? ?/sec    1.00     27.6±0.41ms        ? ?/sec
scan/scan@threejs10x                                         1.02    287.7±4.36ms        ? ?/sec    1.00    281.5±4.88ms        ? ?/sec

@shulaoda shulaoda marked this pull request as draft January 19, 2026 02:10
IWANABETHATGUY
IWANABETHATGUY reviewed Jan 19, 2026
View reviewed changes
IWANABETHATGUY
IWANABETHATGUY reviewed Jan 19, 2026
View reviewed changes
IWANABETHATGUY
IWANABETHATGUY reviewed Jan 19, 2026
View reviewed changes
IWANABETHATGUY
IWANABETHATGUY reviewed Jan 19, 2026
View reviewed changes
@IWANABETHATGUY
Copy link
Member

IWANABETHATGUY commented Jan 19, 2026

I prefer filter_map over for_each + if because it always reduces one indentation and makes the code more readable.

@shulaoda shulaoda marked this pull request as ready for review January 19, 2026 16:24
@shulaoda shulaoda marked this pull request as draft January 19, 2026 16:24
@shulaoda shulaoda force-pushed the 01-15-refactor_import_record_make_resolved_module_optional branch from 8215b7d to 9d55cf9 Compare January 19, 2026 17:43
@shulaoda shulaoda marked this pull request as ready for review January 19, 2026 21:54
Copilot AI reviewed Jan 19, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@shulaoda Graphite App
Copy link
Member Author

shulaoda commented Jan 20, 2026 •
edited by graphite-app bot
Loading

Merge activity

  • Jan 20, 8:23 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jan 20, 8:23 AM UTC: shulaoda added this pull request to the Graphite merge queue.
  • Jan 20, 8:33 AM UTC: Merged by the Graphite merge queue.

@graphite-app graphite-app bot force-pushed the 01-15-refactor_import_record_make_resolved_module_optional branch from f04ad6d to e88bacb Compare January 20, 2026 08:24
@graphite-app graphite-app bot merged commit e88bacb into main Jan 20, 2026
34 checks passed
@graphite-app graphite-app bot deleted the 01-15-refactor_import_record_make_resolved_module_optional branch January 20, 2026 08:33
This was referenced Jan 20, 2026
Merged
Merged
@github-actions github-actions bot mentioned this pull request Jan 21, 2026
Closed
@sapphi-red sapphi-red mentioned this pull request Jan 21, 2026
Closed
@shulaoda shulaoda mentioned this pull request Jan 21, 2026
Closed
@sapphi-red sapphi-red mentioned this pull request Jan 22, 2026
Closed
@github-actions github-actions bot mentioned this pull request Jan 22, 2026
Merged
shulaoda pushed a commit that referenced this pull request Jan 22, 2026
@github-actions
release: v1.0.0-rc.1 (#8011)
0e1ec0d 
## [1.0.0-rc.1] - 2026-01-22

### 🚀 Features

- debug_info: add facade chunk elimination reason (#7980) by @IWANABETHATGUY
- support lazy barrel optimization (#7933) by @shulaoda
- add `experimental.lazyBarrel` option (#7908) by @shulaoda
- skip unused external modules from IIFE parameter list (#7978) by @sapphi-red
- add custom panic hook for better crash reporting (#7752) by @shulaoda
- treeshake: add `invalidImportSideEffects` option (#7958) by @shulaoda
- merge allow-extension emitted chunks (#7940) by @IWANABETHATGUY
- nativeMagicString generateMap (#7944) by @IWANABETHATGUY
- Include meta.magicString in RenderChunkMeta (#7943) by @IWANABETHATGUY
- debug_info: add debug info for eliminated facade chunks (#7946) by @IWANABETHATGUY
- stablize `strictExecutionOrder` and move to `output.strictExecutionOrder` from `experimental.strictExecutionOrder` (#7901) by @sapphi-red
- add documentation link to require() error message (#7898) by @Copilot
- add `codeSplitting: boolean` and deprecate `inlineDynamicImports` (#7870) by @hyf0
- dev: change lazy module URL to `/@vite/lazy` from `/lazy` (#7884) by @sapphi-red

### 🐛 Bug Fixes

- transform JS files containing `</script>` to escape template literals (#7987) by @IWANABETHATGUY
- apply avoid-breaking-exported-api = false to clippy.toml and fix clippy errors (#7982) by @Boshen
- pass `kind` from `this.resolve` (#7981) by @sapphi-red
- rolldown_plugin_vite_resolve: ignore yarn resolution errors and fallback to other resolvers (#7968) by @sapphi-red
- renamer: prevent renaming symbols when there no conflicts (#7936) by @Dunqing
- correct minifyInterExports when emitted chunk got merged (#7941) by @IWANABETHATGUY
- deduplicate entry points when module is both emitted and dynamically imported (#7885) by @IWANABETHATGUY
- dev: add `@vite-ignore` to lazy compilation proxy module import (#7883) by @sapphi-red

### 🚜 Refactor

- rust: enable clippy nursery lint group (#8002) by @Boshen
- rust: fix inconsistent_struct_constructor clippy lint (#7999) by @Boshen
- rust: fix needless_pass_by_ref_mut clippy lint (#7994) by @Boshen
- rust: fix unnecessary_wraps clippy lint (#7993) by @Boshen
- rust: fix enum_variant_names clippy lint (#7992) by @Boshen
- fix single_match clippy lint (#7997) by @Boshen
- rust: fix redundant_clone clippy lint (#7996) by @Boshen
- rust: rename CJS to Cjs to follow upper_case_acronyms lint (#7991) by @Boshen
- rust: remove unnecessary Box wrapper around Vec in MemberExprRef (#7990) by @Boshen
- import_record: make resolved_module optional (#7907) by @shulaoda
- remove unnecessary `.parse` (#7966) by @sapphi-red
- remove unused `ImportRecordMeta::IsPlainImport` (#7948) by @shulaoda
- proper set chunk meta (#7939) by @IWANABETHATGUY
- module_loader: remove `try_spawn_with_cache` (#7920) by @shulaoda
- link_stage: simplify `ImportStatus::NoMatch` to unit variant (#7909) by @shulaoda
- improve global scope symbol reservation in chunk deconfliction (#7906) by @IWANABETHATGUY
- simplify ast unwrapping in generate stage (#7900) by @IWANABETHATGUY
- generate_stage: optimize cross-chunk imports computation (#7889) by @shulaoda
- link_stage: move runtime require logic into match branch (#7892) by @shulaoda
- link_stage: simplify runtime require reference conditions (#7891) by @shulaoda
- link_stage: inline and simplify external dynamic import check (#7890) by @shulaoda
- generate_stage: simplify external module import collection logic (#7887) by @shulaoda
- avoid redundant module lookup in TLA computation (#7886) by @shulaoda
- dev: `devEngine.compileEntry` does not return null (#7882) by @sapphi-red
- dev: fix type errors for test HMR runtime (#7881) by @sapphi-red
- dev: move `clientId` property to `DevRuntime` base class (#7880) by @sapphi-red
- dev: generate client id in browser (#7878) by @hyf0

### 📚 Documentation

- apis: organize hook filters documentation and add composable filters section (#8003) by @sapphi-red
- update `vitepress-plugin-group-icons` (#7947) by @yuyinws
- add in-depth documentation for lazy barrel optimization (#7969) by @shulaoda
- bump theme & update activeMatch for reference (#7963) by @mdong1909
- mark `build()` API as experimental (#7954) by @sapphi-red
- enhance search functionality with improved scoring and filtering logic (#7935) by @hyf0
- add minor comments to multiple types (#7930) by @sapphi-red
- refactor advanedChunks related content to adapt manual code splitting concept (#7925) by @hyf0
- apis: add content to Bundler API page (#7926) by @sapphi-red
- apis: restructure plugin API related docs (#7924) by @sapphi-red
- add plugin API docs (#7923) by @sapphi-red
- apis: add docs to important APIs (#7913) by @sapphi-red
- move the important APIs to the top of the sidebar (#7912) by @sapphi-red
- apis: add more content to CLI documentation (#7911) by @sapphi-red
- apis: generate CLI docs from --help output (#7910) by @sapphi-red
- add fathom analytics (#7896) by @mdong1909

### ⚡ Performance

- use u32 for string indices in string_wizard and rolldown to reduce memory usage (#7989) by @IWANABETHATGUY
- rust: remove all usages of `with_scope_tree_child_ids(true)` for `SemanticBuilder` (#7995) by @Dunqing
- renamer: skip unnecessary nested scope symbol processing (#7899) by @Dunqing
- module_loader: use ArcStr for importer_id to avoid string copy (#7922) by @shulaoda
- module_loader: defer `ModuleTaskOwner` construction until needed (#7921) by @shulaoda
- renamer: optimize symbol renaming by eliminating `rename_non_root_symbol` pass (#7867) by @Dunqing

### 🧪 Testing

- add lazy barrel optimization test cases (#7967) by @shulaoda

### ⚙️ Miscellaneous Tasks

- remove lazy barrel option (#8010) by @shulaoda
- mark watch API as experimental (#8004) by @sapphi-red
- deps: update dependency lodash-es to v4.17.23 [security] (#8001) by @renovate[bot]
- git ignore zed local config (#7988) by @IWANABETHATGUY
- setup publint for published packages (#7972) by @Copilot
- enable `tagged_template_transform ` uncondionally (#7975) by @IWANABETHATGUY
- deps: update oxc to v0.110.0 (#7964) by @renovate[bot]
- deps: update oxc apps (#7962) by @renovate[bot]
- ai: add upgrade-oxc Claude skill (#7957) by @Boshen
- deps: update rollup submodule for tests to v4.55.2 (#7959) by @sapphi-red
- deps: update test262 submodule for tests (#7960) by @sapphi-red
- deps: update crate-ci/typos action to v1.42.1 (#7961) by @renovate[bot]
- deps: update rust crates (#7951) by @renovate[bot]
- deps: update npm packages (#7953) by @renovate[bot]
- deps: update github-actions (#7952) by @renovate[bot]
- deps: update npm packages (#7950) by @renovate[bot]
- format magic-string test before write to disk (#7945) by @IWANABETHATGUY
- deps: update dependency rolldown-plugin-dts to ^0.21.0 (#7915) by @renovate[bot]
- deps: update dependency oxlint-tsgolint to v0.11.1 (#7914) by @renovate[bot]
- deps: update dependency diff to v8.0.3 [security] (#7904) by @renovate[bot]
- remove outdated TODO comment in `collect_depended_symbols` (#7888) by @shulaoda
- deps: update oxc resolver to v11.16.3 (#7876) by @renovate[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@IWANABETHATGUY IWANABETHATGUY IWANABETHATGUY approved these changes

@hyf0 hyf0 Awaiting requested review from hyf0

Assignees

@shulaoda shulaoda

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shulaoda @IWANABETHATGUY