Skip to content

Conversation

@delino
Copy link
Contributor

@delino delino bot commented Nov 3, 2025

Description

Fixes #11143

This PR fixes a bug where rewriteRelativeImportExtensions: true doesn't work correctly when TypeScript path aliases (jsc.paths) are configured.

Problem

When both settings were enabled:

  • rewriteRelativeImportExtensions: true
  • jsc.paths with path aliases (e.g., "@/*": [".//*"])

The extension rewriting would fail for aliased imports:

// Input
export { a } from "@/util/index.ts";

// Previous output (broken)
export { a } from "./util/index.ts";  // Extension NOT rewritten

// Fixed output
export { a } from "./util/index.js";  // Extension correctly rewritten

Root Cause

The transformation passes were executing in the wrong order:

  1. typescript_import_rewriter() ran first, checking if imports start with ./ or ../
  2. Since @/util/index.ts doesn't match that pattern, it skipped extension rewriting
  3. import_rewriter() ran second and resolved the alias: @/util/index.ts./util/index.ts
  4. But the extension rewriter had already run, so .ts was never rewritten to .js

Solution

Integrated extension rewriting directly into NodeImportResolver:

  1. Added rewrite_import_extensions field to the resolver's Config struct
  2. Implemented extension rewriting logic that runs AFTER path resolution
  3. Updated the pass logic to skip the separate typescript_import_rewriter when using a resolver with extension rewriting enabled

This ensures extensions are rewritten after path aliases are resolved, fixing the issue.

Changes

  • crates/swc_ecma_transforms_module/src/path.rs:

    • Added rewrite_import_extensions field to Config
    • Implemented rewrite_extension() helper method
    • Applied extension rewriting in to_specifier() after path resolution
  • crates/swc/src/config/mod.rs:

    • Updated build_resolver() to accept rewrite_import_extensions parameter
    • Updated get_resolver() to pass through the flag
    • Modified cache key to include the new flag
    • Updated pass creation logic to conditionally use the separate rewriter

Tests

Added comprehensive test in crates/swc/tests/fixture/issues-11xxx/11143/ covering:

  • ✅ Path aliases with .ts.js
  • ✅ Path aliases with .tsx.jsx
  • ✅ Path aliases with .mts.mjs
  • ✅ Path aliases with .cts.cjs
  • ✅ Regular relative imports (regression test)
  • ✅ Export all statements
  • ✅ Dynamic imports

Related

Fixes #11143

## Problem
When both `rewriteRelativeImportExtensions: true` and `jsc.paths` were configured,
the extension rewriting would not work for imports using path aliases.

For example:
- Input: `export { a } from "@/util/index.ts";`
- Previous output: `export { a } from "./util/index.ts";` (extension NOT rewritten)
- Fixed output: `export { a } from "./util/index.js";` (extension correctly rewritten)

## Root Cause
The transformation passes were executing in the wrong order:
1. `typescript_import_rewriter()` ran first, checking for relative paths (`./` or `../`)
2. Since `@/util/index.ts` doesn't start with `./` or `../`, it skipped rewriting
3. `import_rewriter()` ran second and resolved `@/util/index.ts` → `./util/index.ts`
4. But the extension rewriter had already run, so `.ts` was never rewritten to `.js`

## Solution
Integrated extension rewriting directly into `NodeImportResolver`:

1. Added `rewrite_import_extensions: bool` field to `Config` in `path.rs`
2. Implemented `rewrite_extension()` method that applies the same logic as TypeScript's
   `rewriteRelativeImportExtensions` after path resolution
3. Updated the main config to pass this flag through to the resolver
4. Modified the pass logic to skip the separate `typescript_import_rewriter` pass
   when using a resolver with extension rewriting enabled

This ensures extension rewriting happens AFTER path alias resolution, fixing the issue.

## Changes
- `crates/swc_ecma_transforms_module/src/path.rs`:
  - Added `rewrite_import_extensions` field to `Config` struct
  - Implemented `rewrite_extension()` helper method
  - Applied extension rewriting in `to_specifier()` after path resolution

- `crates/swc/src/config/mod.rs`:
  - Updated `build_resolver()` to accept `rewrite_import_extensions` parameter
  - Updated `get_resolver()` to accept and pass through the flag
  - Modified resolver cache key to include `rewrite_import_extensions`
  - Updated pass logic to conditionally use separate rewriter only when needed

