Skip to content

Conversation

Zeahan
Copy link
Contributor

@Zeahan Zeahan commented Oct 17, 2025

…me in swc options (#11903)

Summary

In the previous implementation, the remaining fields of the Options struct (swc_core::base::config::Options) were initialized using ..Default::default(). However, this approach can result in some fields having default values that do not match the expected behavior. The root cause is that the fields in Options are defined with serde macros, and custom logic is used to generate default values during deserialization.

pub(crate) fn default_env_name() -> String {
    if let Ok(v) = env::var("SWC_ENV") {
        return v;
    }

    match env::var("NODE_ENV") {
        Ok(v) => v,
        Err(_) => "development".into(),
    }
}

pub struct Options {
    #[serde(default = "default_env_name")]
    pub env_name: String,

    #[serde(default = "default_swcrc")]
    pub swcrc: bool,

    #[cfg(not(all(target_arch = "wasm32", not(target_os = "wasi"))))]
    #[serde(default = "default_cwd")]
    pub cwd: PathBuf,
    // ...
}

When initializing Options with ..Default::default(), this custom logic is bypassed, leading to discrepancies between the generated fields and the original behavior in swc-loader. This inconsistency may cause certain swc plugins (such as @swc/plugin-emotion) to behave unexpectedly.

To address this issue and ensure consistency with the original swc implementation, the initialization of the remaining fields in Options has been changed to use deserialization from an empty JSON object via serde_json (which is the approach used in the swc source code). This ensures that the configuration is generated in a manner consistent with the original swc behavior.

Related links

#11903

Checklist

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

Copy link

netlify bot commented Oct 17, 2025

Deploy Preview for rspack ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit cbe85dc
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/68f75ea01687380008f1a6c9
😎 Deploy Preview https://deploy-preview-11906--rspack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@CLAassistant
Copy link

CLAassistant commented Oct 17, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the release: bug fix release: bug related release(mr only) label Oct 17, 2025
@CPunisher
Copy link
Contributor

Could you please add a test case?

@CPunisher CPunisher self-requested a review October 17, 2025 08:54
@Zeahan
Copy link
Contributor Author

Zeahan commented Oct 17, 2025

Could you please add a test case?

Thanks for your reply, a test case will be provided later.

@Zeahan
Copy link
Contributor Author

Zeahan commented Oct 17, 2025

Could you please add a test case?

@CPunisher rust test cases have been provided.
Since the complete rspack build process requires an additional custom SWC plugin for testing, Node.js test cases are not implemented, sorry for that.

Copy link

codspeed-hq bot commented Oct 17, 2025

CodSpeed Performance Report

Merging #11906 will not alter performance

Comparing Zeahan:fix-swc-env-name (cbe85dc) with main (a457128)

Summary

✅ 17 untouched

@Zeahan Zeahan force-pushed the fix-swc-env-name branch 3 times, most recently from a8307e9 to cf1a735 Compare October 17, 2025 14:48
@Zeahan
Copy link
Contributor Author

Zeahan commented Oct 21, 2025

@CPunisher Hi, I’ve tried running CI multiple times in my forked repo after syncing my PR branch with the main repo, but I noticed that the test Serial.test.js always fails.
截屏2025-10-21 17 58 40
Then, I tried running CI based on the latest version of the main branch (a457128) and encountered similar test errors.

I realized that these errors might be caused by another PR rather than mine, so I rolled back the base of my PR branch to 0f38cf19, and the CI finally passed in my forked repo.
截屏2025-10-21 18 15 29
I’ve now force-pushed the updated PR branch. Would you mind approving the CI workflows again?

@CPunisher
Copy link
Contributor

ild process requires an additional custom SWC plugin for testing, Node.js test cases are not implemented, sorry for that.

😂 Sorry, recently our ci is not stable.

Copy link
Contributor

github-actions bot commented Oct 22, 2025

📝 Benchmark detail: Open

Name Base (2025-10-22 0eb1382) Current Change
10000_big_production-mode_disable-minimize + exec 56.9 s ± 1.21 s 27.9 s ± 1.14 s -51.03 %
10000_development-mode + exec 1.32 s ± 26 ms 1.61 s ± 32 ms +21.59 %
10000_development-mode_hmr + exec 650 ms ± 5 ms 649 ms ± 16 ms -0.13 %
10000_development-mode_noop-loader + exec 2.28 s ± 108 ms 2.56 s ± 64 ms +12.32 %
10000_production-mode + exec 30.2 s ± 699 ms 1.6 s ± 53 ms -94.70 %
10000_production-mode_persistent-cold + exec 30.4 s ± 659 ms 1.8 s ± 17 ms -94.09 %
10000_production-mode_persistent-hot + exec 30.2 s ± 608 ms 1.34 s ± 41 ms -95.57 %
arco-pro_development-mode + exec 1.6 s ± 78 ms 1.67 s ± 184 ms +4.32 %
arco-pro_development-mode_hmr + exec 359 ms ± 1.3 ms 367 ms ± 1.4 ms +2.10 %
arco-pro_production-mode + exec 3.05 s ± 72 ms 3.04 s ± 143 ms -0.37 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.11 s ± 87 ms 3.06 s ± 209 ms -1.88 %
arco-pro_production-mode_persistent-cold + exec 3.13 s ± 108 ms 3.12 s ± 57 ms -0.46 %
arco-pro_production-mode_persistent-hot + exec 1.77 s ± 62 ms 1.82 s ± 143 ms +2.36 %
arco-pro_production-mode_traverse-chunk-modules + exec 3.08 s ± 108 ms 3.03 s ± 111 ms -1.67 %
large-dyn-imports_development-mode + exec 1.59 s ± 49 ms 1.9 s ± 57 ms +19.92 %
large-dyn-imports_production-mode + exec 5.74 s ± 56 ms 1.86 s ± 106 ms -67.68 %
threejs_development-mode_10x + exec 1.32 s ± 13 ms 1.43 s ± 24 ms +8.41 %
threejs_development-mode_10x_hmr + exec 908 ms ± 21 ms 904 ms ± 23 ms -0.42 %
threejs_production-mode_10x + exec 4.07 s ± 19 ms 4.23 s ± 235 ms +4.16 %
threejs_production-mode_10x_persistent-cold + exec 4.2 s ± 28 ms 4.32 s ± 50 ms +2.73 %
threejs_production-mode_10x_persistent-hot + exec 3.74 s ± 203 ms 3.84 s ± 199 ms +2.45 %
10000_big_production-mode_disable-minimize + rss memory 8740 MiB ± 376 MiB 8479 MiB ± 250 MiB -2.99 %
10000_development-mode + rss memory 646 MiB ± 15.5 MiB 762 MiB ± 26.2 MiB +17.98 %
10000_development-mode_hmr + rss memory 815 MiB ± 19.3 MiB 930 MiB ± 49.2 MiB +14.17 %
10000_development-mode_noop-loader + rss memory 963 MiB ± 22.8 MiB 1068 MiB ± 27.2 MiB +10.93 %
10000_production-mode + rss memory 658 MiB ± 19.9 MiB 697 MiB ± 42.2 MiB +5.92 %
10000_production-mode_persistent-cold + rss memory 759 MiB ± 52 MiB 866 MiB ± 27.9 MiB +14.09 %
10000_production-mode_persistent-hot + rss memory 750 MiB ± 42.1 MiB 867 MiB ± 40.9 MiB +15.61 %
arco-pro_development-mode + rss memory 555 MiB ± 42.9 MiB 576 MiB ± 56.7 MiB +3.84 %
arco-pro_development-mode_hmr + rss memory 433 MiB ± 17.1 MiB 475 MiB ± 14.1 MiB +9.64 %
arco-pro_production-mode + rss memory 669 MiB ± 17.9 MiB 671 MiB ± 92.3 MiB +0.42 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 659 MiB ± 81.5 MiB 675 MiB ± 32.1 MiB +2.45 %
arco-pro_production-mode_persistent-cold + rss memory 735 MiB ± 35.4 MiB 780 MiB ± 25.6 MiB +6.07 %
arco-pro_production-mode_persistent-hot + rss memory 583 MiB ± 77 MiB 598 MiB ± 70.6 MiB +2.58 %
arco-pro_production-mode_traverse-chunk-modules + rss memory 669 MiB ± 79.4 MiB 690 MiB ± 37.6 MiB +3.15 %
large-dyn-imports_development-mode + rss memory 695 MiB ± 14.1 MiB 788 MiB ± 17.4 MiB +13.30 %
large-dyn-imports_production-mode + rss memory 632 MiB ± 3.37 MiB 627 MiB ± 5.47 MiB -0.72 %
threejs_development-mode_10x + rss memory 566 MiB ± 20.9 MiB 597 MiB ± 8.49 MiB +5.51 %
threejs_development-mode_10x_hmr + rss memory 804 MiB ± 9.92 MiB 859 MiB ± 28.5 MiB +6.88 %
threejs_production-mode_10x + rss memory 778 MiB ± 223 MiB 792 MiB ± 178 MiB +1.80 %
threejs_production-mode_10x_persistent-cold + rss memory 839 MiB ± 43.3 MiB 832 MiB ± 39 MiB -0.88 %
threejs_production-mode_10x_persistent-hot + rss memory 699 MiB ± 24.4 MiB 732 MiB ± 25.5 MiB +4.78 %

Threshold exceeded: ["10000_development-mode + exec","10000_development-mode_noop-loader + exec","large-dyn-imports_development-mode + exec","threejs_development-mode_10x + exec"]

@Zeahan
Copy link
Contributor Author

Zeahan commented Oct 22, 2025

@CPunisher Adding default_env_name is enough to solve this issue—that was my first thought as well. However, with this approach, some default field values(such as cwd and swcrc) would still be different from the original SWC behavior.

So in the end, I decided to copy this part of the code directly from the SWC repo 😂(https://github.com/swc-project/swc/blob/4848301edca71ed84d147e90baccae217b0a8938/bindings/binding_core_node/src/bundle.rs#L208), hoping to offer a more complete solution rather than just fixing the env_name problem. But of course, I’m happy to go with whichever approach your team prefers! 😁

@CPunisher
Copy link
Contributor

Yes, I have realized your consideration. I will merge this pr after we stablize CI.

Thanks for your contribution!

@CPunisher CPunisher merged commit 3cb5cfc into web-infra-dev:main Oct 22, 2025
68 of 70 checks passed
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)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants