Add tests for spaces in folder names

[PaulBags]

This file of npx uses __dirname. On windows paths are being split on spaces (as of current node.js LTS release), leading to issues like this (which still isn't resolved, despite dev protest).

See below for a log of this issue while attempting to run the react.js tutorial command npx create-react-app my-app (this error occurs both inside and outside the provided node.js cmd environment, with or without adminministrative permissions).

I therefore propose adding a test for spaces in directory names.

0 info it worked if it ends with ok
1 verbose cli [ 'C:\\Program Files\\nodejs\\node.exe',
1 verbose cli   'C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js',
1 verbose cli   'install',
1 verbose cli   'create-react-app@latest',
1 verbose cli   '--global',
1 verbose cli   '--prefix',
1 verbose cli   'C:\\Users\\User',
1 verbose cli   'Name\\AppData\\Roaming\\npm-cache\\_npx\\3672',
1 verbose cli   '--loglevel',
1 verbose cli   'error',
1 verbose cli   '--json' ]
2 info using npm@6.4.1
3 info using node@v10.15.3
4 verbose npm-session dfbc7ea6f6a9f350
5 silly install loadCurrentTree
6 silly install readGlobalPackageData
7 silly fetchPackageMetaData error for file:Name\AppData\Roaming\npm-cache\_npx\3672 Could not install from "Name\AppData\Roaming\npm-cache\_npx\3672" as it does not contain a package.json file.
8 http fetch GET 304 https://registry.npmjs.org/create-react-app 391ms (from cache)
9 silly pacote tag manifest for create-react-app@latest fetched in 403ms
10 timing stage:rollbackFailedOptional Completed in 1ms
11 timing stage:runTopLevelLifecycles Completed in 529ms
12 verbose stack Error: ENOENT: no such file or directory, open 'c:\react_test\Name\AppData\Roaming\npm-cache\_npx\3672\package.json'
13 verbose cwd c:\react_test
14 verbose Windows_NT 10.0.15063
15 verbose argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "install" "create-react-app@latest" "--global" "--prefix" "C:\\Users\\User" "Name\\AppData\\Roaming\\npm-cache\\_npx\\3672" "--loglevel" "error" "--json"
16 verbose node v10.15.3
17 verbose npm  v6.4.1
18 error code ENOLOCAL
19 error Could not install from "Name\AppData\Roaming\npm-cache\_npx\3672" as it does not contain a package.json file.
20 verbose exit [ 1, true ]

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[Trott]

On windows paths are being split on spaces (as of current node.js LTS release)

I ran this on all supported Node.js releases and got the same expected result.

path.win32.dirname('c:\\foo bar\\baz'); // returns 'c:\\foo bar'

[Trott]

/ping @PaulBags I don't understand what you're trying to say here. Can you please clarify? Are you suggesting there's a bug in some currently-supported versions of Node.js with regard to path.dirname() on Windows? If so, can you be explicit about what it is? If not, can you explain how adding these tests to Node.js will help fix bugs in other applications?

[PaulBags]

The version of npx that ships with node LTS on windows is splitting paths on spaces, paths that it decides itself so I can't fix them by wrapping in quotes, and appears to be using __dirname from node to do so.

The author of npx claimed this was fixed, with zero proof - the project has since been marked read only and appears to have been taken over by node.

If the problem isn't here then I don't where to look, but yes it is currently supported node.

[Trott]

appears to have been taken over by node.

Nope. It is handled by npm, not node.

If the problem isn't here then I don't where to look, but yes it is currently supported node.

The repo to use is https://github.com/npm/npx. (They have the issue tracker turned off probably because they use https://npm.community/ as their issue tracker.)

All that said...

Your output above says you're running node 10.15.3 and npm 6.4.1. Can you upgrade to node 10.16.0 and then update npm to 6.10.2. (npm install -g npm should do the trick.) With those updates, does the problem persist?

[Trott]

Although the explanation for why these tests are being added is (I believe) not quite correct, there's no reason not to test paths with spaces, so this LGTM. Whoever lands this (probably me) will want to update the commit message.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/24822/

[Trott]

Landed in 1ddd46f.

Thanks for the contribution! 🎉