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

Add a utility to convert file URL or path to one of them? #41521

Open
fisker opened this issue Jan 14, 2022 · 32 comments
Open

Add a utility to convert file URL or path to one of them? #41521

fisker opened this issue Jan 14, 2022 · 32 comments
Labels
feature request Issues that request new features to be added to Node.js. url Issues and PRs related to the legacy built-in url module.

Comments

@fisker
Copy link
Contributor

fisker commented Jan 14, 2022

What is the problem this feature will solve?

It's a nice way to accept both URL and path string in API, but we need covert it to either string or URL, it would be nice to have a built-in method to do this.

What is the feature you are proposing to solve the problem?

For converting to string:

Maybe add path.from(URL | string)? @sindresorhus proposed here.

For converting to URL:

url.from(URL | string)

I'm not sure, since URL is a standard, I guess adding method URL object won't be acceptable, maybe to node:url module?

What alternatives have you considered?

No response

@fisker fisker added the feature request Issues that request new features to be added to Node.js. label Jan 14, 2022
@fisker fisker changed the title Add a utility to convert file URL and path to path? Add a utility to convert between file URL or path to one of them? Jan 14, 2022
@fisker fisker changed the title Add a utility to convert between file URL or path to one of them? Add a utility to convert file URL or path to one of them? Jan 14, 2022
@aduh95
Copy link
Contributor

aduh95 commented Jan 14, 2022

We have a toPathIfFileURL internal API already, maybe we could expose it in the node:url module

node/lib/internal/url.js

Lines 1558 to 1562 in 38b7961

function toPathIfFileURL(fileURLOrPath) {
if (!isURLInstance(fileURLOrPath))
return fileURLOrPath;
return fileURLToPath(fileURLOrPath);
}

For converting to URL:

url.from(URL | string)

I don't think there is any reliable way to check if a string is a path or a file URL... I'm afraid the best option here is to assume that a string is a path and call pathToFileURL(string) (which is what is doing Node.js internally as well) – or if you prefer to assume strings are URLs in your application, call new URL(string).

@aduh95 aduh95 added the url Issues and PRs related to the legacy built-in url module. label Jan 14, 2022
@sindresorhus
Copy link

We have a toPathIfFileURL internal API already, maybe we could expose it in the node:url module

Wouldn't it be more natural to have it in node:path, since we want to convert it to a path? My proposal for path.from() was inspired by Array.from and Buffer.from type methods.

@sindresorhus
Copy link

I don't think there is any reliable way to check if a string is a path or a file URL...

To be clear, the string in that example is meant to be a file path, not a string file URL.

Example usage:

import path from 'node:path';

// Passthrough
path.from('/Users/sindresorhus/dev');
//=> '/Users/sindresorhus/dev'

path.from(new URL('file:///Users/sindresorhus/dev'));
//=> '/Users/sindresorhus/dev'

@benjamingr
Copy link
Member

Is this what you had in mind in terms of semantics? benjamingr@0704851#diff-6610c8d452ba38a0db8c591e06af555bc7755ace17065a066c853ffcc5271559R7-R20

@fisker
Copy link
Contributor Author

fisker commented Feb 17, 2022

Yes, that's what I want.

@benjamingr
Copy link
Member

@aduh95 wdyt about an implementation that does that? ( #41521 (comment) )

@aduh95
Copy link
Contributor

aduh95 commented Feb 17, 2022

@aduh95 wdyt about an implementation that does that? ( #41521 (comment) )

I don't think we'll get consensus landing in core something that treats differently a string depending on if it starts with file: or not – what if the user meant ./file:/ (i.e. they really have a local directory named file:)?

The workarounds that come to mind are:

  • only support absolute paths – or require relative paths to start with a . (like in require calls).
  • always treat a string as a path.
  • publish this as an npm module instead (I think npm module would get away with not supporting having directory named file:).

Any combination of the above would address my concern, but I'm open to other ideas if someone thinks of something else.

@benjamingr
Copy link
Member

benjamingr commented Feb 17, 2022

@aduh95 I think it's safe to say "if this can be an npm module, sindre has already probably published one" :D Making small useful modules kind of their thing.

Edit: lol found it https://www.npmjs.com/package/file-url
Edit2: actually that's the other side of conversion

@benjamingr
Copy link
Member

Basically: I think the main issue with this being on npm is that file urls and paths are both very common in modern Node.js and not having a utility to convert them in core is weird.

I'm sure we can bikeshed the name and semantics as long as the core use case (convert file url to path) is addressed.

Would it be better if the name was explicit path.fromFileUrl that only takes either a file:// url string or an actual URL with the file protocol and throws an error on non-file urls?

@sindresorhus
Copy link

I have no need for supporting file:// strings.

What I would like is:

export const from = urlOrPath => urlOrPath instanceof URL ? fileURLToPath(urlOrPath) : urlOrPath;
import path from 'path';

path.from('/Users/sindresorhus');
//=> '/Users/sindresorhus'

path.from(new URL('file:///Users/sindresorhus'));
//=> '/Users/sindresorhus'

@sindresorhus
Copy link

Would it be better if the name was explicit path.fromFileUrl that only takes either a file:// url string or an actual URL with the file protocol and throws an error on non-file urls?

No, this does not solve the request.

@benjamingr
Copy link
Member

So you would expect:

path.from('file:///Users/sindresorhus');
//=> file:///Users/sindresorhus

?

@targos
Copy link
Member

targos commented Feb 17, 2022

Edit: was already mentioned above

I have no need for supporting file:// strings.

What I would like is:

export const from = urlOrPath => urlOrPath instanceof URL ? fileURLToPath(urlOrPath) : urlOrPath;
import path from 'path';

path.from('/Users/sindresorhus');
//=> '/Users/sindresorhus'

path.from(new URL('file:///Users/sindresorhus'));
//=> '/Users/sindresorhus'

So basically you would like toPathIfFileURL to become public API?

node/lib/internal/url.js

Lines 1555 to 1563 in 9caceb2

function isURLInstance(fileURLOrPath) {
return fileURLOrPath != null && fileURLOrPath.href && fileURLOrPath.origin;
}
function toPathIfFileURL(fileURLOrPath) {
if (!isURLInstance(fileURLOrPath))
return fileURLOrPath;
return fileURLToPath(fileURLOrPath);
}

@fisker
Copy link
Contributor Author

fisker commented Feb 17, 2022

FYI: I have url-or-path published myself, and I found check protocol not that useful, and I removed recently.

@sindresorhus
Copy link

@benjamingr Yes

@sindresorhus
Copy link

So basically you would like toPathIfFileURL to become public API?

Yes

@benjamingr
Copy link
Member

Would it be fine if the name was more explicit like path.fromURL or path.fromFileURL ?

@aduh95
Copy link
Contributor

aduh95 commented Feb 17, 2022

I don't think we can be much more explicit than url.toPathIfFileURL.

@github-actions
Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Aug 17, 2022
@sindresorhus
Copy link

Please keep this open.

@benjamingr
Copy link
Member

@MoLow wanna take a look?

@MoLow
Copy link
Member

MoLow commented Aug 17, 2022

Sure, once I'm back from vacation

@khaosdoctor
Copy link
Contributor

khaosdoctor commented Aug 24, 2022

Would it be fine if the name was more explicit like path.fromURL or path.fromFileURL ?

+1 to this, the semantics seems more concise with the rest of the API

@sindresorhus
Copy link

sindresorhus commented Apr 22, 2023

The pull request for this issue was recently closed for lack of response from one of the reviewers.

The main open question is how file URL strings ('file:///c:/foo.txt') should be handled. Should they be normalized to a path or passed through?

I would argue they should be either passed through or even throw a user-friendly error. Node.js APIs do not seem to currently handle file URL strings and I don't think they should either:

fs.readFileSync('file:///Users/sindresorhus/x.md');
// Error: ENOENT: no such file or directory, open

@aduh95
Copy link
Contributor

aduh95 commented Apr 22, 2023

Node.js APIs do not seem to currently handle file URL strings

Just to clarify, it does handle it but as a relative path (i.e. in your example it tries to read ./file:/Users/sindresorhus/x.md).

@SetTrend
Copy link

SetTrend commented Apr 28, 2023

This solution would particularly be useful for converting import.meta.url into a file path.

@aduh95
Copy link
Contributor

aduh95 commented Apr 28, 2023

This solution would particularly be useful for converting import.meta.url into a file path.

You can already use fileURLToPath(import.meta.url) for that.

@SetTrend
Copy link

Oh, sorry. Got mislead by reading as this issue is still open.

Copy link
Contributor

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

@github-actions github-actions bot added the stale label Jan 28, 2024
@sindresorhus
Copy link

Please keep this open.

@github-actions github-actions bot removed the stale label Jan 29, 2024
@MrMothman
Copy link

Is this still open to being implemented? I haven't done open source before in a meaningful way but I want to try this if I still can?

@RedYetiDev
Copy link
Member

AFAICT this is implemented as url.pathToFileURL and url.fileURLToPath.

@nodejs/path would it be worth it to make path.toURL and path.fromURL an alias to those functions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. url Issues and PRs related to the legacy built-in url module.
Projects
Status: Pending Triage
Development

Successfully merging a pull request may close this issue.

14 participants