refactor!: only call `closeBundle` hook when bundling actually happens by shulaoda · Pull Request #5715 · rolldown/rolldown

shulaoda · 2025-08-13T02:16:55Z

By default, closed is set to true. This is a difference from Rollup, because in Rolldown, rolldown.rolldown will not execute any build logic by default, and it will only initialize the bundler when generate or write is called.

Why do this?

In Rollup, rollup.rollup runs a full build flow, triggering build hooks and caching results, so a bundler instance is always available to be closed later. In contrast, rolldown.rolldown only returns a JS-side object without initializing a Rust-side bundler. The bundler is only created when generate or write is called, which means no build hooks are triggered beforehand.

The previous logic had two issues I think:

It always created a new bundler just to call close, rather than closing the existing instance.
If no instance existed, it would still create one solely to trigger the closeBundle hook — but since no build hooks had been run, this was effectively meaningless.

This difference in architecture between Rollup and Rolldown means that blindly emulating Rollup’s behavior here was misleading. The original code’s sole purpose of triggering closeBundle without a preceding build doesn’t align with how Rolldown’s lifecycle works.

shulaoda · 2025-08-13T02:17:11Z

refactor!: only call closeBundle hook when bundling actually happens #5715 👈 (View in Graphite)
main

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

netlify · 2025-08-13T02:17:46Z

✅ Deploy Preview for rolldown-rs ready!

Name	Link
🔨 Latest commit	`3f2dc9c`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/689df8276b928c00096880db
😎 Deploy Preview	https://deploy-preview-5715--rolldown-rs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

github-actions · 2025-08-13T02:30:33Z

Benchmarks Rust

target: main(bbf0382)
pr: 08-13-refactor_improve_bundler_close_logic(3f2dc9c)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     79.7±2.39ms        ? ?/sec    1.07     85.1±1.76ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.03     99.2±5.99ms        ? ?/sec    1.00     96.1±1.55ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    118.7±2.90ms        ? ?/sec    1.01    119.7±2.66ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    140.9±2.59ms        ? ?/sec    1.03    145.4±2.92ms        ? ?/sec
bundle/bundle@threejs                                        1.00     46.0±2.71ms        ? ?/sec    1.02     47.0±1.17ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     54.4±1.40ms        ? ?/sec    1.00     54.7±0.70ms        ? ?/sec
bundle/bundle@threejs10x                                     1.02   493.6±10.19ms        ? ?/sec    1.00    484.4±6.89ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.02    576.0±6.46ms        ? ?/sec    1.00    564.9±3.73ms        ? ?/sec
scan/scan@rome_ts                                            1.01     98.6±2.36ms        ? ?/sec    1.00     97.6±3.86ms        ? ?/sec
scan/scan@threejs                                            1.01     35.1±1.94ms        ? ?/sec    1.00     34.7±0.84ms        ? ?/sec
scan/scan@threejs10x                                         1.00   354.8±11.75ms        ? ?/sec    1.04   369.3±13.63ms        ? ?/sec

graphite-app · 2025-08-13T02:59:15Z

Merge activity

Aug 13, 2:59 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
Aug 13, 2:59 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..

packages/rolldown/tests/build-api/build-api.test.ts

pkg-pr-new · 2025-08-13T03:35:14Z

Open in StackBlitz

@rolldown/browser

npm i https://pkg.pr.new/@rolldown/browser@5715

@rolldown/debug

npm i https://pkg.pr.new/@rolldown/debug@5715

@rolldown/pluginutils

npm i https://pkg.pr.new/@rolldown/pluginutils@5715

rolldown

npm i https://pkg.pr.new/rolldown@5715

@rolldown/binding-android-arm64

npm i https://pkg.pr.new/@rolldown/binding-android-arm64@5715

@rolldown/binding-darwin-arm64

npm i https://pkg.pr.new/@rolldown/binding-darwin-arm64@5715

@rolldown/binding-darwin-x64

npm i https://pkg.pr.new/@rolldown/binding-darwin-x64@5715