## Tests
Added comprehensive test in `crates/swc/tests/fixture/issues-11xxx/11143/` covering:
- Path aliases with `.ts` → `.js`
- Path aliases with `.tsx` → `.jsx`
- Path aliases with `.mts` → `.mjs`
- Path aliases with `.cts` → `.cjs`
- Regular relative imports (regression test)
- Export all statements
- Dynamic imports

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@changeset-bot
Copy link

changeset-bot bot commented Nov 3, 2025

⚠️ No Changeset found

Latest commit: 72aaad5

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

Copy link
Member

kdy1 commented Nov 3, 2025

🤖 This pull request has been linked to DevBird Task #1189

View the task details and manage the automated development workflow in DevBird.

Learn more about DevBird here or the announcement blog post here.

Copy link
Member

kdy1 commented Nov 3, 2025

📋 DevBird Task Prompt

Fix the bug where rewriteRelativeImportExtensions: true doesn't work correctly when TypeScript path aliases are used.

Problem Description

When SWC is configured with both:

  • rewriteRelativeImportExtensions: true (to rewrite .ts/.tsx/.mts/.cts extensions to .js/.jsx/.mjs/.cjs)
  • jsc.paths with path aliases (e.g., "@/*": [".//*"])

The extension rewriting fails for aliased imports. For example:

  • Input: export { a } from "@/util/index.ts";
  • Current output: export { a } from "./util/index.ts"; (extension NOT rewritten)
  • Expected output: export { a } from "./util/index.js"; (extension rewritten)

Regular relative imports like "./util/index.ts" work correctly and get rewritten to "./util/index.js".

Root Cause Analysis

The issue occurs due to the order of transformation passes in /home/runner/work/swc/swc/crates/swc/src/config/mod.rs:

  1. Line 1479-1480: typescript_import_rewriter() is created first
  2. Line 1486: import_rewriter() is created second for path alias resolution
  3. Line 1524: They are chained as (rewrite_relative_import_pass, transform_pass)

The execution order is:

  1. typescript_import_rewriter() runs FIRST, sees @/util/index.ts
  2. The rewriter checks if the path starts with ./ or ../ (see /home/runner/work/swc/swc/crates/swc_ecma_transforms_module/src/rewriter/import_rewriter_typescript.rs:81-87)
  3. Since @/util/index.ts doesn't start with ./ or ../, it skips extension rewriting
  4. import_rewriter() runs SECOND and resolves @/util/index.ts./util/index.ts
  5. But the extension rewriter has already run, so .ts never gets rewritten to .js

Technical Context

Key files:

  • /home/runner/work/swc/swc/crates/swc_ecma_transforms_module/src/rewriter/import_rewriter_typescript.rs - TypeScript extension rewriter implementation
  • /home/runner/work/swc/swc/crates/swc_ecma_loader/src/resolvers/tsc.rs - TypeScript path alias resolver
  • /home/runner/work/swc/swc/crates/swc_ecma_transforms_module/src/path.rs - Node import resolver that calls path alias resolver
  • /home/runner/work/swc/swc/crates/swc/src/config/mod.rs - Main config that chains the passes together

Documentation References

Solution Approach

There are two potential solutions:

Option 1 (Recommended): Integrate extension rewriting into import_rewriter

Modify the NodeImportResolver in /home/runner/work/swc/swc/crates/swc_ecma_transforms_module/src/path.rs to:

  1. Add a configuration flag to enable extension rewriting during path resolution
  2. After resolving a path alias to a relative path, apply the extension rewriting logic
  3. This ensures extension rewriting happens AFTER path alias resolution

Benefits:

  • Single pass through the AST (better performance)
  • Extensions are rewritten immediately after resolution
  • More maintainable - one place handles both concerns

Option 2 (Alternative): Reorder the passes

Modify /home/runner/work/swc/swc/crates/swc/src/config/mod.rs to:

  1. Apply path resolution first
  2. Then apply extension rewriting as a second pass
  3. This requires making the extension rewriter work on already-resolved paths

Benefits:

  • Minimal changes to existing code structure
  • Clear separation of concerns

