Skip to content

Comments

refactor: use LinkingMetadata::stmt_info_included to check if a stmt_info is included#7572

Merged
graphite-app[bot] merged 1 commit intomainfrom
12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included
Dec 18, 2025
Merged

refactor: use LinkingMetadata::stmt_info_included to check if a stmt_info is included#7572
graphite-app[bot] merged 1 commit intomainfrom
12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included

Conversation

@IWANABETHATGUY
Copy link
Member

@IWANABETHATGUY IWANABETHATGUY commented Dec 18, 2025

use LinkingMetadata::stmt_info_included as single source of truth to inspect if a stmt is included in the final bundle.

Copy link
Member Author

IWANABETHATGUY commented Dec 18, 2025


How to use the Graphite Merge Queue

Add the label graphite: merge 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.

@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review December 18, 2025 09:58
@IWANABETHATGUY IWANABETHATGUY changed the title refactor: use meta stmt_info_included to check if a stmt_info is included refactor: use LinkingMetadata::stmt_info_included to check if a stmt_info is included Dec 18, 2025
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from b2fcfa3 to dbde4a3 Compare December 18, 2025 13:52
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_is_included_to_check_if_a_module_is_included branch from 31eaa46 to d5c41db Compare December 18, 2025 13:52
@graphite-app graphite-app bot changed the base branch from 12-18-refactor_use_meta_is_included_to_check_if_a_module_is_included to graphite-base/7572 December 18, 2025 16:52
@graphite-app graphite-app bot force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from dbde4a3 to 8649da9 Compare December 18, 2025 17:20
@graphite-app graphite-app bot force-pushed the graphite-base/7572 branch from d5c41db to 473918d Compare December 18, 2025 17:20
@graphite-app graphite-app bot changed the base branch from graphite-base/7572 to 12-18-refactor_use_meta_is_included_to_check_if_a_module_is_included December 18, 2025 17:20
@graphite-app graphite-app bot force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from 8649da9 to 4883c75 Compare December 18, 2025 17:20
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from 4883c75 to d8ccfb2 Compare December 18, 2025 18:03
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_is_included_to_check_if_a_module_is_included branch from 473918d to 4f8ebd2 Compare December 18, 2025 18:03
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from d8ccfb2 to f55d9b8 Compare December 18, 2025 18:21
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_is_included_to_check_if_a_module_is_included branch from 4f8ebd2 to a7c247c Compare December 18, 2025 18:21
@graphite-app graphite-app bot changed the base branch from 12-18-refactor_use_meta_is_included_to_check_if_a_module_is_included to graphite-base/7572 December 18, 2025 18:35
@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 18, 2025

Merge activity

@graphite-app graphite-app bot changed the base branch from graphite-base/7572 to main December 18, 2025 18:44
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from f55d9b8 to 2faee0c Compare December 18, 2025 19:07
@netlify
Copy link

netlify bot commented Dec 18, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit c36f65c
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/694458443e93d9000819e47a

Copilot AI review requested due to automatic review settings December 18, 2025 19:21
@IWANABETHATGUY IWANABETHATGUY force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from 2faee0c to 57a292d Compare December 18, 2025 19:21
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 PR refactors the codebase to use LinkingMetadata::stmt_info_included as the single source of truth for checking whether a statement is included in the final bundle. Previously, this information was duplicated in both StmtInfo::is_included and LinkingMetadata::stmt_info_included, leading to redundancy and potential synchronization issues.

Key changes:

  • Removed the is_included field from the StmtInfo struct
  • Updated all code sites to check statement inclusion via LinkingMetadata::stmt_info_included[idx] instead
  • Modified to_debug_stmt_info_for_tree_shaking to accept is_included as a parameter rather than reading it from the struct

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/rolldown_common/src/types/stmt_info.rs Removed is_included field from StmtInfo; updated debug method to accept inclusion status as parameter
crates/rolldown_common/src/module/normal_module.rs Updated to pass stmt_info_included from metadata to debug method; changed iteration to use iter_enumerated()
crates/rolldown/src/utils/chunk/deconflict_chunk_symbols.rs Changed filtering logic to check meta.stmt_info_included[idx] instead of stmt_info.is_included
crates/rolldown/src/stages/link_stage/wrapping.rs Removed is_included: false field initialization when creating wrapper statements
crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs Removed code that copied inclusion status back to stmt_info.is_included; changed signature of include_runtime_symbol from &mut self to &self
crates/rolldown/src/stages/link_stage/patch_module_dependencies.rs Updated to filter statements using meta.stmt_info_included[idx] instead of checking the field directly
crates/rolldown/src/stages/link_stage/create_exports_for_ecma_modules.rs Removed is_included: false field initialization when creating export-related statements
crates/rolldown/src/stages/generate_stage/compute_cross_chunk_links.rs Changed to check statement inclusion via linking_info.stmt_info_included[stmt_info_idx]
crates/rolldown/src/module_finalizers/mod.rs Updated to use linking_info.stmt_info_included[stmt_info_idx] for checking statement inclusion
crates/rolldown/src/module_finalizers/impl_visit_mut.rs Changed to use linking_info.stmt_info_included[idx] for checking wrapper and symbol inclusion

After thoroughly reviewing all the changes, I found no issues with the refactoring. The changes are consistent, correct, and improve the code by eliminating data duplication. All index accesses are properly aligned, and the logic has been correctly updated across all usage sites.


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