@rolldown/binding-freebsd-x64

npm i https://pkg.pr.new/@rolldown/binding-freebsd-x64@5715

@rolldown/binding-linux-arm-gnueabihf

npm i https://pkg.pr.new/@rolldown/binding-linux-arm-gnueabihf@5715

@rolldown/binding-linux-arm64-gnu

npm i https://pkg.pr.new/@rolldown/binding-linux-arm64-gnu@5715

@rolldown/binding-linux-arm64-musl

npm i https://pkg.pr.new/@rolldown/binding-linux-arm64-musl@5715

@rolldown/binding-linux-x64-gnu

npm i https://pkg.pr.new/@rolldown/binding-linux-x64-gnu@5715

@rolldown/binding-linux-x64-musl

npm i https://pkg.pr.new/@rolldown/binding-linux-x64-musl@5715

@rolldown/binding-openharmony-arm64

npm i https://pkg.pr.new/@rolldown/binding-openharmony-arm64@5715

@rolldown/binding-wasm32-wasi

npm i https://pkg.pr.new/@rolldown/binding-wasm32-wasi@5715

@rolldown/binding-win32-arm64-msvc

npm i https://pkg.pr.new/@rolldown/binding-win32-arm64-msvc@5715

@rolldown/binding-win32-ia32-msvc

npm i https://pkg.pr.new/@rolldown/binding-win32-ia32-msvc@5715

@rolldown/binding-win32-x64-msvc

npm i https://pkg.pr.new/@rolldown/binding-win32-x64-msvc@5715

commit: 48a5ad0

IWANABETHATGUY · 2025-08-14T02:45:35Z

@sapphi-red , can you have a look at whether these two tests are important for the user?

crates/rolldown_binding/src/lib.rs

sapphi-red · 2025-08-14T09:00:46Z

The new behavior makes sense to me.

I think we should add a test that ensures that the closeBundle hook is not called when the bundler is not created (= when options hook or other hooks are not called).

test('closeBundle hook is not called if closed directly', async () => {
  await expect(async () => {
    const bundle = await rolldown({
      input: './main.js',
      cwd: import.meta.dirname,
      plugins: [
        {
          name: 'test',
          closeBundle() {
            this.error('close bundle error');
          },
        },
      ],
    });
    await bundle.close();
  }).not.toThrow()
});

packages/rolldown/tests/build-api/build-api.test.ts

Related to #5720 Since #5715 is still pending review, we will merge it separately for now.

@shulaoda