Drawbacks:

  • Requires two passes through the AST
  • More complex pass coordination

Implementation Steps

For Option 1 (Recommended):

  1. Update NodeImportResolver configuration:

    • Add a rewrite_extensions: bool field to the Config struct in path.rs:100
    • Pass this configuration through from the main SWC config
  2. Integrate extension rewriting logic:

    • In the to_specifier method (path.rs:145), after converting the resolved path to a specifier
    • Apply the extension rewriting logic from get_output_extension() if rewrite_extensions is enabled
    • You can reuse the get_output_extension() function from import_rewriter_typescript.rs or create a shared utility
  3. Update the main config:

    • In /home/runner/work/swc/swc/crates/swc/src/config/mod.rs, pass the rewrite_relative_import_extensions flag to the resolver configuration
    • This happens in the build_resolver function calls (lines 1548-1576)
  4. Update import_rewriter:

    • The import_rewriter in /home/runner/work/swc/swc/crates/swc_ecma_transforms_module/src/rewriter/mod.rs needs to be updated to use the new resolver configuration
  5. Handle both passes correctly:

    • When both path resolution AND extension rewriting are enabled, we don't need the separate typescript_import_rewriter() pass
    • Update the logic at line 1479-1524 in config/mod.rs to only create the typescript_import_rewriter pass when path resolution is NOT being used

Testing Requirements

Create comprehensive tests covering:

  1. Path alias with extension rewriting:

    • Input: import { a } from "@/util/index.ts"
    • With config: paths: {"@/*": [".//*"]} and rewriteRelativeImportExtensions: true
    • Expected: import { a } from "./util/index.js"
  2. Various extension types:

    • .ts.js
    • .tsx.jsx
    • .mts.mjs
    • .cts.cjs
    • .d.ts should NOT be rewritten
    • .json should remain .json
  3. Different import types:

    • Static imports: import { a } from "@/util/index.ts"
    • Re-exports: export { a } from "@/util/index.ts"
    • Export all: export * from "@/util/index.ts"
    • Dynamic imports: import("@/util/index.ts")
  4. Without path aliases (regression test):

    • Ensure relative imports still work: import { a } from "./util/index.ts"import { a } from "./util/index.js"
  5. Multiple path alias patterns:

    • Wildcard patterns: "@/*": ["src/*"]
    • Exact mappings: "@utils": ["src/utils"]
    • Multiple destinations: "@/*": ["src/*", "lib/*"]
  6. Edge cases:

    • Node modules should not be affected
    • Absolute paths should work correctly
    • Paths without extensions should remain unchanged

Success Criteria

Important Notes from CLAUDE.md

  1. Write performant code - prefer performance over other things
  2. Use &str or Cow<str> instead of String when creating Atom instances (prefer &str over Cow<str>)
  3. Write unit tests for your code
  4. Do not remove or modify existing tests
  5. Write documentation for your code
  6. Run cargo fmt --all before committing

Issue Reference

Branch Name

Use branch name: devbird/fix-extension-rewrite-path-aliases


This comment was automatically added by DevBird. You can disable this feature in DevBird Settings.

@claude
Copy link
Contributor

claude bot commented Nov 3, 2025

Claude finished @DeLino[bot]'s task —— View job


Code Review Complete

I've conducted a thorough review of PR #11219 that fixes the rewriteRelativeImportExtensions issue with TypeScript path aliases. Here's my comprehensive feedback:

Overall Assessment: Excellent Implementation

This is a well-designed fix that properly addresses the root cause of the issue. The solution is elegant, performant, and maintains backward compatibility.


🎯 Core Implementation Review

path.rs Changes - Excellent

The integration of extension rewriting into NodeImportResolver is the right approach:

  • Lines 104-106: New rewrite_import_extensions field in Config is well-documented
  • Lines 149-186: rewrite_extension() method correctly implements TypeScript's logic:
    • ✅ Proper relative path checking (./ or ../ prefixes)
    • ✅ Correctly excludes .d.ts files
    • ✅ Proper extension mapping (.ts.js, .tsx.jsx, etc.)
    • ✅ Preserves .json files unchanged
  • Lines 273-278: Extension rewriting applied at the correct point - AFTER path resolution
  • Performance: Single-pass implementation is more efficient than the previous two-pass approach

