Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update resolveBin to use fileURLToPath for cross-platform compatibility #2069

Merged
merged 1 commit into from
Mar 31, 2025

Conversation

astrochemx
Copy link
Contributor

PR Checklist

Overview

🧐 The Problem

On Windows, file paths typically begin with a drive letter (e.g., C:). However, to conform to the file:// URI scheme in Node.js, they must be prefixed with a slash (e.g., /C:), resulting in a correct file URL like file:///C:/GHub/file.js.

Currently, the resolveBin function does not properly handle this conversion and mistakenly outputs an absolute path with a leading slash, such as /C:/GHub/file.js. This format is invalid on Windows, where the correct absolute path should be C:/GHub/file.js.

Mishandling paths that start with a leading slash on Windows can cause them to be interpreted as relative to the current drive, leading to incorrect resolutions. This can result in an erroneous path with duplicated drive letters (like C:/C:/GHub/file.js).

💡 The Solution

To resolve this issue, we should use the fileURLToPath function from the node:url module instead of manually handling paths via regular expressions. This approach ensures that paths are correctly converted and eliminates the risk of incorrect path handling on Windows.

✔️ Testing

I have tested this change on Windows and WSL (Ubuntu 24.04.2 LTS), and it works as expected.

Additional Info

💖

Copy link
Owner

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 thanks!

@JoshuaKGoldberg JoshuaKGoldberg merged commit 04e7396 into JoshuaKGoldberg:main Mar 31, 2025
12 checks passed
@JoshuaKGoldberg
Copy link
Owner

@all-contributors please add @astrochemx for code.

🤖 Beep boop! This comment was added automatically by all-contributors-auto-action.
Not all contributions can be detected from Git & GitHub alone. Please comment any missing contribution types this bot missed.
...and of course, thank you for contributing! 💙

Copy link
Contributor

@JoshuaKGoldberg

I've put up a pull request to add @astrochemx! 🎉

Copy link

🎉 This is included in version v2.18.6 🎉

The release is available on:

Cheers! 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug: resolveBin function doesn't work correctly on Windows
2 participants