-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
Remove restriction on '/' prefixes #49449
Comments
To be clear, this is about direct import specifiers (https://nodejs.org/api/esm.html#esm_import_specifiers), e.g.: // imports file:///usr/local/etc/mytool.config.mjs, assuming import.meta.url has file:// protocol.
import config from '/usr/local/etc/mytool.config.mjs'; I think that makes sense. Given |
Wait, why would we want to allow this? What are the use cases for root-absolute specifiers? I can't conceive of a time in CJS where requiring from a /-prefixed path wasn't a bug (or bad design, as was later revealed when it was ran on a different machine). |
@ljharb because it adds consistency and I cannot think of a reason to disallow these specifiers on concrete concerns if we are not reserving them for future features. Any future feature using those prefixes not matching the generally accepted behavior of URL resolution would be bad so it isn't like we can use the feature for other purposes beyond pinning to some kind of root. |
I absolutely agree that if |
@ljharb why is it a footgun? is there clear reasoning here or is it just a linting issue? |
It's a footgun because it's not project-portable or machine-portable; any time you have code that uses |
@ljharb that statement is true for any given one off script that isn't installed via a package manager and uses bare specifiers for things. I do see utility in various things like the ability to |
I have definitely imported things using absolute paths. If it's top-level application code that will only ever run in a docker container, I don't see anything wrong with importing something (like a config file) from an absolute path. I haven't seen strong evidence that this behavior caused issues in |
Node.js does have a "directory" like this - because modules in node use the The fact that Apache may serve some of the |
That node's implementation uses URLs doesn't mean that node module specifiers are conceptually URLs - they're file paths (which happen to be a subset of URLs). |
@ljharb the file specifiers are URLs, not paths, they do have fragments taken into consideration. |
I realize they are URLs. I'm saying most users will never think of them that way. |
I think it would be dangerous if we allow users to think that they are file paths. If allowing |
@DerekNonGeneric that's because MDN is geared towards the web, not node, and on the web, that's a domain-relative URL. I feel very strongly that if a leading |
Yes, for require('/tmp/a%20b.js'); // the file "/tmp/a%20b.js" ("file:///tmp/a%2520b.js")
import('/tmp/a%20b.js'); // the file "/tmp/a b.js" ("file:///tmp/a%20b.js") If we don't think people can differentiate between those two different paths, maybe we shouldn't allow a leading slash in P.S.: I think we can expect people to think of imports as URL-based longer term. There's way more JavaScript-first devs than node.js-first devs and hopefully the former will become used to thinking in URLs when using |
@DerekNonGeneric that seems like you just want to run a |
Wait, what does |
Right, but "c:" isn't necessarily the fs root on windows - where linux has Perhaps it maps to the root of the working directory's drive? |
It works on everywhere but windows natively and windows has workarounds for this via cygwin, windows subsystem for linux, etc. The point was that the request doesn't seem to be something that node needs to specifically handle, and the details of chroot are quite complex. |
@DerekNonGeneric there is no point that I stated that windows is second class. |
After reading the other recent thread, I think my bigger concern is with *nix platforms where people may confuse I would be in favor of a clear warning in our resolver when resolving a host-relative URL against a |
Given that we have self referential packages, and
"imports"
inpackage.json
. In addition since//
is reserved I think we should remove the reservation since it will by the default URL parser be treated as protocol relative.The text was updated successfully, but these errors were encountered: