Skip to content

Handle a drive-less file URL instead of throwing - #162

Closed
PerryLink wants to merge 1 commit into
vitejs:mainfrom
PerryLink:fix/drive-less-file-url
Closed

PerryLink wants to merge 1 commit into
vitejs:mainfrom
PerryLink:fix/drive-less-file-url

Conversation

@PerryLink

Copy link
Copy Markdown

What

parseFile calls fileURLToPath() directly. On Windows, a file:/// URL without a drive
letter
— the form several tools emit — makes Node throw ERR_INVALID_FILE_URL_PATH, and
launchEditor() propagates that to the caller instead of opening anything.

require('launch-editor')('file:///foo/bar.js:1:1')
// TypeError [ERR_INVALID_FILE_URL_PATH] at parse-file.js:8

The change

Catch exactly that error and map the drive-less path onto the current drive, so the editor is still
launched. //server/share URLs are still rejected by the existing UNC guard, and every other input
takes the same path as before.

Verification

  • before: the call above throws and exits 1; node --test parse-file.test.js → 3 pass / 1 fail
    (the new case);
  • after: the same call returns normally; parse-file.test.js 4/4; the whole launch-editor
    package suite 84/84.

No CLA/DCO requirement in the repository.

@sapphi-red

Copy link
Copy Markdown
Member

It's not clear to me why this is needed. When does this happen?

@PerryLink

Copy link
Copy Markdown
Author

Fair question, and my PR description overstated it — I can't point to a tool that emits a drive-less URL on Windows. What I can substantiate is the API form the README already promises (// file:// URIs are also supported) and the example in #53:

launch('file:///foo/bar/foo.js:12:34')

On POSIX that resolves to /foo/bar/foo.js; on Windows the same call throws ERR_INVALID_FILE_URL_PATH because fileURLToPath requires a drive, so the documented file-URI support only works for file:///C:/... there. The middleware forwards file:// values unchanged (#95), so ?file=file%3A%2F%2F%2Ffoo.js takes the same path.

The current patch guesses the current drive for those URLs. If you'd rather keep rejecting a drive-less path, I'm happy to close this — or turn it into an explicit, clearer error instead of the raw Node one. Which would you prefer?

@sapphi-red

Copy link
Copy Markdown
Member

It doesn't make sense to me to add support for that case.

@sapphi-red sapphi-red closed this Oct 27, 2026
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.

2 participants