Skip to content

Comments

fix(dev/lazy): remove unnecessary rewrite from top level this to undefined#8020

Merged
graphite-app[bot] merged 1 commit intomainfrom
01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_
Jan 23, 2026
Merged

fix(dev/lazy): remove unnecessary rewrite from top level this to undefined#8020
graphite-app[bot] merged 1 commit intomainfrom
01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_

Conversation

@hyf0
Copy link
Member

@hyf0 hyf0 commented Jan 22, 2026

Closes #7956.

Patch output is already ran in esm env, we don't need to rewrite this to undefined to keep semantic or reduce sizes for patch output.

For 7956, test is added within this PR.

Copy link
Member Author

hyf0 commented Jan 22, 2026


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.

@hyf0 hyf0 marked this pull request as ready for review January 22, 2026 20:00
Copilot AI review requested due to automatic review settings January 22, 2026 20:00
@hyf0 hyf0 requested review from sapphi-red and removed request for Copilot January 22, 2026 20:00
@github-actions
Copy link
Contributor

Benchmarks Rust

  • target: 01-23-fix_dev_lazy_should_keep_lazy_entries_imports_for_patch_file(5542dd9)
  • pr: 01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_(02b1f8d)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.05     72.8±1.46ms        ? ?/sec    1.00     69.3±1.40ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.03     78.2±1.69ms        ? ?/sec    1.00     76.0±1.43ms        ? ?/sec
bundle/bundle@rome_ts                                        1.04    104.8±3.16ms        ? ?/sec    1.00    100.9±1.83ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.04    116.3±1.69ms        ? ?/sec    1.00    111.8±1.58ms        ? ?/sec
bundle/bundle@threejs                                        1.00     36.8±0.57ms        ? ?/sec    1.00     36.7±2.08ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.03     42.2±0.62ms        ? ?/sec    1.00     41.0±0.65ms        ? ?/sec
bundle/bundle@threejs10x                                     1.01    375.2±4.53ms        ? ?/sec    1.00    372.4±6.67ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    429.9±5.00ms        ? ?/sec    1.01    433.7±5.16ms        ? ?/sec
scan/scan@rome_ts                                            1.00     81.4±1.84ms        ? ?/sec    1.01     82.5±1.53ms        ? ?/sec
scan/scan@threejs                                            1.00     28.2±0.39ms        ? ?/sec    1.02     28.8±1.65ms        ? ?/sec
scan/scan@threejs10x                                         1.00    289.4±7.12ms        ? ?/sec    1.01    291.0±4.68ms        ? ?/sec

@sapphi-red
Copy link
Member

It seems the tests are failing

@hyf0 hyf0 changed the base branch from 01-23-fix_dev_lazy_should_keep_lazy_entries_imports_for_patch_file to graphite-base/8020 January 23, 2026 05:56
@hyf0 hyf0 force-pushed the 01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_ branch from 02b1f8d to 2a47eb2 Compare January 23, 2026 06:47
@hyf0 hyf0 force-pushed the graphite-base/8020 branch from 5542dd9 to 0edc16c Compare January 23, 2026 06:47
Copilot AI review requested due to automatic review settings January 23, 2026 06:47
@hyf0 hyf0 changed the base branch from graphite-base/8020 to 01-23-fix_dev_lazy_should_keep_lazy_entries_imports_for_patch_file January 23, 2026 06:47
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 fixes a bug where this in static class fields was incorrectly replaced with void 0 in HMR patch output. The fix leverages the existing this_expr_replace_map populated during AST scanning to correctly identify which this expressions should be rewritten, and removes the unnecessary rewriting of top-level this to undefined for ESM modules in HMR patches.

Changes:

  • Replaced the ctx.is_current_scope_valid_for_tla() logic with this_expr_replace_map lookup to correctly identify top-level this expressions
  • Removed unnecessary rewriting of this to undefined for ESM modules in HMR patch output (patches already run in ESM environment)
  • Added test case to verify this in static class fields is not incorrectly rewritten

Reviewed changes

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

