perf(es/compat): Merge export_namespace_from to Transformer

[kdy1]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 99eaf19

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

[claude]

Code Review: PR #11309 - Merge export_namespace_from to Transformer

Overview

This PR migrates the export_namespace_from transformation from swc_ecma_compiler to the new swc_ecma_transformer architecture. The implementation is a clean port that maintains functionality while fitting into the new hook-based system.

✅ Strengths

1. Code Structure: The implementation properly uses the VisitMutHook trait and fits well into the transformer's hook architecture.

2. Performance Optimization: The early exit check (lines 15-29) efficiently skips processing when no namespace exports are present, aligning with SWC's performance-first philosophy.

3. Memory Efficiency: Pre-allocates Vec with proper capacity (items.len() + count) to minimize reallocations (line 31).

4. Utility Function: The normalize_module_export_name function properly handles edge cases like unpaired surrogates with good documentation (lines 6-20 in utils/mod.rs).

5. Code Consistency: The implementation matches the existing swc_ecma_compiler version almost exactly, ensuring behavioral compatibility.

🔍 Issues & Suggestions

1. Missing Unit Tests (Critical per CLAUDE.md rule #5)

The PR adds new functionality but includes no unit tests. According to the repository guidelines:

• Rule Handle early errors in ecmascript parser #5: "Write unit tests for your code"
• The existing swc_ecma_compiler implementation likely has tests that should be ported or referenced

Recommendation: Add unit tests covering:

• Basic namespace export transformation
• Mixed specifiers (namespace + named exports)
• String-based export names (for surrogate handling)
• Edge cases (empty specifiers, type-only exports)

2. Missing Documentation (Per CLAUDE.md rule #7)

The module and struct lack documentation comments explaining:

• What the transformation does
• Example input/output
• Why the transformation is necessary (ES2020 compatibility)

Recommendation: Add module-level and struct documentation:

/// Transforms ES2020 export namespace syntax to compatible imports/exports.
///
/// Converts: `export * as ns from 'module'`
/// Into:     `import * as _ns from 'module'; export { _ns as ns }`
pub fn hook() -> impl VisitMutHook<TraverseCtx> {
    ExportNamespaceFromPass
}

3. Atom Usage Follows Guidelines ✅

Good adherence to CLAUDE.md rule #4:

• Line 8: Uses Cow::Borrowed(name) for Ident case (optimal)
• Line 15: Uses Cow::Owned only when necessary for string conversion

4. Minor: Variable Naming

Line 50: local_bridge could be more descriptive, perhaps namespace_bridge or bridge_ident to clarify its role as a temporary identifier.

🔒 Security & Correctness

No security concerns identified. The code:

• Properly handles untrusted input (module export names)
• Sanitizes unpaired surrogates to prevent potential issues
• Maintains span information for error reporting

📋 Integration Concerns

1. Missing Integration: The new Es2020Options.export_namespace_from field needs to be wired up in the transformer's Options configuration and exposed through the public API.

2. Backwards Compatibility: Verify that existing code using swc_ecma_compiler or swc_ecma_compat_es2020 continues to work during the migration period.

📊 Performance Considerations

The implementation is well-optimized:

• ✅ Early exit for common case (no namespace exports)
• ✅ Single-pass transformation
• ✅ Pre-allocated vectors with accurate capacity
• ✅ Minimal cloning (only src and with when necessary)

Summary

Recommendation: Request Changes

The code quality is good and the implementation is sound, but the PR needs:

1. Unit tests (required per repository guidelines)
2. Documentation (required per repository guidelines)
3. Verification of integration with the transformer options system

Once tests and documentation are added, this will be a solid addition to the codebase.

---

Review generated with attention to CLAUDE.md guidelines

[Copilot]

Pull Request Overview

This PR merges the export_namespace_from transformation into the main Transformer as part of the ES2020 compatibility layer, improving performance by integrating it directly rather than as a separate pass.

Key Changes:

• Introduced a new utility function normalize_module_export_name to handle module export name normalization with WTF-8 support
• Added ES2020 export_namespace_from option and conditional hook implementation
• Implemented transformation of export * as name from 'source' syntax into separate import and re-export statements

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
crates/swc_ecma_transformer/src/utils/mod.rs	New utility module with normalize_module_export_name function for handling module export names with WTF-8 encoding support
crates/swc_ecma_transformer/src/lib.rs	Added utils module declaration
crates/swc_ecma_transformer/src/es2020/mod.rs	Updated ES2020 options to include export_namespace_from flag and conditional hook implementation using OptionalHook
crates/swc_ecma_transformer/src/es2020/export_namespace_from.rs	New implementation of export namespace transformation that converts namespace exports into import/export pairs
crates/swc_ecma_transformer/Cargo.toml	Added swc_atoms dependency for atom manipulation in the new utility function
Cargo.lock	Updated lock file to reflect the new swc_atoms dependency

---

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

[codspeed-hq]

CodSpeed Performance Report

Merging #11309 will not alter performance

_{Comparing kdy1/tr-es2020 (99eaf19) with main (440b391)}

Summary

✅ 138 untouched

[Copilot]

Pull Request Overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.

---

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

[claude]

Pull Request Review: Merge export_namespace_from to Transformer

Summary

This PR migrates the export_namespace_from transformation from the older compiler infrastructure to the new swc_ecma_transformer architecture using the hook system. The implementation is largely a port of existing logic with some structural improvements.

Code Quality ✅

Strengths:

• Clean separation of concerns with the new hook-based architecture
• Proper use of VisitMutHook trait for the transformation
• Consistent error handling with #[cfg(swc_ast_unknown)] for unknown nodes
• Good reuse of utility functions (private_ident! macro, normalize_module_export_name)
• Follows Rust best practices with proper borrowing and ownership

Minor Issues:

1. Performance consideration in capacity calculation (Line 31 in export_namespace_from.rs):

   let mut stmts = Vec::<ModuleItem>::with_capacity(items.len() + count * 2);

   The old implementation at crates/swc_ecma_compiler/src/es2020/export_namespace_from.rs:27 uses items.len() + count, but the new one uses items.len() + count * 2. While this is more accurate (each namespace export generates 2-3 new items), it's worth documenting why this differs or ensuring it's the optimal allocation. This is actually an improvement as it avoids reallocations when there are multiple namespace exports without other specifiers.

Potential Bugs 🔍

No critical bugs found. The logic is a faithful port of the existing implementation with identical transformation behavior:

• Correctly identifies namespace exports with specifiers.iter().any(|s| s.is_namespace())
• Properly handles mixed specifiers (namespace + named exports)
• Correctly clones src and with attributes as needed
• Maintains span information throughout

Performance Considerations ⚡

Positive impacts:

1. Better capacity pre-allocation: The new count * 2 allocation is more accurate and reduces potential reallocations
2. Early return optimization: Returns early when count == 0 (line 27-29)
3. Single iteration pattern: Uses drain() which is efficient for in-place transformation

Adherence to CLAUDE.md #1 (Write performant code):

• ✅ Efficient use of Vec::with_capacity to pre-allocate
• ✅ Uses drain() to avoid unnecessary copies
• ✅ Early exit when no work is needed
• ✅ Avoids string allocations by using Cow<Atom> in normalize_module_export_name

Security Concerns 🔒

No security issues identified. The code:

• Handles untrusted input (module export names) safely
• Uses to_string_lossy() to handle unpaired surrogates correctly (line 15 in utils/mod.rs)
• Doesn't introduce any injection vulnerabilities
• Properly validates node types with exhaustive matching

Test Coverage 📋

Observation: The PR doesn't add new tests, but this is acceptable since:

1. Existing tests in crates/swc_ecma_transforms_proposal/tests/export.rs cover the transformation behavior
2. This is a refactoring/migration PR, not new functionality
3. The existing test suite should validate correctness

Recommendation: After merging, ensure the existing test suite in swc_ecma_transforms_proposal/tests/export.rs (lines 43-54) exercises this new code path. Consider adding integration tests if the new transformer architecture needs separate validation.

Adherence to CLAUDE.md Guidelines ✅

1. ✅ Performance: Code is performant with efficient allocations
2. ✅ Comments: Good documentation in English (see line 10-14 in utils/mod.rs)
3. ✅ No unstable features: Uses stable Rust
4. ✅ Atom usage: Properly uses Cow<Atom> and &str (CLAUDE.md Transformers for ecmascript #4)
5. ⚠️ Unit tests: No new tests added (acceptable for refactoring)
6. ⚠️ Formatting: Cannot verify cargo fmt --all was run (rustfmt not installed in CI environment)
7. ✅ Documentation: Functions and complex logic are documented

Recommendations 📝

1. Minor: Consider adding a doc comment to ExportNamespaceFromPass explaining what transformation it performs (e.g., "Transforms export * as foo from 'bar' into separate import and export declarations")

2. Minor: The normalize_module_export_name function could benefit from an example in its documentation showing the surrogate handling case

3. Verify: Ensure cargo fmt --all is run before final merge (per CLAUDE.md Unignore "431ecef8c85d4d24.js" and "8386fbff927a9e0e.js". #8)

4. Future: Consider adding a benchmark if this is on a hot path, given the PR title emphasizes "perf"

Overall Assessment ✨

This is a well-executed refactoring PR that successfully migrates functionality to the new architecture while maintaining correctness and improving performance slightly. The code quality is high, follows project conventions, and introduces no bugs or security issues.

Recommendation: Approve with minor suggestions ✅

[claude]

Code Review - PR #11309

Summary

This PR migrates the export_namespace_from transform from swc_ecma_compiler to the new swc_ecma_transformer architecture, integrating it as a hook. The implementation is generally solid with good adherence to the existing code patterns.

---

✅ Positive Aspects

1. Clean code migration: The implementation closely mirrors the existing version in swc_ecma_compiler/src/es2020/export_namespace_from.rs, maintaining the same transformation logic.

2. Good architecture alignment: Properly uses the new hook-based architecture with VisitMutHook and OptionalHook.

3. Follows project conventions: Uses Cow<Atom> and &str appropriately as specified in CLAUDE.md Transformers for ecmascript #4.

4. Proper dependency management: Correctly adds swc_atoms dependency to Cargo.toml.

---

🔍 Issues & Recommendations

1. Performance Issue: Capacity Allocation Discrepancy ⚠️

Location: export_namespace_from.rs:31

let mut stmts = Vec::<ModuleItem>::with_capacity(items.len() + count * 2);

Issue: The new implementation allocates count * 2 additional slots, while the original in swc_ecma_compiler only allocates count extra slots:

// Original (line 27 in swc_ecma_compiler)
let mut stmts = Vec::<ModuleItem>::with_capacity(items.len() + count);

Impact: For each export namespace statement, the transform generates:

• 1 import declaration
• 1 export declaration
• Optionally 1 additional export for remaining specifiers

So worst case is count * 3 items, but typically count * 2 is reasonable. However, since the original used count, this should be justified or changed for consistency.

Recommendation: Either:

• Change to items.len() + count to match the original (preferred for consistency)
• Add a comment explaining why count * 2 is better if there's a performance reason

2. Missing Unit Tests ⚠️

Issue: The PR doesn't include any tests for the new implementation. Per CLAUDE.md #5: "Write unit tests for your code."

Current test coverage: Tests exist in crates/swc_ecma_transforms_proposal/tests/export.rs but they test the old implementation via swc_ecma_transforms_compat::es2020::export_namespace_from.

Recommendation: Add tests for the new transformer hook, or ensure existing tests will exercise this code path once it's integrated.

3. Documentation Missing

Issue: The new module and functions lack documentation comments. Per CLAUDE.md #7: "Write documentation for your code."

Recommendation: Add doc comments:

/// Hook for transforming ES2020 export namespace from syntax.
///
/// Transforms `export * as name from 'module'` into:
/// ```js
/// import * as _name from 'module';
/// export { _name as name };
/// ```
pub fn hook() -> impl VisitMutHook<TraverseCtx> {
    ExportNamespaceFromPass
}

/// Pass that transforms export namespace specifiers into import + export pairs.
struct ExportNamespaceFromPass;

4. Minor: Formatting Check

Reminder: Per CLAUDE.md #8, ensure cargo fmt --all has been run before committing. The code appears formatted correctly, but please confirm.

---

🔒 Security Considerations

No security concerns identified. The transform correctly handles:

• Unpaired surrogates in string export names
• Generation of private identifiers to avoid collisions

---

🎯 Performance Considerations

1. Early return optimization ✅: Good early return when count == 0 (line 27-29)
2. Capacity pre-allocation ⚠️: See issue ecmascript parser #1 above regarding capacity calculation
3. Iterator usage ✅: Proper use of drain() to move items without cloning

---

📝 Code Quality

Minor suggestion: Consider adding a constant for the prefix:

const NAMESPACE_BRIDGE_PREFIX: &str = "_";

// Then use:
let local_bridge = private_ident!(format!(
    "{}{}",
    NAMESPACE_BRIDGE_PREFIX,
    normalize_module_export_name(&name)
));

This makes the magic string more maintainable if it needs to change.

---

✅ Action Items

1. Verify or adjust the capacity allocation (count * 2 vs count)
2. Add unit tests or ensure test coverage
3. Add documentation comments
4. Run cargo fmt --all
5. Consider the constant suggestion for the prefix

---

Overall, this is solid work that follows the project's architectural direction. The main concerns are the missing tests and documentation, which are important for maintainability.

[github-actions]

Binary Sizes

File	Size
swc.linux-x64-gnu.node	31M (31954376 bytes)

Commit: c6964e7

[claude]

Code Review for PR #11309: Merge export_namespace_from to Transformer

Overview

This PR migrates the export_namespace_from ES2020 transform from swc_ecma_compiler to the new swc_ecma_transformer crate, which is part of the ongoing performance optimization effort. The change follows the pattern established in the codebase for modularizing transforms.

---

✅ Positive Aspects

1. Code Quality & Style Adherence

   • ✅ Follows CLAUDE.md guidelines: written in English, no nightly features used
   • ✅ Uses Cow<Atom> appropriately in normalize_module_export_name (guideline Transformers for ecmascript #4)
   • ✅ Code is well-structured and matches existing patterns in swc_ecma_compiler

2. Performance Considerations

   • ✅ Pre-allocates vectors with correct capacity: Vec::with_capacity(items.len() + count) (line 30)
   • ✅ Early return optimization when count == 0 (lines 23-28)
   • ✅ Uses drain(..) to avoid unnecessary clones
   • ✅ Efficient filtering to determine if transformation is needed

3. Architecture

   • ✅ Proper integration with the hook system via VisitMutHook
   • ✅ Uses OptionalHook pattern for conditional execution
   • ✅ Clean separation of concerns

---

⚠️ Issues & Concerns

1. Missing Tests (Critical)

Per CLAUDE.md guideline #5: "Write unit tests for your code."

The PR adds new code to swc_ecma_transformer but does not include any tests. While tests exist in crates/swc_ecma_transforms_proposal/tests/export.rs for the old implementation, there are no tests verifying the new implementation works correctly.

Recommendation: Add unit tests in the transformer crate or ensure existing integration tests cover this new code path.

2. Function Naming Inconsistency

• Old implementation: normalize_name() (private function in swc_ecma_compiler)
• New implementation: normalize_module_export_name() (public function in utils)

While the new name is more descriptive, this change should be mentioned in the PR description. The function is now pub(crate), making it available to other modules.

3. Missing Documentation (Per CLAUDE.md #7)

The new module and functions lack documentation comments:

• export_namespace_from.rs module has no doc comment
• hook() function has no doc comment
• ExportNamespaceFromPass struct has no doc comment
• normalize_module_export_name() utility function has no doc comment

Recommendation: Add doc comments explaining:

• What the transform does (converts export * as name from 'module' to import + export)
• When it should be used (ES2020 compatibility)
• The algorithm/approach

4. Code Duplication

The implementation is an almost exact copy from swc_ecma_compiler/src/es2020/export_namespace_from.rs:

• Lines 10-115 in the old file are nearly identical to lines 13-132 in the new file
• Only differences are the struct wrapper and function signature

Question: Is the old implementation in swc_ecma_compiler being removed, or will both coexist? If both remain, this creates maintenance burden.

---

🔍 Code-Specific Observations

export_namespace_from.rs:52

private_ident!(format!(
    "_{}",
    normalize_module_export_name(&name)
))

✅ Good use of private identifier generation for the bridge variable

utils/mod.rs:1-20

The normalize_module_export_name function handles unpaired surrogates correctly with to_string_lossy(). This is good defensive programming.

However, the comment on lines 11-14 could be improved:

// Normally, the export name should be valid UTF-8. But it might also contain
// unpaired surrogates. Node would give an error in this case:
// `SyntaxError: Invalid module export name: contains unpaired
// surrogate`. Here, we temporarily replace the unpaired surrogates
// with U+FFFD REPLACEMENT CHARACTER by using Wtf8::to_string_lossy.

The comment says Node would error, but the code handles it gracefully. Consider clarifying that SWC is more lenient than Node here.

---

🔒 Security

No security concerns identified. The code:

• Doesn't handle user input directly
• Doesn't perform unsafe operations
• Properly handles edge cases (empty vectors, string normalization)

---

🎯 Performance

The PR title mentions this is a performance improvement ("perf(es/compat)"). However:

• No benchmarks or performance measurements are provided
• The implementation is identical to the previous one, so performance characteristics should be similar

Question: What is the expected performance improvement? Is it from reduced pass overhead, better caching, or something else?

---

📋 Checklist Before Merge

• Add unit tests for the new transformer implementation
• Add documentation comments for public/crate-visible items
• Run cargo fmt --all (per CLAUDE.md Unignore "431ecef8c85d4d24.js" and "8386fbff927a9e0e.js". #8) - appears done ✅
• Clarify if old implementation will be removed
• Consider adding a benchmark to quantify performance improvement
• Verify all existing tests still pass with the new implementation

---

📝 Summary

This is a solid refactoring that follows established patterns in the codebase. The main concerns are:

1. Missing test coverage for the new code
2. Missing documentation for maintainability
3. Unclear migration path - is the old code being deprecated?

The code quality is good and follows performance best practices. Once tests and documentation are added, this should be ready to merge.

Recommended Action: Request tests and documentation before merging.

---

Review generated by Claude Code

[claude]

Code Review: PR #11309 - Merge export_namespace_from to Transformer

Overview

This PR refactors the export_namespace_from ES2020 compatibility transform by moving it from the swc_ecma_compiler crate to the new swc_ecma_transformer architecture. This is a performance optimization that consolidates transforms into a unified system.

---

✅ Strengths

1. Performance-focused design: The refactoring aligns with the project's goal of merging transforms to reduce overhead from multiple passes.

2. Clean separation: The transform logic is now isolated in a dedicated module (es2020/export_namespace_from.rs) with a clear hook-based architecture.

3. Memory efficiency: Pre-allocation on line 31:

   let mut stmts = Vec::<ModuleItem>::with_capacity(items.len() + count);

   This is an excellent optimization that avoids reallocations.

4. Proper use of Cow<Atom>: The normalize_module_export_name utility follows the project guidelines by preferring borrowed data (Cow::Borrowed) over owned allocations where possible (lines 6-8 in utils/mod.rs).

5. Edge case handling: Good handling of unpaired surrogates in string export names with proper documentation (lines 10-14 in utils/mod.rs).

---

🔍 Potential Issues & Suggestions

1. Performance: String allocation in identifier generation (Minor)

Location: export_namespace_from.rs:50-53

let local_bridge = private_ident!(format!(
    "_{}",
    normalize_module_export_name(&name)
));

Issue: The format! macro creates a String allocation for every namespace export specifier. According to CLAUDE.md guideline #4: "When creating Atom instances, it's better to use Cow<str> or &str instead of String"

Suggestion: Consider optimizing this to avoid the intermediate String:

let name_str = normalize_module_export_name(&name);
let local_bridge = private_ident!(Cow::Owned(format!("_{}", name_str)));
// Or if private_ident! can accept a closure:
let local_bridge = private_ident!(&format!("_{}", normalize_module_export_name(&name)));

However, this may require changes to the private_ident! macro implementation. If the macro already optimizes this internally, this is not a concern.

2. Code duplication: Multiple clones (Minor)

Location: export_namespace_from.rs:82-84

src: src.clone(),
type_only: false,
with: with.clone(),

Observation: Both src and with are cloned twice in some code paths (when there are both namespace and origin specifiers). This is necessary given the borrow checker constraints, but worth noting for performance-conscious code.

Impact: Likely minimal since Box<Str> clones are cheap (reference counting), but in hot paths with many export statements, this could add up.

3. Test coverage ⚠️

Location: Missing test additions

Issue: The PR modifies the implementation but doesn't add new tests to swc_ecma_transformer. While existing tests in swc_ecma_transforms_proposal/tests/export.rs should still work, guideline #5 states: "Write unit tests for your code."

Recommendation: Consider adding unit tests specifically for the new implementation at crates/swc_ecma_transformer/tests/es2020/export_namespace_from.rs or documenting that existing integration tests provide sufficient coverage.

4. Documentation (Minor)

Location: export_namespace_from.rs

Issue: The module and main transform function lack documentation comments. Guideline #7: "Write documentation for your code."

Suggestion: Add module-level and function-level docs:

//! ES2020 export namespace from transform.
//!
//! Transforms `export * as foo from 'bar'` into:
//! ```js
//! import * as _foo from 'bar';
//! export { _foo as foo };
//! ```

/// Creates a hook that transforms export namespace syntax.
pub fn hook() -> impl VisitMutHook<TraverseCtx> {
    ExportNamespaceFromPass
}

---

🔒 Security Considerations

No security concerns identified. The transform correctly handles:

• Unpaired surrogates in export names (preventing potential injection issues)
• Module specifier cloning (no reference issues)
• Private identifier generation (using the private_ident! macro which should handle conflicts)

---

✅ Best Practices Compliance

• ✅ No unstable Rust features used
• ✅ Follows performance-first approach
• ✅ Uses Cow<Atom> appropriately in utility functions
• ⚠️ Could benefit from additional documentation
• ⚠️ Could benefit from explicit unit tests

---

🎯 Overall Assessment

This is a solid refactoring that achieves its performance optimization goals. The code quality is high, with good attention to memory efficiency and edge cases. The main areas for improvement are:

1. Minor potential optimization in identifier generation
2. Adding documentation comments
3. Clarifying test coverage strategy

Recommendation: ✅ Approve with minor suggestions

The identified issues are minor and don't block merging. The transform logic is sound, and the performance benefits of consolidating into the new transformer architecture are valuable.

---

Review generated by Claude Code following CLAUDE.md guidelines

[Copilot]

Pull Request Overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

---

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

[kdy1]

• perf(es/compat): Merge optional_catch_binding #11313
• perf(es/compat): Merge object_rest_spread #11312
• perf(es/compat): Merge async_to_generator #11311
• perf(es/compat): Merge exponentation_operator #11310
• perf(es/compat): Merge export_namespace_from to Transformer #11309 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Code Review: PR #11309 - Merge export_namespace_from to Transformer

Summary

This PR refactors the export_namespace_from transform by moving it from the compiler-based approach to the new swc_ecma_transformer architecture. This is a good performance-focused change that reduces overhead.

---

✅ Positive Aspects

1. Performance-focused: Moving to the Transformer architecture reduces compilation overhead - aligns with CLAUDE.md guideline ecmascript parser #1
2. Good use of Cow: The normalize_module_export_name function properly uses Cow::Borrowed for the common case (identifiers) and only allocates with Cow::Owned when necessary (string literals with potential encoding issues) - follows CLAUDE.md guideline Transformers for ecmascript #4
3. Proper span preservation: The implementation correctly preserves spans throughout the transformation
4. Handles edge cases: Correctly handles string export names with unpaired surrogates
5. Clean separation: Mixed exports are properly split into separate declarations

---

🔍 Potential Issues & Suggestions

1. Performance: Unnecessary Cow allocation in format! macro

Location: crates/swc_ecma_transformer/src/es2020/export_namespace_from.rs:50-53

let local_bridge = private_ident\!(format\!(
    "_{}",
    normalize_module_export_name(&name)
));

Issue: The normalize_module_export_name returns Cow<Atom>, but using it inside format\! forces an allocation even when it's Cow::Borrowed. This defeats the performance benefit of using Cow.

Suggestion: Consider optimizing this:

let name_str = normalize_module_export_name(&name);
let local_bridge = match name_str {
    Cow::Borrowed(atom) => private_ident\!(format\!("_{}", atom)),
    Cow::Owned(atom) => private_ident\!(format\!("_{}", atom)),
};

Or even better, if private_ident\! can work with string formatting directly, avoid the intermediate Cow allocation.

---

2. Potential inefficiency: Double iteration over specifiers

Location: crates/swc_ecma_transformer/src/es2020/export_namespace_from.rs:15-25

The code first iterates over all items to count namespace exports, then iterates again to transform them. For large modules, this could be optimized.

Suggestion: Consider combining the counting and transformation in a single pass, or use a flag-based approach:

let mut has_namespace_export = false;
// Check only until we find one
for m in items.iter() {
    if matches\!(m, ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(NamedExport {
        specifiers,
        src: Some(..),
        type_only: false,
        ..
    })) if specifiers.iter().any(|s| s.is_namespace())) {
        has_namespace_export = true;
        break;
    }
}

if \!has_namespace_export {
    return;
}

The full counting isn't necessary since we just need to know if any exist.

---

3. Documentation missing

Location: Multiple files

Per CLAUDE.md guideline #7, documentation should be written. Consider adding:

• Module-level documentation for export_namespace_from.rs explaining the transformation
• Function-level documentation for normalize_module_export_name
• Example of the transformation in comments

Example:

/// Transforms export namespace specifiers into import + export statements.
///
/// # Example
/// Input:  `export * as foo from 'bar';`
/// Output: `import * as _foo from 'bar'; export { _foo as foo };`
pub fn hook() -> impl VisitMutHook<TraverseCtx> {
    ExportNamespaceFromPass
}

---

4. Test coverage not visible

Location: N/A

I couldn't locate specific unit tests for this new implementation. Per CLAUDE.md guideline #5, unit tests should be written. While the existing integration tests in swc_ecma_transforms_proposal/tests/export.rs may cover this, consider adding:

• Unit tests specifically for the new ExportNamespaceFromPass
• Tests for edge cases like string export names with special characters
• Tests for mixed export statements

---

5. Minor: Vector capacity optimization

Location: crates/swc_ecma_transformer/src/es2020/export_namespace_from.rs:42-45

let mut origin_specifiers = Vec::new();
let mut import_specifiers = Vec::new();
let mut export_specifiers = Vec::new();

Suggestion: Pre-allocate capacity based on specifiers.len() to avoid reallocations:

let mut origin_specifiers = Vec::with_capacity(specifiers.len());
let mut import_specifiers = Vec::with_capacity(specifiers.len());
let mut export_specifiers = Vec::with_capacity(specifiers.len());

---

🔒 Security

No security concerns identified. The handling of unpaired surrogates in string export names is appropriate.

---

📊 Overall Assessment

This is a solid refactoring that improves performance by moving to the new transformer architecture. The code quality is good, but there are opportunities for optimization (particularly around the Cow usage and double iteration) and documentation improvements.

Recommendation: Approve with minor suggestions for performance optimization and documentation.

---

Action Items

1. Consider optimizing the Cow allocation in the format\! macro
2. Optimize the double iteration to single pass with early exit
3. Add documentation as per CLAUDE.md guidelines
4. Verify test coverage for edge cases
5. Pre-allocate vector capacities where sizes are known

cc @kdy1

[Copilot]

Pull Request Overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.

---

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

[claude]

Code Review: Performance Optimization - Merge export_namespace_from to Transformer

Overview

This PR consolidates the export_namespace_from transformation into the main swc_ecma_transformer package, moving away from the compiler-based approach. This is a positive change that aligns with the project's performance goals.

---

✅ Strengths

1. Performance-focused refactoring: Moving the transformation to the single-pass transformer architecture is aligned with CLAUDE.md guideline ecmascript parser #1 ("Write performant code").

2. Clean architecture: The use of OptionalHook pattern is elegant and allows selective enabling of the transformation.

3. Code compiles successfully: Both swc_ecma_transformer and swc_ecma_preset_env packages build without errors.

4. Formatting compliance: Code passes cargo fmt --all --check.

5. Proper string handling: The normalize_module_export_name function correctly handles edge cases with unpaired surrogates using to_string_lossy(), with clear documentation.

6. Efficient identifier creation: Uses Cow<Atom> for normalize_module_export_name return type (CLAUDE.md guideline Transformers for ecmascript #4), though direct &str could be even better in the Ident case.

---

🔍 Potential Issues & Suggestions

1. Memory Allocation in normalize_module_export_name

Location: crates/swc_ecma_transformer/src/utils/mod.rs:6-15

pub(crate) fn normalize_module_export_name(module_export_name: &ModuleExportName) -> Cow<Atom> {
    match module_export_name {
        ModuleExportName::Ident(Ident { sym: name, .. }) => Cow::Borrowed(name),
        ModuleExportName::Str(Str { value: name, .. }) => {
            Cow::Owned(Atom::from(name.to_string_lossy()))
        }
        // ...
    }
}

Issue: The function returns Cow<Atom>, but per CLAUDE.md #4, &str would be more performant. Consider:

• Returning &str instead, which can be directly passed to private_ident\! macro
• Or if Atom is needed, return Cow<'a, str> to defer Atom creation

Suggestion:

pub(crate) fn normalize_module_export_name(module_export_name: &ModuleExportName) -> Cow<str> {
    match module_export_name {
        ModuleExportName::Ident(Ident { sym: name, .. }) => Cow::Borrowed(name.as_ref()),
        ModuleExportName::Str(Str { value: name, .. }) => name.to_string_lossy(),
        // ...
    }
}

2. Double Iteration in transform_export_namespace_from

Location: crates/swc_ecma_transformer/src/es2020/export_namespace_from.rs:14-25

The method iterates through items twice:

1. First to count namespace exports (lines 15-25)
2. Then to process them (lines 33-118)

Issue: This is inefficient for large module item lists.

Suggestion: Combine into a single pass:

fn transform_export_namespace_from(&mut self, items: &mut Vec<ModuleItem>) {
    let mut has_namespace_export = false;
    
    // Quick scan - stop early if found
    for item in items.iter() {
        if matches\!(item, ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(NamedExport {
                specifiers,
                src: Some(..),
                type_only: false,
                ..
            })) if specifiers.iter().any(|s| s.is_namespace()))
        {
            has_namespace_export = true;
            break;
        }
    }

    if \!has_namespace_export {
        return;
    }

    // ... rest of transformation
}

Or even better, skip the count entirely and allocate conservatively:

// Allocate space for worst case (3 items per namespace export)
let mut stmts = Vec::with_capacity(items.len() * 2);

3. Missing Unit Tests

Location: crates/swc_ecma_transformer/

Issue: The new export_namespace_from.rs module has no accompanying tests. CLAUDE.md guideline #5 requires unit tests.

Suggestion: Add tests under crates/swc_ecma_transformer/tests/ or inline tests that verify:

• Basic namespace export transformation
• Mixed namespace and named exports
• String-based export names
• Edge cases with special characters

4. Missing Documentation

Location: crates/swc_ecma_transformer/src/es2020/export_namespace_from.rs

Issue: The public hook() function and ExportNamespaceFromPass struct lack doc comments (CLAUDE.md guideline #7).

Suggestion: Add documentation:

/// Returns a hook that transforms ES2020 export namespace from syntax.
///
/// This transforms `export * as ns from 'mod'` into:
/// ```js
/// import * as _ns from 'mod';
/// export { _ns as ns };
/// ```
pub fn hook() -> impl VisitMutHook<TraverseCtx> {
    ExportNamespaceFromPass
}

/// Visitor that transforms export namespace from declarations
/// by splitting them into separate import and export statements.
struct ExportNamespaceFromPass;

5. Potential Code Reuse Opportunity

Location: crates/swc_ecma_transformer/src/utils/mod.rs

The normalize_module_export_name function might already exist elsewhere in the codebase. Consider checking swc_ecma_utils to avoid duplication.

---

🔒 Security & Correctness

✅ No security concerns identified
✅ Proper handling of unpaired surrogates in string export names
✅ Correct transformation logic that preserves semantics

---

📊 Test Coverage

⚠️ No tests added: The PR modifies the transformation logic but doesn't include tests for the new code path. Existing tests in other packages may cover this indirectly through integration tests, but direct unit tests would be valuable.

Recommendation: Before merging, verify that existing preset_env tests adequately cover the export_namespace_from transformation path, or add new tests.

---

📝 Summary

This is a well-structured performance optimization that successfully consolidates the export namespace transformation. The main concerns are:

1. Performance: Minor optimization opportunities in double-iteration and string handling
2. Testing: Missing unit tests for the new code
3. Documentation: Missing doc comments

Recommendation: The PR is solid and can be merged after:

• Adding unit tests (required per CLAUDE.md)
• Adding documentation (required per CLAUDE.md)
• Consider the performance optimizations (optional but aligned with project goals)

Great work on the refactoring! 👍

[claude]

Pull Request Review: Merge export_namespace_from to Transformer

This PR migrates the export_namespace_from ES2020 transformation from the swc_ecma_compiler crate to the new swc_ecma_transformer crate, as part of the ongoing consolidation effort. Overall, the implementation looks solid and maintains functional equivalence with the original code.

✅ Strengths

1. Code Consolidation: Successfully moves the transform into the unified transformer architecture, which should improve maintainability and reduce duplication.

2. Functional Correctness: The core transformation logic is identical to the original implementation in swc_ecma_compiler, maintaining correctness:

   • Properly transforms export * as name from 'module' into separate import and export statements
   • Correctly handles mixed specifiers (namespace + named exports)
   • Preserves with clauses appropriately

3. Performance Optimizations:

   • Early return when no namespace exports are found (line 27-29 in export_namespace_from.rs)
   • Pre-allocates vector capacity based on expected size (line 31)
   • Uses Cow<Atom> in the normalize function to avoid unnecessary allocations (line 6 in utils/mod.rs)

4. Integration: Clean integration into the preset_env pipeline with proper placement before ES2015 transforms (line 215 in preset_env/lib.rs)

🔍 Issues & Concerns

1. Performance Regression - Double Iteration (Medium Priority)

In export_namespace_from.rs:14-25, the code iterates through all module items twice:

• First to count items needing transformation
• Then again during the actual transformation (line 33)

Original code location: crates/swc_ecma_transformer/src/es2020/export_namespace_from.rs:14-25

let count = items
    .iter()
    .filter(|m| {
        matches\!(m, ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(NamedExport {
                specifiers,
                src: Some(..),
                type_only: false,
                ..
            })) if specifiers.iter().any(|s| s.is_namespace()))
    })
    .count();

Recommendation: Since the capacity calculation is only an optimization, consider either:

• Using items.len() * 2 as a simple heuristic (pessimistic but safe)
• Collecting matched items in the first pass to avoid re-checking
• Skip the count and use Vec::with_capacity(items.len()) with a comment

This is less critical if modules rarely have many items, but aligns better with the PR's performance goals (note the perf prefix in the PR title).

2. Potential Naming Collision (Low-Medium Priority)

The normalize_module_export_name function (utils/mod.rs:6) and the bridge identifier generation (export_namespace_from.rs:50-53) could potentially create collisions if:

• A module already has a variable named _foo
• An export namespace is named foo

The private_ident\! macro should handle this, but it's worth verifying it generates a unique identifier with proper hygiene. The original compiler code has the same pattern, so this is not a regression, but worth documenting or adding a test case.

3. Unrelated Test Output Change (Low Priority)

The change in corejs3/usage-regexp/output.mjs:6 from new RegExp to RegExp appears unrelated to this PR:

-var c = new RegExp(".", "imsuy");
+var c = RegExp(".", "imsuy");

Question: Is this an intentional fix/optimization from moving the transformation order, or an unintended side effect? If intentional, it might warrant a note in the PR description.

4. Missing Unit Tests (Medium Priority)

Following CLAUDE.md guideline #5 ("Write unit tests for your code"), I don't see dedicated unit tests for the new implementation in swc_ecma_transformer. The transformation is tested indirectly through preset_env tests, but consider adding:

• Direct tests in swc_ecma_transformer for the hook
• Edge cases: empty specifiers, mixed specifiers, string-based export names
• Performance: modules with many namespace exports

📝 Code Quality Notes

1. Good adherence to CLAUDE.md:

   • ✅ Uses Cow<str> appropriately (guideline Transformers for ecmascript #4)
   • ✅ Proper English comments (guideline Clean up #2)
   • ✅ No unstable features (guideline EcmaScript lexer is inefficient #3)
   • ⚠️ Missing unit tests (guideline Handle early errors in ecmascript parser #5)
   • ✅ Existing tests preserved (guideline Internal documentations #6)

2. Documentation: The normalize_module_export_name function has excellent inline documentation explaining the unpaired surrogate edge case.

3. Error Handling: Proper use of #[cfg(swc_ast_unknown)] for unknown AST node variants.

🔐 Security

No security concerns identified. The transformation is purely syntactic and doesn't introduce injection vulnerabilities.

🎯 Recommendations Summary

Must Address Before Merge:

• None (the code is functionally correct)

Should Consider:

1. Optimize the double iteration for better performance alignment with PR goals
2. Add unit tests for the new transformer implementation
3. Clarify the RegExp test output change

Nice to Have:

• Document identifier collision handling or add test coverage

Conclusion

This is a well-executed refactoring that successfully moves functionality to the unified transformer architecture. The code quality is good and maintains functional correctness. The main opportunity for improvement is optimizing the double iteration to better align with the performance goals indicated by the PR title prefix.

Recommendation: ✅ Approve with suggestions - The code is ready to merge, but addressing the double iteration would strengthen the performance claims.