Skip to content

fix: only uppercase facade symbols in JSX preserve mode#8519

Merged
IWANABETHATGUY merged 3 commits intomainfrom
01-29-fix_6036
Mar 4, 2026
Merged

fix: only uppercase facade symbols in JSX preserve mode#8519
IWANABETHATGUY merged 3 commits intomainfrom
01-29-fix_6036

Conversation

@IWANABETHATGUY
Copy link
Member

@IWANABETHATGUY IWANABETHATGUY commented Mar 2, 2026

closed #6036

Description: Fixes JSX preserve mode to only uppercase generated/facade symbols (like CJS interop bindings import_xxx) when used as JSX element names, not user-defined variables. Also adds MustStartWithCapitalLetterForJSX flag propagation from imported symbols to namespace refs, and marks JSX member expression root identifiers for proper handling during scope hoisting.

Copy link
Member Author


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 Mar 2, 2026

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 994d3f8
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69a7ed0b62d5bb000853683b

@IWANABETHATGUY IWANABETHATGUY changed the title fix: 6036 fix: only uppercase facade symbols in JSX preserve mode Mar 2, 2026
@IWANABETHATGUY IWANABETHATGUY force-pushed the 01-29-fix_6036 branch 2 times, most recently from a74cf70 to 75ef6c4 Compare March 2, 2026 10:02
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review March 2, 2026 10:04
@IWANABETHATGUY IWANABETHATGUY marked this pull request as draft March 2, 2026 10:06
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

Fixes JSX preserve mode behavior around scope-hoisted symbol rewriting, aiming to prevent the #6036 panic and to only uppercase generated/facade symbols when needed for JSX element positions.

Changes:

  • Mark JSX element-name root identifiers (including JSX member-expression roots) so later stages can treat them correctly during scope hoisting.
  • Propagate MustStartWithCapitalLetterForJSX from imported symbols to their generated namespace bindings used for rendering.
  • Adjust renaming to uppercase only facade (generated) symbols in JSX preserve mode; update related snapshots.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
crates/rolldown/src/ast_scanner/impl_visit.rs Marks JSX element-name symbols (incl. member-expression roots) for JSX-preserve handling.
crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Propagates JSX-capitalization flag to namespace refs for namespace-alias rendering.
crates/rolldown/src/utils/renamer.rs Uppercases first letter only for flagged facade symbols in JSX preserve mode.
crates/rolldown/tests/rolldown/issues/6036/artifacts.snap Updates expected output for #6036 (notably the generated React namespace binding casing).
crates/rolldown/tests/esbuild/loader/jsx_preserve_capital_letter/artifacts.snap Updates expected JSX preserve snapshot output for the “capital letter” loader case.
crates/rolldown/tests/esbuild/loader/jsx_preserve_capital_letter_minify/artifacts.snap Updates expected JSX preserve + minify snapshot output.
crates/rolldown/tests/esbuild/loader/jsx_preserve_capital_letter_minify/diff.md Updates documented esbuild-vs-rolldown diff for the minify case.
crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap Refreshes filename-hash snapshot entries due to output changes.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

Benchmarks Rust

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.01     78.5±2.11ms        ? ?/sec    1.00     77.5±1.40ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     85.4±2.04ms        ? ?/sec    1.00     85.7±2.43ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    159.0±3.39ms        ? ?/sec    1.01    160.0±4.91ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    176.7±3.49ms        ? ?/sec    1.02    180.8±5.82ms        ? ?/sec
bundle/bundle@threejs                                        1.00     72.4±2.72ms        ? ?/sec    1.00     72.1±2.93ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.02     79.3±2.20ms        ? ?/sec    1.00     77.5±1.14ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    753.3±4.75ms        ? ?/sec    1.01    757.8±7.23ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    864.2±6.72ms        ? ?/sec    1.01    871.6±7.53ms        ? ?/sec
scan/scan@rome_ts                                            1.00     76.1±1.86ms        ? ?/sec    1.00     76.3±1.60ms        ? ?/sec
scan/scan@threejs                                            1.00     27.5±1.67ms        ? ?/sec    1.01     27.6±1.61ms        ? ?/sec
scan/scan@threejs10x                                         1.00    265.6±4.18ms        ? ?/sec    1.00    264.4±2.72ms        ? ?/sec

@IWANABETHATGUY IWANABETHATGUY force-pushed the 01-29-fix_6036 branch 2 times, most recently from 8516306 to 62c8d5f Compare March 2, 2026 12:30
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review March 4, 2026 07:32
Copilot AI review requested due to automatic review settings March 4, 2026 07:32
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 5 out of 5 changed files in this pull request and generated no new comments.

@IWANABETHATGUY IWANABETHATGUY merged commit 8359f9c into main Mar 4, 2026
35 checks passed
@IWANABETHATGUY IWANABETHATGUY deleted the 01-29-fix_6036 branch March 4, 2026 09:29
@github-actions github-actions bot mentioned this pull request Mar 5, 2026
shulaoda added a commit that referenced this pull request Mar 5, 2026
## [1.0.0-rc.7] - 2026-03-05