## [1.0.0-beta.33] - 2025-08-18 ### 💥 BREAKING CHANGES - only call `closeBundle` hook when bundling actually happens (#5715) by @shulaoda ### 🚀 Features - rolldown_plugin_vite_css_post: align transform logic except minify (#5768) by @shulaoda - rolldown_plugin_vite_css_post: align html inline css logic (#5767) by @shulaoda - support merge cjs ns in module group level (#5760) by @IWANABETHATGUY - rolldown_plugin_vite_css_post: filter transform id (#5766) by @shulaoda - rolldown: oxc v0.82.2 (#5754) by @Boshen - rollup-test: log error when `pringStatus` (#5744) by @situ2001 - rolldown_plugin_vite_css_post: initialize (#5743) by @shulaoda - rolldown_plugin_vite_css: align `transform` hook logic (#5736) by @shulaoda - rolldown_plugin_vite_css: align partial transform hook logic (#5733) by @shulaoda - add original wrap_kind (#5729) by @IWANABETHATGUY - concatenateWrappedModule (#5724) by @IWANABETHATGUY - rolldown: oxc v0.82.1 (#5717) by @Boshen - improve error message for `unresolved_import` when platform is `neutral` (#5700) by @IWANABETHATGUY ### 🐛 Bug Fixes - rolldown_plugin_transform: merge tsconfig jsx options even when `oxc.jsx.runtime` is set (#5771) by @hi-ogawa - jsx preserve break component which is default export (#5764) by @shulaoda - rolldown_plugin_asset: should directly stringify raw content (#5749) by @situ2001 - resolve symbol deconfliction order for cross-chunk imports by @IWANABETHATGUY - rolldown_error: improve resolve diagnostic message (#5740) by @shulaoda - vitest ci failed (#5741) by @IWANABETHATGUY - rolldown: options `context` should be available in renderStart (#5672) by @situ2001 - ensure lazy module eval order when import variable from other chunk (#5727) by @IWANABETHATGUY - wasm build panic on stackblitz (#5723) by @shulaoda - plugin/vite-resolve: try non-prefixed id before prefixed id (#5711) by @sapphi-red - devtool: shouldn't filter out spans for devtool use case (#5713) by @hyf0 - plugin/vite-resolve: fallback on more resolution errors that happened when trying with prefix (#5710) by @sapphi-red - plugin/vite-resolve: don't consider ids with `npm:` prefix as a built-in module (#5709) by @sapphi-red ### 🚜 Refactor - rolldown_plugin_json: use common plugin utils (#5769) by @shulaoda - hmr: remove unnecessary code of handling runtime module (#5752) by @hyf0 - hmr: enhance HMR update logic and improve clarity (#5748) by @hyf0 - improve `ScopeHoistingFinalizerContext` (#5739) by @shulaoda - move `finalize_normal_module` into `ScopeHoistingFinalizerContext` (#5738) by @shulaoda - private fields `wrap_kind` and `original_wrap_kind` and keep them sync (#5730) by @IWANABETHATGUY ### 📚 Documentation - rolldown_plugin_data_uri: update README (#5746) by @situ2001 - install guide for minor platforms (#5716) by @sapphi-red - update description for platform `neutral` (#5701) by @IWANABETHATGUY ### ⚡ Performance - hmr: only refetch changed modules (#5753) by @hyf0 - rolldown_plugin_reporter: gzip size computation (#5734) by @IWANABETHATGUY - hmr: reuse previous ast for non-changed modules (#5725) by @hyf0 - rolldown_ecmascript: do not run semantic twice for `dce-only` (#5707) by @Boshen ### 🧪 Testing - rolldown: should await for `toMatchFileSnapshot` (#5759) by @situ2001 - hmr: improve test of `import.meta.hot.invalidate` (#5747) by @hyf0 ### ⚙️ Miscellaneous Tasks - deps: lock file maintenance (#5765) by @renovate[bot] - deps: lock file maintenance rust crates (#5763) by @renovate[bot] - deps: lock file maintenance npm packages (#5762) by @renovate[bot] - deps: update github-actions (#5755) by @renovate[bot] - deps: update dependency tinybench to v5 (#5756) by @renovate[bot] - deps: update github-actions (major) (#5757) by @renovate[bot] - deps: update crate-ci/typos action to v1.35.4 (#5714) by @renovate[bot] - prepare-release: regenerate `binding.js` after version bump (#5704) by @shulaoda - deps: update dependency tsdown to v0.14.1 (#5705) by @renovate[bot] - rollup-tests: skip occasionally failing test case (#5703) by @shulaoda - update binding.js (#5702) by @shulaoda Co-authored-by: sapphi-red <[email protected]>

Clarified the behavior described in #6910 and #5715.

shulaoda marked this pull request as ready for review August 13, 2025 02:19

shulaoda marked this pull request as draft August 13, 2025 02:20

shulaoda force-pushed the 08-13-refactor_improve_bundler_close_logic branch from 7cf4785 to 92d465d Compare August 13, 2025 02:59

shulaoda force-pushed the 08-13-refactor_improve_bundler_close_logic branch from 92d465d to db6a26f Compare August 13, 2025 03:02

shulaoda marked this pull request as ready for review August 13, 2025 03:02

graphite-app bot reviewed Aug 13, 2025

View reviewed changes

packages/rolldown/tests/build-api/build-api.test.ts Outdated Show resolved Hide resolved

shulaoda force-pushed the 08-13-refactor_improve_bundler_close_logic branch from db6a26f to 48a5ad0 Compare August 13, 2025 03:05

