-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Discussion: File URLs in Node.js #22502
Comments
comply to new validation requirements of node 10 for custom loaders
I think utility methods like these would make sense to add along with some documentation around how and why they're useful. |
those two functions already exist in |
I think some examples would get more clear explanation on why this is a great idea: When converting path to URL there are multiple ways to mess up: new URL(__filename) // errors (needs scheme)
__filename = './foo#1';
// '/foo' instead of the correct '/foo%231'
new URL(__filename, 'file:///').pathname;
__filename = './foo?2';
// '/foo' instead of the correct '/foo%3F1'
new URL(__filename, 'file:///').pathname;
__filename = '//nas/foo.txt';
// '/nas/foo.txt' instead of the correct '/foo.txt'
new URL(`file://${__filename}`).pathname; When converting from URL to path similar errors can occur (not just limited to other languages): url = new URL('file://nas/foo.txt');
// foo.txt, but that is missing the remote host 😱
fs.readFile(url.pathname, () => {});
url = new URL('file:///你好.txt');
// reads '/%E4%BD%A0%E5%A5%BD.txt' instead of '/你好.txt'
fs.readFile(url.pathname, () => {});
url = new URL('file:///hello world.txt');
// reads '/hello%20world.txt' instead of '/hello world.txt'
fs.readFile(url.pathname, () => {}); Another concern that hasn't been mentioned here is that lots of windows tooling uses url = new URL('file:///c:/foo/data.csv');
// passes 'c:/foo/data.csv' instead of 'c:\\foo\\data.csv'
spawn('script.bat', [url.pathname], () => {}); |
This was resolved in #22506. |
I wanted to open up a discussion on this topic, as previous work around this has perhaps lacked wider context (#20950)
Basically when we provide file:/// URLs to users (through
import.meta.url
, the Loader API for modules which relies heavily on file URLs, or any other means), we are expecting users to understand a lot of intricacies of how file URLs work in Node, which it already seems 90% of people will miss.For example, see - wasm-tool/node-loader@e4f6b7d.
There are two major problems most people will walk into blindly when converting from file URLs to paths in Node.js:
fs.readFile(url.pathname)
works in unix systems, but will break on Windows. This means Windows support will naturally hit a wide and reliably propagating point of friction as these workflows integrate into the npm ecosystem. This issue will keep coming up across many projects as they work with file URLs.Non-latin characters need to be percent decoded.
import './你好.mjs'
will be resolved intofile:///.../%E4%BD%A0%E5%A5%BD.mjs
, so that in order to support loading the native characters from the file system, a percent decode operation needs to be performed on the path, with some special cases (eg not percent decoding path separators).(1) is the immediate issue that will show as one of the standard Windows compatibility issues (alongside
path.replace(/\\/g, '/')
), and (2) seems like a deeper less seen Anglocentric preference that will continue to propagate here as well.Since I've personally not been able to make any progress on this problem through #20950 I'd be interested to hear what we might be able to do about this.
What I would like to suggest here is two new native functions:
Let me know if that sounds like a good idea here, and I can see if we can get something into
path
orurl
... (suggestions on which is best are welcome too).The text was updated successfully, but these errors were encountered: