-
-
Notifications
You must be signed in to change notification settings - Fork 33.8k
path: add fromURL
#44315
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
path: add fromURL
#44315
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -267,6 +267,38 @@ path.format({ | |
| // Returns: 'C:\\path\\dir\\file.txt' | ||
| ``` | ||
|
|
||
| ## `path.fromURL(pathOrUrl)` | ||
|
|
||
| <!-- YAML | ||
| added: REPLACEME | ||
| --> | ||
|
|
||
| * `pathOrUrl` {string|URL}. | ||
| * Returns: {string} | ||
|
|
||
| Converts a [`URL`][] instance to a path string, | ||
| returns `pathOrUrl` if it is already a string. | ||
|
|
||
| A [`TypeError`][] is thrown if `pathOrUrl` is a | ||
| [`URL`][] with a schema other than `file://`. | ||
|
|
||
| ```js | ||
| path.fromURL(new URL('file:///Users/node/dev')); | ||
| // Returns: '/Users/node/dev' | ||
|
|
||
| path.fromURL('file:///Users/node/dev'); | ||
| // Returns: 'file:///Users/node/dev' | ||
|
|
||
| path.fromURL(new URL('file:///c:/foo.txt')); | ||
| // Returns On Windows: 'c:\\foo.txt' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am still unsure if I like the fact that path.fromURL(url.pathToFileURL(absolutePath)) === path.fromURL(absolutePath)but that does not seem to be true in all cases. On the other hand, this seems to be what @sindresorhus expects.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree this makes more sence:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you decide if the user refers to a file: URL or to a relative path inside a directory named
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that's orthogonal to my concern regarding OS-specific path normalization, and not what this comment is about.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am actually less concerned about that since you can be explicit and use
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, that's also not what my comment is about. My concern is the fact that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tniessen do you think we should change the behavior even if the name of the method is changed to one of the names suggested here ( |
||
|
|
||
| path.fromURL('index/path'); | ||
| // Returns: 'index/path' | ||
MoLow marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| path.fromURL(new URL('http://example.com')); | ||
| // Throws 'TypeError [ERR_INVALID_URL_SCHEME]: The URL must be of scheme file' | ||
| ``` | ||
|
|
||
| ## `path.isAbsolute(path)` | ||
|
|
||
| <!-- YAML | ||
|
|
@@ -604,6 +636,7 @@ The API is accessible via `require('node:path').win32` or `require('node:path/wi | |
|
|
||
| [MSDN-Rel-Path]: https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#fully-qualified-vs-relative-paths | ||
| [`TypeError`]: errors.md#class-typeerror | ||
| [`URL`]: url.md#the-whatwg-url-api | ||
| [`path.parse()`]: #pathparsepath | ||
| [`path.posix`]: #pathposix | ||
| [`path.sep`]: #pathsep | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,21 @@ const { | |
| validateString, | ||
| } = require('internal/validators'); | ||
|
|
||
| const { | ||
| codes: { | ||
| ERR_INVALID_ARG_TYPE, | ||
| ERR_INVALID_URL_SCHEME, | ||
| }, | ||
| } = require('internal/errors'); | ||
|
|
||
|
|
||
| let urlModule; | ||
| function lazyURLModule() { | ||
| // Lazy require to avoid circular dependency | ||
| urlModule ??= require('internal/url'); | ||
| return urlModule; | ||
| } | ||
|
|
||
| const platformIsWin32 = (process.platform === 'win32'); | ||
|
|
||
| function isPathSeparator(code) { | ||
|
|
@@ -1061,6 +1076,19 @@ const win32 = { | |
| return ret; | ||
| }, | ||
|
|
||
| fromURL(fileURLOrPath) { | ||
| if (typeof fileURLOrPath === 'string') { | ||
| return fileURLOrPath; | ||
| } | ||
| if (!lazyURLModule().isURLInstance(fileURLOrPath)) { | ||
| throw new ERR_INVALID_ARG_TYPE('fileURLOrPath', ['string', 'URL'], fileURLOrPath); | ||
| } | ||
| if (fileURLOrPath.protocol !== 'file:') | ||
| throw new ERR_INVALID_URL_SCHEME('file'); | ||
|
|
||
| return lazyURLModule().getPathFromURLWin32(fileURLOrPath); | ||
| }, | ||
|
|
||
| sep: '\\', | ||
| delimiter: ';', | ||
| win32: null, | ||
|
|
@@ -1527,12 +1555,26 @@ const posix = { | |
| return ret; | ||
| }, | ||
|
|
||
| fromURL(fileURLOrPath) { | ||
| if (typeof fileURLOrPath === 'string') { | ||
| return fileURLOrPath; | ||
| } | ||
| if (!lazyURLModule().isURLInstance(fileURLOrPath)) { | ||
| throw new ERR_INVALID_ARG_TYPE('fileURLOrPath', ['string', 'URL'], fileURLOrPath); | ||
| } | ||
| if (fileURLOrPath.protocol !== 'file:') | ||
| throw new ERR_INVALID_URL_SCHEME('file'); | ||
|
|
||
| return lazyURLModule().getPathFromURLPosix(fileURLOrPath); | ||
| }, | ||
|
|
||
| sep: '/', | ||
| delimiter: ':', | ||
| win32: null, | ||
| posix: null | ||
| }; | ||
|
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: unrelated change. |
||
| posix.win32 = win32.win32 = win32; | ||
| posix.posix = win32.posix = posix; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
| const assert = require('node:assert'); | ||
| const { fromURL, win32 } = require('node:path'); | ||
| const { describe, it } = require('node:test'); | ||
|
|
||
|
|
||
| describe('path.fromURL', { concurrency: true }, () => { | ||
| it('should passthrough path string', () => assert | ||
| .strictEqual(fromURL('/Users/node/dev'), '/Users/node/dev')); | ||
| it('should passthrough an empty string', () => assert | ||
| .strictEqual(fromURL(''), '')); | ||
| it('should passthrough a URL string with file schema', () => assert | ||
| .strictEqual(fromURL('file:///Users/node/dev'), 'file:///Users/node/dev')); | ||
| it('should parse a URL instance with file schema', () => assert | ||
| .strictEqual(fromURL(new URL('file:///Users/node/dev')), '/Users/node/dev')); | ||
| it('should parse a URL instance with file schema', () => assert | ||
| .strictEqual(fromURL(new URL('file:///path/to/file')), '/path/to/file')); | ||
| it('should remove the host from a URL instance', () => assert | ||
| .strictEqual(fromURL(new URL('file://localhost/etc/fstab')), '/etc/fstab')); | ||
| it('should parse a windows file URI', () => { | ||
| const fn = common.isWindows ? fromURL : win32.fromURL; | ||
| assert.strictEqual(fn(new URL('file:///c:/foo.txt')), 'c:\\foo.txt'); | ||
| }); | ||
| it('should throw an error if the URL is not a file URL', () => assert | ||
| .throws(() => fromURL(new URL('http://localhost/etc/fstab')), { code: 'ERR_INVALID_URL_SCHEME' })); | ||
| it('should throw an error if converting invalid types', () => [{}, [], 1, null, undefined, | ||
| Promise.resolve(new URL('file:///some/path/')), Symbol(), true, false, 1n, 0, 0n, () => {}] | ||
| .forEach((item) => assert.throws(() => fromURL(item), { code: 'ERR_INVALID_ARG_TYPE' }))); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the name misleading, given that the input is not necessarily a
URL? In fact, URL strings of the formfile:///homeare ignored and not treated as URLs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would you consider to be a better name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
url.toPathIfFileURL?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe
path.fromURLInstance?