File Description
crates/rolldown/src/hmr/impl_traverse_for_hmr_ast_finalizer.rs Updated this expression rewriting logic to use this_expr_replace_map and only rewrite for CommonJS modules; removed TraverseCtxExt trait
crates/rolldown/tests/rolldown/topics/hmr/runtime_correctness/cases/require/cjs-lib.js Added test case with this in static class field to verify the fix
crates/rolldown/tests/rolldown/topics/hmr/runtime_correctness/cases/require/index.js Added assertion to verify baz3 is undefined (not set by this.baz3 = 'wrong' in static field)
crates/rolldown/tests/rolldown/topics/hmr/runtime_correctness/artifacts.snap Updated snapshots to show correct output where this in static class fields is not rewritten

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

@graphite-app graphite-app bot changed the base branch from 01-23-fix_dev_lazy_should_keep_lazy_entries_imports_for_patch_file to graphite-base/8020 January 23, 2026 06:59
@graphite-app graphite-app bot force-pushed the graphite-base/8020 branch from 0edc16c to e60205e Compare January 23, 2026 08:59
@graphite-app graphite-app bot force-pushed the 01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_ branch from 2a47eb2 to 1270c1a Compare January 23, 2026 08:59
@graphite-app graphite-app bot changed the base branch from graphite-base/8020 to main January 23, 2026 08:59
@graphite-app graphite-app bot force-pushed the 01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_ branch from 1270c1a to 2bcf7cd Compare January 23, 2026 09:00
@netlify
Copy link

netlify bot commented Jan 23, 2026

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit a2e12bc
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6973441f254e1c0008d99cdd

@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 23, 2026

Merge activity

