Skip to content

fix(cli): skip mkdir when --out-file targets the current directory#11720

Merged
kdy1 merged 1 commit intomainfrom
claude/issue-11717-20260320-0904
Mar 20, 2026
Merged

fix(cli): skip mkdir when --out-file targets the current directory#11720
kdy1 merged 1 commit intomainfrom
claude/issue-11717-20260320-0904

Conversation

@kdy1
Copy link
Member

@kdy1 kdy1 commented Mar 20, 2026

When --out-file file.min.js is used without a directory component, Path::parent() returns an empty string. Calling create_dir_all("") fails on Windows with EEXIST. Skip the directory creation when the parent path is empty.

Closes #11717

Generated with Claude Code

When `--out-file file.min.js` is used without a directory component,
`Path::parent()` returns an empty string. Calling `create_dir_all("")`
fails on Windows with EEXIST. Skip the directory creation when the
parent path is empty since the current directory always exists.

Closes #11717

Co-authored-by: Donny/강동윤 <kdy1@users.noreply.github.com>
@kdy1 kdy1 requested a review from a team as a code owner March 20, 2026 15:44
Copilot AI review requested due to automatic review settings March 20, 2026 15:44
@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 Mar 20, 2026

⚠️ No Changeset found

Latest commit: b3c1f62

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

@kdy1 kdy1 enabled auto-merge (squash) March 20, 2026 15:44
@claude
Copy link
Contributor

claude bot commented Mar 20, 2026

PR Review

Summary

Clean, well-scoped fix for --out-file file.min.js (no directory component) failing on Windows because Path::parent() returns "" and create_dir_all("") errors with EEXIST.

Code Quality

The fix is correct and minimal. Both call sites where parent() is used for directory creation are patched consistently — emit_output (line 244) and CompileOptions (line 480). The as_os_str().is_empty() check is the idiomatic way to detect this edge case in Rust.

Potential Issues

None identified. The guard correctly short-circuits before create_dir_all when the parent is empty (i.e., the file targets the current directory), and the subsequent File::create / fs::write calls work fine with bare filenames.

Minor Observation

In emit_output (line 244), the two conditions are combined in one if:

if !output_dir.as_os_str().is_empty() && !output_dir.is_dir() {

While in the second site (line 480), only the empty check is used:

if !parent.as_os_str().is_empty() {
    fs::create_dir_all(parent)?;
}

The second site unconditionally calls create_dir_all when parent is non-empty, even if the directory already exists. This is fine since create_dir_all is a no-op for existing directories, but the patterns are slightly inconsistent. Not a bug — just a style note. The first site's !is_dir() check avoids a syscall, which is a minor optimization.

Performance

No concerns. The added as_os_str().is_empty() check is essentially free (no allocation, no syscall).

Security

No concerns.

Test Coverage

Good — the new test issue_11717_out_file_same_directory covers the exact failure case: compiling with --out-file file.min.js (no directory component) and asserting it succeeds. The test verifies both that the command exits successfully and that the output contains expected content.

One suggestion: consider also testing the --out-dir path (the emit_output function) with a relative filename to cover both patched sites, though the risk is low since the emit_output path constructs the output path differently via resolve_output_file_path.

Verdict

LGTM — clean fix, well-tested, addresses a real cross-platform issue.

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

Fixes a Windows-specific CLI failure when --out-file is given without a directory component (e.g. file.min.js), where the computed parent path can be empty and create_dir_all("") fails.

Changes:

  • Skip directory creation when the computed parent path is empty for --out-file and --out-dir output paths.
  • Add a regression test covering --out-file file.min.js written to the current directory (issue #11717).

Reviewed changes

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

File Description
crates/swc_cli_impl/src/commands/compile.rs Avoids calling create_dir_all with an empty parent path when emitting output files.
crates/swc_cli_impl/tests/issues.rs Adds a regression test to ensure --out-file in the current directory succeeds.

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

@github-actions
Copy link
Contributor

Binary Sizes

File Size
swc.linux-x64-gnu.node 28M (29065032 bytes)

Commit: 2f6b10c

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 20, 2026

Merging this PR will not alter performance

✅ 219 untouched benchmarks


Comparing claude/issue-11717-20260320-0904 (b3c1f62) with main (33fe6f2)

Open in CodSpeed

@kdy1 kdy1 merged commit f3f4e51 into main Mar 20, 2026
50 of 51 checks passed
@kdy1 kdy1 deleted the claude/issue-11717-20260320-0904 branch March 20, 2026 16:09
@github-actions github-actions bot added this to the Planned milestone Mar 20, 2026
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.

Outputting in the same directory as the current is broken

3 participants