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

🐛 Bug: resolveBin function doesn't work correctly on Windows #2064

Closed
3 tasks done
astrochemx opened this issue Mar 28, 2025 · 3 comments · Fixed by #2069
Closed
3 tasks done

🐛 Bug: resolveBin function doesn't work correctly on Windows #2064

astrochemx opened this issue Mar 28, 2025 · 3 comments · Fixed by #2069
Labels
status: accepting prs Please, send a pull request to resolve this! type: bug Something isn't working :(

Comments

@astrochemx
Copy link
Contributor

astrochemx commented Mar 28, 2025

Bug Report Checklist

  • I have tried restarting my IDE and the issue persists.
  • I have pulled the latest main branch of the repository.
  • I have searched for related issues and found none that matched my issue.

Expected

To create a new app without any errors.

Actual

On Windows, an error occurs in the resolveBin function due to incorrect path handling, which results in a duplicate drive letter, such as C:\C:\GHub\create-typescript-app\node_modules\.pnpm\[email protected]\node_modules\cspell-populate-words\bin\index.mjs.

Example of an error:
Error: Cannot find module 'C:\C:\GHub\create-typescript-app\node_modules\.pnpm\[email protected]\node_modules\cspell-populate-words\bin\index.mjs'
    at Function._resolveFilename (node:internal/modules/cjs/loader:1405:15)
    at defaultResolveImpl (node:internal/modules/cjs/loader:1061:19)
    at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1066:22)
    at Function._load (node:internal/modules/cjs/loader:1215:37)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:151:5)
    at node:internal/main/run_main_module:33:47 {
      code: 'MODULE_NOT_FOUND',
      requireStack: []
    }
Node.js v23.10.0
    at getFinalError (file:///C:/GHub/bingo/node_modules/.pnpm/[email protected]/node_modules/execa/lib/return/final-error.js:6:9)
    at makeError (file:///C:/GHub/bingo/node_modules/.pnpm/[email protected]/node_modules/execa/lib/return/result.js:108:16)
    at getAsyncResult (file:///C:/GHub/bingo/node_modules/.pnpm/[email protected]/node_modules/execa/lib/methods/main-async.js:167:4)       
    at handlePromise (file:///C:/GHub/bingo/node_modules/.pnpm/[email protected]/node_modules/execa/lib/methods/main-async.js:150:17)       
    at async runCommand (file:///C:/GHub/bingo/packages/bingo/lib/runners/applyScriptsToSystem.js:10:24)
    at async file:///C:/GHub/bingo/packages/bingo/lib/runners/applyScriptsToSystem.js:22:17
    at async Promise.all (index 1)
    at async applyScriptsToSystem (file:///C:/GHub/bingo/packages/bingo/lib/runners/applyScriptsToSystem.js:20:9)
    at async Promise.all (index 0)
    at async runCreation (file:///C:/GHub/bingo/packages/bingo/lib/runners/runCreation.js:8:5)

Additional Info

🧐 Cause of the Issue

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. In this case, it results in an erroneous path with duplicated drive letters (like C:/C:/GHub/file.js), as seen in the error output.

💡 Proposed Solution

To resolve this issue, we should use the fileURLToPath function from the node:url module instead of manually handling paths via regular expressions.

🖥️ Proposed Code

Original code:

export function resolveBin(bin: string) {
	return import.meta.resolve(bin).replace(/^file:\/\//gu, "");
}

Updated code:

import { fileURLToPath } from "node:url";

export function resolveBin(bin: string) {
	return fileURLToPath(import.meta.resolve(bin));
}

This approach ensures that paths are correctly converted and eliminates the risk of incorrect path handling on Windows.

✔️ Testing

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


💖

@astrochemx astrochemx added the type: bug Something isn't working :( label Mar 28, 2025
@JoshuaKGoldberg
Copy link
Owner

You are just hitting all the Windows bugs 🙂

Related: JoshuaKGoldberg/bingo#300

@JoshuaKGoldberg JoshuaKGoldberg added the status: accepting prs Please, send a pull request to resolve this! label Mar 28, 2025
JoshuaKGoldberg pushed a commit that referenced this issue Mar 31, 2025
…mpatibility (#2069)

<!-- 👋 Hi, thanks for sending a PR to create-typescript-app! 🎁
Please fill out all fields below and make sure each item is true and [x]
checked.
Otherwise we may not be able to review your PR. -->

## PR Checklist

- [x] Addresses an existing open issue: fixes #2064
- [x] That issue was marked as [`status: accepting
prs`](https://github.com/JoshuaKGoldberg/create-typescript-app/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22)
- [x] Steps in
[CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/create-typescript-app/blob/main/.github/CONTRIBUTING.md)
were taken

## Overview

<!-- Description of what is changed and how the code change does that.
-->

### 🧐 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](https://nodejs.org/api/url.html#urlfileurltopathurl-options)
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

💖
@JoshuaKGoldberg
Copy link
Owner

@all-contributors please add @astrochemx for bug.

🤖 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

🎉 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
status: accepting prs Please, send a pull request to resolve this! type: bug Something isn't working :(
Projects
None yet
2 participants