Skip to content

fix: no mangle for enum member value#11629

Merged
ahabhgk merged 2 commits intomainfrom
fix-mangle-enum
Sep 10, 2025
Merged

fix: no mangle for enum member value#11629
ahabhgk merged 2 commits intomainfrom
fix-mangle-enum

Conversation

@ahabhgk
Copy link
Contributor

@ahabhgk ahabhgk commented Sep 10, 2025

Summary

Because we don't control the codegen of enum value (just use the loader generated), so we can't mangle it.

var enum_E = /*#__PURE__*/ function(E) {
    E[E["Dynamic"] = Math.random()] = "Dynamic"; // we don't control the codegen here, it's generated by loader
    return E;
}({});

enum_E.Dynamic // should always use Dynamic here, we can't mangle it to something shorter

Related links

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings September 10, 2025 07:12
@netlify
Copy link

netlify bot commented Sep 10, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 1f9d61a
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/68c126438104ed0008a5c578

@github-actions github-actions bot added team The issue/pr is created by the member of Rspack. release: bug fix release: bug related release(mr only) labels Sep 10, 2025
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 an issue where enum values with dynamic expressions were being incorrectly mangled during code optimization. The change prevents mangling of enum values since their codegen is controlled by the loader rather than the bundler itself.

  • Prevents mangling of enum values by setting can_mangle: Some(false) for enum exports
  • Adds test case to verify that dynamic enum values (with expressions like Math.random()) and static enum values are handled correctly during inlining

Reviewed Changes

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

File Description
crates/rspack_plugin_javascript/src/dependency/esm/esm_export_specifier_dependency.rs Sets can_mangle: Some(false) for enum export specifications to prevent mangling
tests/rspack-test/configCases/inline-enum/enum-expression/rspack.config.js Test configuration enabling TypeScript parsing and enum inlining
tests/rspack-test/configCases/inline-enum/enum-expression/index.js Test case verifying enum inlining behavior for both dynamic and static values
tests/rspack-test/configCases/inline-enum/enum-expression/enum.ts TypeScript enum definition with both dynamic and static values for testing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ahabhgk ahabhgk requested a review from chenjiahan September 10, 2025 07:13
@ahabhgk ahabhgk changed the title fix: no mangle for enum value fix: no mangle for enum member value Sep 10, 2025
@ahabhgk ahabhgk enabled auto-merge (squash) September 10, 2025 07:18
@github-actions
Copy link
Contributor

📦 Binary Size-limit

Comparing 1f9d61a to chore: enable canary nightly release (#11627) by pshu

🙈 Size remains the same at 47.23MB

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 10, 2025

CodSpeed Performance Report

Merging #11629 will not alter performance

Comparing fix-mangle-enum (1f9d61a) with main (e6c3a24)

Summary

✅ 17 untouched benchmarks

@ahabhgk ahabhgk merged commit 23ec7ac into main Sep 10, 2025
70 of 72 checks passed
@ahabhgk ahabhgk deleted the fix-mangle-enum branch September 10, 2025 07:57
@chenjiahan
Copy link
Member

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants