🦋 Changeset detected

Latest commit: ffd4ad3

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ kdy1
❌ github-actions[bot]
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

Binary Sizes

Commit: 7b0035d

Merging this PR will not alter performance

✅ 184 untouched benchmarks

_{Comparing claude/issue-11505-20260127-0559 (ffd4ad3) with main (d72c9df)}

@claude fix ci

@claude fix ci

PR Review: Make module transforms optional via module feature

Summary

This PR makes swc_ecma_transforms_module an optional dependency of the swc crate, allowing bundlers that handle their own module transforms to reduce binary size.

Code Quality & Best Practices ✅

Strengths:

• Clean feature flag organization with clear documentation in comments
• Proper #[cfg(feature = "module")] gating throughout the codebase
• Stub implementations allow configuration to still be deserialized even without the feature
• Good separation - the bindings and CLI explicitly opt-in to the module feature

Suggestions:

1. Changeset package name: The changeset lists swc_ecma_transforms_module: major, but this change primarily affects the swc crate. Consider adding swc: minor to the changeset since you're adding a new feature, or at minimum document that the major bump is because swc_ecma_transforms_module is now an optional dependency (which may be a breaking change for some consumers).

Potential Issues ⚠️

1. Silent feature degradation: When the module feature is disabled but a user's config specifies module.type: "commonjs" (or similar), the transform silently becomes a noop. Consider adding a warning or error when module config is provided but the feature is disabled:

   #[cfg(not(feature = "module"))]
   impl ModuleConfig {
       pub fn build<'cmt>(...) -> Box<dyn Pass + 'cmt> {
           // Consider logging a warning here
           // tracing::warn!("Module transform config provided but 'module' feature is disabled");
           Box::new(noop_pass())
       }
   }

2. Feature flag in #[cfg(not(feature = "module"))] block at line 712-722: The stub module_pass still calls ModuleConfig::build and references rewrite_import_pass. While this works (since both are noops), it adds unnecessary runtime overhead. Consider simplifying to just:

   #[cfg(not(feature = "module"))]
   let module_pass: Box<dyn Pass> = Box::new((
       Optional::new(helpers::inject_helpers(unresolved_mark), inject_helpers),
   ));

Performance Considerations ✅

• The change achieves its stated goal of reducing binary size for bundlers
• When the feature is enabled, there should be no performance difference from before
• When disabled, noop passes have minimal runtime overhead

Security Concerns ✅

No security issues identified. This is a feature-gating change that doesn't introduce new functionality.

Test Coverage ⚠️

• The code compiles with --no-default-features --features lint,isolated-dts (verified locally)
• The code compiles with default features (verified locally)
• Suggestion: Consider adding a CI job that runs tests with the module feature disabled to catch regressions. This could be a simple cargo check with reduced features.

Minor Observations

1. The let _ = &var; pattern for suppressing unused warnings is functional but could be more idiomatic with #[allow(unused)] on the variables that exist across both feature branches.

2. Good job ensuring BuiltInput struct has the resolver field conditionally compiled.

Overall Assessment

This is a well-implemented feature flag change. The approach of creating stub types that can still deserialize config is good - it means existing configs won't break at parse time. The main improvement would be adding a runtime warning when module config is provided without the feature enabled, so users aren't silently getting unexpected behavior.

Recommendation: Approve with minor suggestions above.