…ndefined` (#8020)

Closes #7956.

Patch output is already ran in esm env, we don't need to rewrite `this` to `undefined` to keep semantic or reduce sizes for patch output.

For 7956, test is added within this PR.
@graphite-app graphite-app bot force-pushed the 01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_ branch from 2bcf7cd to a2e12bc Compare January 23, 2026 09:49
@graphite-app graphite-app bot merged commit a2e12bc into main Jan 23, 2026
34 checks passed
@graphite-app graphite-app bot deleted the 01-23-fix_dev_lazy_remove_unnecessary_rewrite_from_top_level_this_to_undefined_ branch January 23, 2026 10:00
This was referenced Jan 28, 2026
shulaoda added a commit that referenced this pull request Jan 28, 2026
## [1.0.0-rc.2] - 2026-01-28

⚡ Lazy Barrel Optimization

- Skips compilation of unused re-export modules in side-effect-free barrel modules
- Particularly beneficial for projects importing from large component libraries

For large component libraries like Ant Design, when you import just one component:

```js
import { Button } from 'antd';
Button;
```

| Metric                    | Without lazy barrel | With lazy barrel |
| -------------------- | --------------------- | ---------------- |
| Modules compiled | 2986                       | 250              |
| Build time               | ~210ms                  | ~50ms            |

By enabling lazy barrel, Rolldown reduces the number of compiled modules by 90% and speeds up the build by 2-4x. 

Enable it in your config:

```js
export default {
  experimental: {
    lazyBarrel: true,
  },
};
```

For more details, see the https://rolldown.rs/in-depth/lazy-barrel-optimization

### 💥 BREAKING CHANGES

- expose `\0rolldown/runtime` in transform hook (#8068) by @hyf0
- rename `rolldown:runtime` to `\0rolldown/runtime.js` (#8067) by @hyf0

### 🚀 Features

- remove inlined constants in smart mode (#8085) by @sapphi-red
- allow more options for `this.emitFile` with `type: 'prebuilt-chunk'` (#8062) by @sapphi-red
- warn when both code and postBanner contain shebang (#8039) by @Copilot

### 🐛 Bug Fixes

- update the links to Rolldown docs in the error messages (#8103) by @sapphi-red
- handle tsconfig.json load errors (#8105) by @sapphi-red
- include inlined constants in namespace object (#8099) by @sapphi-red
- vite test ci (#8084) by @IWANABETHATGUY
- renamer: nested binding shadowing external module namespace in UMD/IIFE formats (#8083) by @Dunqing
- deduplicate ESM chunk imports by canonical symbol (#8059) by @IWANABETHATGUY
- refine side-effect detection for BigInt and RegExp (#8060) by @IWANABETHATGUY
- rust: use string literal span for `new URL` error diagnostic (#8043) by @valadaptive
- rust: use ModuleType::Asset for `new URL` imports (#8035) by @valadaptive
- CJS-ESM interop - property assignment on CJS module exports (#8006) by @IWANABETHATGUY
- eliminate the facade chunk if the dynamic entry module has been merged into common chunk (#8046) by @IWANABETHATGUY
- Inlining dynamic imports broken with multiple entry points (#8037) by @IWANABETHATGUY
- devtools: revert `Chunk#id` to `Chunk#chunk_id` (#8040) by @hyf0
- invert `__exportAll` parameter logic to reduce default output size (#8036) by @Copilot
- `</script` tag search should be case insensitive (#8033) by @IWANABETHATGUY
- use directory name as-is for the variable name even if the name contained `.` (#8029) by @Copilot
- dev/lazy: remove unnecessary rewrite from top level `this` to `undefined` (#8020) by @hyf0
- dev/lazy: should keep lazy entries imports for patch file (#8019) by @hyf0
- `output.generatedCode.preset: 'es2015'` was not set by default (#8026) by @sapphi-red
- node: align option validator to types (#8023) by @sapphi-red
- node: allow `output.strictExecutionOrder` by the option validator (#8022) by @sapphi-red
- types: return `this` from on / off methods of `RolldownWatcher` (#8015) by @sapphi-red

### 🚜 Refactor

- rolldown_plugin_vite_dynamic_import_vars: remove v1 implementation (#8096) by @shulaoda
- rolldown_plugin_vite_import_glob: remove v1 implementation (#8095) by @shulaoda
- lazy-barrel: restructure lazy barrel implementation (#8070) by @shulaoda
- remove `use_built_ins` and `use_spread` from internal JSX options (#8079) by @sapphi-red
- remove `experimental.transformHiresSourcemap` (#8055) by @Copilot
- rust: use `is_data_url` more consistently (#8042) by @valadaptive
- use `FxIndexMap` to store EntryPoint (#8032) by @IWANABETHATGUY
- node: add type checks that ensures validator schema is up to date with types (#8024) by @sapphi-red

### 📚 Documentation

- link to vite plugin registry (#8086) by @sapphi-red
- lazy-barrel: improve documentation and enable in sidebar (#8072) by @shulaoda
- add more examples and details (#8054) by @sapphi-red
- in-depth: add dead code elimination page (#8007) by @sapphi-red
- update status from beta to release candidate (#8012) by @shulaoda

### ⚡ Performance

- run inline-const pass for modules that are affected by inlining (#8064) by @sapphi-red

### 🧪 Testing

- lazy-barrel: use package.json sideEffects instead of plugin hook (#8077) by @shulaoda
- lazy-barrel: enable tests and add treeshake-behavior cases (#8071) by @shulaoda

### ⚙️ Miscellaneous Tasks

- deps: update crate-ci/typos action to v1.42.3 (#8087) by @renovate[bot]
- deps: update rollup submodule for tests to v4.56.0 (#8073) by @sapphi-red
- deps: update oxc to v0.111.0 (#8063) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.21.6 (#8076) by @renovate[bot]
- deps: update test262 submodule for tests (#8074) by @sapphi-red
- deps: update crate-ci/typos action to v1.42.2 (#8069) by @renovate[bot]
- deps: update oxc apps (#8066) by @renovate[bot]
- remove `{@include ./foo.md}` from d.ts files (#8056) by @sapphi-red
- deps: update dependency oxlint-tsgolint to v0.11.2 (#8057) by @renovate[bot]
- deps: update github-actions (#8050) by @renovate[bot]
- deps: update npm packages (#8051) by @renovate[bot]
- deps: update rust crates (#8049) by @renovate[bot]
- debug: add IdxExt debug trait for human-readable index debugging (#8045) by @IWANABETHATGUY
- deps: update dependency rolldown-plugin-dts to v0.21.5 (#8034) by @renovate[bot]
- deps: update oxc resolver to v11.16.4 (#8031) by @renovate[bot]
- deps: update dependency rolldown-plugin-dts to v0.21.4 (#8030) by @renovate[bot]
- deps: update dependency rust to v1.93.0 (#8018) by @renovate[bot]
- archive 2025 beta changelog (#8014) by @shulaoda
- update release workflow version pattern from beta to rc (#8013) by @shulaoda

### ❤️ New Contributors

* @valadaptive made their first contribution in [#8043](#8043)

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