…t_info is included (#7572)

use `LinkingMetadata::stmt_info_included` as single source of truth to inspect if a stmt is included in the final bundle.
@graphite-app graphite-app bot force-pushed the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch from 57a292d to c36f65c Compare December 18, 2025 19:38
@graphite-app graphite-app bot merged commit c36f65c into main Dec 18, 2025
31 checks passed
@graphite-app graphite-app bot deleted the 12-18-refactor_use_meta_stmt_info_included_to_check_if_a_stmt_info_is_included branch December 18, 2025 19:49
shulaoda added a commit that referenced this pull request Dec 22, 2025
## [1.0.0-beta.56] - 2025-12-22

### 💥 BREAKING CHANGES

- rename `MIXED_EXPORT` error to `MIXED_EXPORTS` (#7565) by @sapphi-red

### 🚀 Features

- rename `id` property to `exporter` in CIRCULAR_REEXPORT error (#7592) by @sapphi-red
- add `ids` property to `CIRCULAR_DEPENDENCY` error (#7591) by @sapphi-red
- node/dev: expose `devMode.lazy` (#7549) by @hyf0
- set log and pos properties for `parseAst` function errors (#7568) by @sapphi-red
- set log and pos properties for logs (#7567) by @sapphi-red
- test-dev-sever: support to manually configure port, run tests in concurrent (#7576) by @hyf0
- add `exporter` property to `MISSING_EXPORT` error (#7564) by @sapphi-red
- add `id` property to `PARSE_ERROR` error (#7563) by @sapphi-red
- support ImporterId hook filter (#7540) by @IWANABETHATGUY

### 🐛 Bug Fixes

- types: better "go to definition" experience for interface `OutputPlugin` (#7610) by @KazariEX
- `postBanner` content should be placed after shebang (#7583) by @btea
- use sanitized filename for preserve modules chunk name (#7603) by @IWANABETHATGUY
- correct filter out unused cjs namespace  (#7602) by @IWANABETHATGUY
- watch: property respect `notify.pollInternal` and `notify.compareContents` (#7595) by @sapphi-red
- make `cleanDir` work with default output directory (#7579) by @shulaoda
- merge `MISSING_NAME_OPTION_FOR_UMD_EXPORT` error to `MISSING_NAME_OPTION_FOR_IIFE_EXPORT` error (#7566) by @sapphi-red
- dev/hmr: ensure cjs modules with no exports reference correct `module` identifier (#7544) by @leegeunhyeok

### 🚜 Refactor

- remove `stable_id` field from `PARSE_ERROR` error (#7593) by @sapphi-red
- make include_runtime_symbol reuseable after linking stage (#7580) by @IWANABETHATGUY
- rust/dev: construct the bundler within itself (#7553) by @hyf0
- rust/watcher: polish API of `Watcher` struct (#7551) by @hyf0
- use `LinkingMetadata::stmt_info_included` to check if a stmt_info is included (#7572) by @IWANABETHATGUY
- use `LinkingMetadata::is_included` to check if a module is included (#7571) by @IWANABETHATGUY
- store module and stmt_info is included info to module meta (#7570) by @IWANABETHATGUY
- make include_* method reunsable after linking stage (#7552) by @IWANABETHATGUY
- rust/watcher: construct the bundler within watcher itself (#7550) by @hyf0
- extract make include_runtime_symbol reusable (#7546) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- renovate: add `kill-port` in `ignoreDeps` in renovate.json (#7619) by @sapphi-red
- deps: update rust crates (#7617) by @renovate[bot]
- deps: update npm packages (#7616) by @renovate[bot]
- deps: update github-actions (#7615) by @renovate[bot]
- ci: skip benchmark workflows on draft PRs (#7611) by @Copilot
- deps: update dependency rolldown-plugin-dts to ^0.19.0 (#7607) by @renovate[bot]
- deps: update dependency oxlint-tsgolint to v0.10.0 (#7601) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.18.4 (#7599) by @renovate[bot]
- deps: update notify (#7594) by @sapphi-red
- test-dev-server: add retry mechanism to hmr-full-bundle-mode tests (#7588) by @Copilot
- deps: update napi to v3.7.1 (#7590) by @renovate[bot]
- add JSDoc documentation for memfs type (#7587) by @Copilot
- deps: update dependency oxlint to v1.34.0 (#7589) by @renovate[bot]
- move some tests in ignored-by-unsupported-features that are passing (#7569) by @sapphi-red
- deps: update dependency oxlint-tsgolint to v0.9.2 (#7582) by @renovate[bot]
- deps: update oxc resolver to v11.16.0 (#7574) by @renovate[bot]
- test/dev-server: don't run `pnpm install` during tests (#7560) by @hyf0
- test/dev-server: use `kill-port@1` to improve performance (#7575) by @hyf0
- normalize error object to make some Rollup tests pass (#7562) by @sapphi-red
- ci: separate dev-server(hmr) tests and normal tests (#7558) by @hyf0
- ci: make native rolldown build reusable (#7557) by @hyf0
- resolve some TODOs (#7561) by @sapphi-red

### ❤️ New Contributors

* @KazariEX made their first contribution in [#7610](#7610)
* @leegeunhyeok made their first contribution in [#7544](#7544)

Co-authored-by: shulaoda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants