-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
url: Cannot load ESM module with newline in file name #23696
Comments
esm uses urls which apparently normalize out newlines, so you'll need to use
|
Using your code seems to lead to double-escaping. It still fails. Here is the error message:
Furthermore, requiring to escape newlines when spawning a process would not be consistent: am I supplying a relative URL or a system-dependent path? I assume it is a system path. I can successfully pass I am writing a module to convert |
This sounds like a bug to me - the conversion into a URL of the initial path should be |
The bug is in the
|
/cc @nodejs/url Given:
|
Interesting... yeah Edge, Firefox, and Chrome all produce /cc @nodejs/url |
If 3/4 browsers all give an answer different from the spec, filing an issue at https://github.com/whatwg/url is a good idea. That said, in general I'm not sure you should expect arbitrary strings to round-trip through URL parsing and serialization, so there may be a more fundamental issue here. (I haven't read the whole thread.) |
Seems to be related to this line in the spec:
I confirm that files with a tab in their name also fail. I'll open a issue on the whatwg repo. Chrome and Firefox do not remove tabs and newlines. Edit: whatwg/url#419 |
This bug also occurs with TAB and CR characters. I have a patch that fixes it by percent-encoding these characters before passing them to the |
Am I correct to conclude that Node.js is spec-compliant in its treatment of newlines in ES Module paths, although multiple browsers are not? (If so, should this be closed, as the bug is either in the spec or in the browser implementations and not Node.js?) |
@Trott it's a spec concern at this point, yes. But this issue should stay open while the root spec issue at whatwg/url#419 remains unresolved, as which way that goes would determine whether or not this should be a Node bug. |
Fixes: #23696 PR-URL: #23720 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Fixes: #23696 PR-URL: #23720 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Fixes: #23696 PR-URL: #23720 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Fixes: nodejs#23696 PR-URL: nodejs#23720 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Fixes: #23696 PR-URL: #23720 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Node fails to load ESM entry points if their filename contains a newline character. This is a regression compared to the current commonjs modules.
The following code fails:
Expected: no failed assertion, nothing in stderr (appart from the warning that ESM is experimental).
Actual: Failed assertion. Stderr contains:
Note that the error message does not even contain the newline in the file name.
For comparison, the following code using commonJS works fine:
The text was updated successfully, but these errors were encountered: