Repository navigation
cli: NODE_OPTIONS does not support --require/-r + path with spaces #12971
Description
- Version: v8.0.0 nightly 2017 05 07
- Platform: Windows 7 x64
- Subsystem: cli
Refs: #12028
test.js:
'use strict';
console.log('B');module.js:
console.log('A');"mo dule.js":
console.log('A');Compare:
> node -r ./module.js test.js
A
B
> set NODE_OPTIONS=-r ./module.js
> set NODE_OPTIONS
NODE_OPTIONS=-r ./module.js
> node test.js
A
B> node -r "./mo dule.js" test.js
A
B
> set NODE_OPTIONS=-r "./mo dule.js"
> set NODE_OPTIONS
NODE_OPTIONS=-r "./mo dule.js"
> node test.js
node: dule.js" is not supported in NODE_OPTIONS
Activity
vsemozhetbyt commentedon May 11, 2017
Due to this issue, https://github.com/nodejs/node/blob/master/test/parallel/test-cli-node-options.js fails with spaces in the path to repo root and this cannot be fixed in the test.
cc @sam-github
sam-github commentedon May 11, 2017
I punted on this because I can't see any credible use-case for module names that have spaces in them. However, I didn't consider that Windows occaisonally forces/encourages top-level directory names with spaces in them.
This can be fixed in the test, its choosing to use absolute paths ATM, that's not necessary, relative paths would work fine. I'll rework the test, first.
@vsemozhetbyt do you think its really necessary to add a full-blown argv parser in C++ that understands
',"(and\',\")?vsemozhetbyt commentedon May 11, 2017
@sam-github Yes, it is not the name only, it is the same with spaces in any folder during the module path. I've just use space in the name for an easier repro.
Unfortunately, I do not know C++ and cannot understand all the implementation burden. But now we have a different behavior of cli
-rand NODE_OPTIONS-r. If we do not plan to complete this behavior, maybe we should document the difference.sam-github commentedon May 11, 2017
The difference is the shell, shells parse the quotes (not the node CLI), but with env vars there is no shell, so some subset of what the shell does would need to also be implemented by node. Which is doable.
vsemozhetbyt commentedon May 11, 2017
Well, maybe this is not worth it. I've just stumble upon this fixing #12773. Feel free to close if this is of lowest priority.
sam-github commentedon May 11, 2017
Its an issue, I'll look at it sometime. It would be more important if people run into it in practice, but I'm not sure they will. In your particular case, I can't even build node, much less run the tests, see #12773 (comment)
Having modules with spaces in their name
my module.jsis plainly weird, and its not necessary usually to provide full paths to modules (node -r /a/path/"with spaces"/module.js).Trott commentedon Mar 4, 2018
Should this remain open?
richardlau commentedon Jun 28, 2018
Probably, since #21575 has just been opened about the same issue.
arcanis commentedon Nov 3, 2018
It's likely this issue will appear a bit more. Yarn will use
NODE_OPTIONSto properly setup the PnP environment (at least until the package-level loaders are ready for consumption), meaning that people having spaces into their home directories might have problems.Note that imo supporting a full-blown parser with both
'and"isn't necessary. Supporting\would be enough to at least make it possible to add spaces, and would have a very simple implementation.arcanis commentedon Nov 3, 2018
I've opened #24065 with a fix.
src: allows escaping NODE_OPTIONS with backslashes
src: allows escaping NODE_OPTIONS with backslashes
src: allows escaping NODE_OPTIONS with backslashes
src: allows escaping NODE_OPTIONS with backslashes