shulaoda added the trigger: preview label Aug 13, 2025

shulaoda marked this pull request as draft August 13, 2025 03:57

shulaoda marked this pull request as ready for review August 13, 2025 11:00

shulaoda requested review from IWANABETHATGUY and hyf0 August 14, 2025 02:17

IWANABETHATGUY requested a review from sapphi-red August 14, 2025 02:45

graphite-app bot reviewed Aug 14, 2025

View reviewed changes

crates/rolldown_binding/src/lib.rs Outdated Show resolved Hide resolved

shulaoda mentioned this pull request Aug 14, 2025

fix: wasm build panic on stackblitz #5723

Merged

refactor: improve bundler close logic

5b6be06

shulaoda force-pushed the 08-13-refactor_improve_bundler_close_logic branch from 197bb0f to 5b6be06 Compare August 14, 2025 09:06

graphite-app bot reviewed Aug 14, 2025

View reviewed changes

packages/rolldown/tests/build-api/build-api.test.ts Outdated Show resolved Hide resolved

github-merge-queue bot pushed a commit that referenced this pull request Aug 14, 2025

fix: wasm build panic on stackblitz (#5723)

d88c249

Related to #5720 Since #5715 is still pending review, we will merge it separately for now.

hyf0 changed the title ~~refactor: improve bundler close logic~~ refactor!: only call closeBundle hook when bundling actually happens Aug 14, 2025

hyf0 approved these changes Aug 14, 2025

View reviewed changes

chore: follow the suggestion

3f2dc9c

shulaoda added this pull request to the merge queue Aug 14, 2025

Merged via the queue into main with commit 0f51fe3 Aug 14, 2025
23 checks passed

shulaoda deleted the 08-13-refactor_improve_bundler_close_logic branch August 14, 2025 15:11

This was referenced Aug 18, 2025

release: v1.0.0-beta.33 #5770

Closed

release: v1.0.0-beta.33 #5774

Merged

sapphi-red mentioned this pull request Dec 4, 2025

Decide the behavior when .close is called without .generate / .write is called #6910

Closed

sapphi-red mentioned this pull request Dec 16, 2025

docs: clarify closeBundle behavior #7525

Merged

graphite-app bot pushed a commit that referenced this pull request Dec 16, 2025

docs: clarify closeBundle behavior (#7525)

db60c46

Clarified the behavior described in #6910 and #5715.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

refactor!: only call `closeBundle` hook when bundling actually happens#5715

refactor!: only call `closeBundle` hook when bundling actually happens#5715
shulaoda merged 2 commits intomainfrom
08-13-refactor_improve_bundler_close_logic

shulaoda commented Aug 13, 2025 •

edited

Loading

Uh oh!

shulaoda commented Aug 13, 2025

Uh oh!

netlify bot commented Aug 13, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Aug 13, 2025 •

edited

Loading

Uh oh!

graphite-app bot commented Aug 13, 2025 •

edited

Loading

Uh oh!

Uh oh!

pkg-pr-new bot commented Aug 13, 2025

Uh oh!

IWANABETHATGUY commented Aug 14, 2025

Uh oh!

Uh oh!

sapphi-red commented Aug 14, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

Conversation

shulaoda commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why do this?

The previous logic had two issues I think:

Uh oh!

shulaoda commented Aug 13, 2025

Uh oh!

netlify bot commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for rolldown-rs ready!

Uh oh!

github-actions bot commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Rust

Uh oh!

graphite-app bot commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

pkg-pr-new bot commented Aug 13, 2025

Uh oh!

IWANABETHATGUY commented Aug 14, 2025

Uh oh!

Uh oh!

sapphi-red commented Aug 14, 2025

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shulaoda commented Aug 13, 2025 •

edited

Loading

netlify bot commented Aug 13, 2025 •

edited

Loading

github-actions bot commented Aug 13, 2025 •

edited

Loading

graphite-app bot commented Aug 13, 2025 •

edited

Loading