⚡ Smarter Code Generation Defaults
- DCE-only minification and smart constant inlining are now enabled by default
- Produces cleaner, smaller output bundles without requiring explicit configuration

💡 LLM-Friendly Bundle Analyzer Reports
- New markdown output format for the bundle analyzer plugin with bundle summaries, module graphs, dependency chains, and optimization suggestions
- Optimization suggestions now also recommend using the entriesAware option when common chunks contain modules only reachable from specific entries


### 💥 BREAKING CHANGES

- enable minify: 'dce-only' by default (#8465) by @IWANABETHATGUY
- settings `inlineConst: { mode: 'smart', pass: 1}`  by default (#8444) by @IWANABETHATGUY

### 🚀 Features

- binding: add original getter to BindingMagicString (#8533) by @IWANABETHATGUY
- native-magic-string: add `offset` property support (#8531) by @IWANABETHATGUY
- add `output.strict` option to control `"use strict"` directive emission (#8489) by @Copilot
- watch: expose `watcher.compareContentsForPolling` (#8526) by @hyf0
- watch: use new watcher to support watch mode (#8475) by @hyf0
- rust/watch: handle bulk-change (#8466) by @hyf0
- add LLM-friendly markdown output format to bundle analyzer plugin (#8242) by @IWANABETHATGUY

### 🐛 Bug Fixes

- expose `plugins` on `NormalizedInputOptions` for `buildStart` hook (#8521) by @Copilot
- only uppercase facade symbols in JSX preserve mode (#8519) by @IWANABETHATGUY
- binding: export BindingResult in generated dts header (#8537) by @minsoo-web
- pre-resolve paths option to avoid `invoke_sync` deadlock (#8518) by @IWANABETHATGUY
- remove debug-only jsx_preset and UntranspiledSyntaxError (#8511) by @IWANABETHATGUY
- apply `topLevelVar` to exported `const`/`let` declarations (#8507) by @IWANABETHATGUY
- rolldown_plugin_vite_web_worker_post: avoid replacing `new.target` (#8488) by @sapphi-red
- update copyright year to 2026 (#8486) by @maciekzygmunt

### 🚜 Refactor

- rust: use Oxc's SymbolFlags::ConstVariable instead of custom IsConst flag (#8543) by @Dunqing
- rust: remove FacadeScoping, use Scoping::create_symbol for facade symbols (#8540) by @Dunqing
- rust/watch: remove hacky `reset_closed_for_watch_mode` (#8530) by @hyf0
- binding: return &str instead of String in filename() getter (#8534) by @IWANABETHATGUY
- rust: remove old watch mode implementation (#8525) by @hyf0
- rust/watch: simply watch logic in the binding layer (#8516) by @hyf0
- rust/watch: tweak struct/function names (#8464) by @hyf0

### 📚 Documentation

- explain how external modules work in rolldown (#8457) by @sapphi-red
- add some diagrams using graphviz (#8499) by @sapphi-red
- use `vitepress-plugin-graphviz` (#8498) by @sapphi-red
- list s390x/ppc64le prebuilt binaries (#8495) by @crusty-voidzero
- fix error type for `RolldownBuild.generate` and others (#8490) by @sapphi-red

### ⚡ Performance

- string_wizard: reduce allocations and add ASCII fast paths (#8541) by @IWANABETHATGUY
- use IndexBitSet to replace IndexVec<XXXIdx, bool> for module/stmt inclusion tracking (#8503) by @IWANABETHATGUY
- plugin: use IndexBitSet to optimize skipped plugins checking (#8497) by @ShroXd
- rust/tla: skip compute_tla if there is no module use TLA (#8487) by @ShroXd

### 🧪 Testing

- node/watch: make watch tests run in concurrent and retry-able (#8512) by @hyf0
- add test case for static flag tree-shaking (#8476) by @IWANABETHATGUY
- migrate post-banner sourcemap-with-shebang to Rust (#8477) by @Copilot

### ⚙️ Miscellaneous Tasks

- vscode: `formatOnSave` for markdown files using oxc formatter (#8536) by @minsoo-web
- deps: update test262 submodule for tests (#8528) by @sapphi-red
- remove `retry` workaround from output paths test fixtures (#8520) by @Copilot
- docs: add Shuyuan Wang (h-a-n-a) and remove from acknowledgements (#8509) by @Copilot
- consolidate top_level_var test cases using configVariants (#8508) by @IWANABETHATGUY
- add s390x and ppc64le linux gnu targets (#8493) by @Brooooooklyn

### ◀️ Revert

- fix(rolldown): increase tokio blocking threads size for watch mode (#8517) by @hyf0

### ❤️ New Contributors

* @minsoo-web made their first contribution in [#8536](#8536)
* @crusty-voidzero made their first contribution in [#8495](#8495)
* @maciekzygmunt made their first contribution in [#8486](#8486)

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
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.

[Bug]: Panic with "jsx": "preserve"

3 participants