config/mod.rs Changes - Very Good

  • Lines 1486-1493: Smart conditional logic to avoid duplicate extension rewriting
  • Lines 1545, 1567, 1575, etc.: Consistent parameter threading throughout all resolver configurations
  • Lines 1926, 1948: Proper cache key inclusion to prevent cache pollution
  • Lines 1971: Correct configuration passing to NodeImportResolver

🧪 Test Coverage - Comprehensive

The test suite in fixture/issues-11xxx/11143/ is excellent:

Extension Coverage:

  • .ts.js
  • .tsx.jsx
  • .mts.mjs
  • .cts.cjs

Import Type Coverage:

  • ✅ Named exports: export { a } from "@/util/index.ts"
  • ✅ Export all: export * from "@/util/all.ts"
  • ✅ Dynamic imports: import("@/util/dynamic.ts")
  • ✅ Regular relative imports (regression test)

The expected output perfectly demonstrates the fix working correctly.


🔒 Security Review - No Concerns

  • No unsafe code introduced
  • Path resolution uses established, safe libraries (pathdiff, path_clean)
  • Input validation prevents directory traversal attacks
  • No credential or secret exposure risks

Performance Analysis - Improved

Benefits:

  • Single-pass optimization: Combines path resolution + extension rewriting
  • Smart caching: Cache key includes rewrite_import_extensions to prevent incorrect cache hits
  • Conditional execution: Only applies expensive rewriting when needed

No Performance Regressions:

  • Existing code paths unchanged when feature is disabled
  • Extension rewriting logic is lightweight (string operations only)

📋 Code Quality - Excellent

Rust Best Practices:

  • ✅ Proper use of &str over String (CLAUDE.md guideline Transformers for ecmascript #4)
  • ✅ Good error handling with Result<Atom, Error>
  • ✅ Appropriate use of Option for optional return values
  • ✅ Clean separation of concerns

Documentation:

  • ✅ Well-documented config field with clear description
  • ✅ Method comments explaining TypeScript compatibility
  • ✅ Inline comments for complex logic

Code Style:


🔍 Minor Considerations

1. Code Duplication (Low Priority)

The rewrite_extension() method in path.rs:152-186 duplicates logic from get_output_extension() in import_rewriter_typescript.rs:78-112. Consider:

// Future enhancement: Extract to shared utility
// crates/swc_ecma_transforms_module/src/util/extension_rewriter.rs

2. Edge Case Testing (Enhancement)

Consider adding tests for:

  • Mixed case extensions (.TS, .Tsx) - though TypeScript likely doesn't support these
  • Paths with query parameters or fragments (probably not relevant for Node.js)
  • Very long paths (stress testing)

3. Documentation Reference

The implementation correctly follows TypeScript's behavior, referencing the exact TypeScript source lines. Excellent adherence to upstream behavior.


🎯 Recommendations

For This PR: ✅ Approve and Merge

  • Implementation is correct and complete
  • Fixes the reported issue perfectly
  • No breaking changes
  • Excellent test coverage
  • Performance improvements

Future Enhancements (Not required for this PR):

  1. Shared utility: Extract extension rewriting logic to reduce duplication
  2. Comprehensive benchmarks: Measure performance gains from single-pass approach
  3. Additional edge case tests: Cover more exotic scenarios

Final Verdict

This is a high-quality PR that:

  • ✅ Correctly fixes the reported issue
  • ✅ Implements the solution elegantly and efficiently
  • ✅ Maintains backward compatibility
  • ✅ Includes comprehensive tests
  • ✅ Follows all project guidelines
  • ✅ Improves performance
  • ✅ Has no security concerns

Recommendation: Approve and merge 🚀

The implementation demonstrates deep understanding of both the problem and the codebase architecture. The integration approach is superior to simply reordering passes, and the execution is excellent.


@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

Binary Sizes

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

Commit: ab4a6f3

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025

CodSpeed Performance Report

Merging #11219 will not alter performance

Comparing devbird/fix-extension-rewrite-path-aliases (72aaad5) with main (6223100)

Summary

✅ 138 untouched

@kdy1 kdy1 added this to the Planned milestone Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

"rewriteRelativeImportExtensions": true not work with "jsc.paths" config

3 participants