fix(#99): proof of concept

[hgwood]

JSON.stringify does the job of quoting and escaping. What do you think? Please submit any case you might think of.

ping @ebriand

[kentcdodds]

This looks good, but I'm not certain. Perhaps someone could publish a temporary scoped package of this or something and have people test it out? I just want to be sure this will solve the problems we found before we release a new breaking version!

Thanks so much for this @hgwood!

[hgwood]

I extracted this into a function but forgot to push. Pending.

[hgwood]

I need to add a case where the command itself has spaces. It shouldn't be quoted. Also a case where the command has quotes. I need to verify would the outcome should be.

[codecov-io]

Codecov Report

Merging #102 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #102   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           3      3           
  Lines          42     43    +1     
=====================================
+ Hits           42     43    +1

Impacted Files	Coverage Δ
src/index.js	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6994a24...d7b9aab. Read the comment docs.

[hgwood]

This is published as @hgwood/cross-env@fix99. @ebriand, care to try it?

[hgwood]

Looking back at #100, I'm having doubts. The command there is (ignoring env setters) "cross-env nyc mocha --require test/helper.js \"test/spec/**/*.test.+(js|jsx)\"". So cross-env gets ['nyc', 'mocha', '--require', 'test/helper.js', 'test/spec/**/*.test.+(js|jsx)']. How can cross-env know that the user wishes that this last argument be quoted? Maybe they'd like it to be expanded. cross-env can't know, can it? Am I missing something?

[hgwood]

Maybe we should not use the shell option after all. Thinking about it, the OS shell processes the user's command before running cross-env. Then using the shell option in cross-env triggers another pass, when the user would expect only one.

[kentcdodds]

I'm kinda thinking that as well @hgwood... So how can we solve the original problem? Should we use a different package to spawn our commands?

[hgwood]

I need to investigate more but I'm guessing the reason cross-spawn can't do what we want (that is, quoted scripts) is because it escapes "dangerous" commands. "dangerous" is defined as not matching /^[a-z0-9_-]+$/i. cross-env needs the command not to be escaped. My question now is: why does cross-spawn have this protection? Do you know?

[kentcdodds]

I don't know :-/

[hgwood]

Possibly interesting. It says exec would be more appropriate. But exec has the drawback of buffering stdio (see here). So yes maybe we need to replace cross-spawn with something else.

[hgwood]

Or maybe the shell option should only be enabled if the command passed to cross-env is a quoted script? Following question is how to detect a quoted script? Maybe it contains spaces.

[kentcdodds]

Yeah, definitely don't want to use exec due to the buffering stdio thing. What about switching to spawn-command like I did in #77? Would that work?

[hgwood]

I have looked at the spawn-command code and as far as I can tell it does the same thing as the shell option.

[hgwood]

Here is how I see it right now.

As the user of cross-env, I want one of two things.

• Either my command is not quoted, and I want the shell to process it one time.
• Or my command is quoted, and I want the shell to process it twice (one time to remove the quotes and one other to actually run it).

When I say "command" here, I'm excluding arguments. For example:

• In echo "a", echo is the command and it's not quoted.
• In "echo a", echo a is the command and it's quoted.

If I'm correct, what cross-env should do is enable shell only in the second case. Now how can cross-env decide which case it is running? I'm not sure.

[hgwood]

Maybe cross-env could have 2 bins? One for each case. That would let the user decide and cross-env would not have to guess (and potentially fail).

[hgwood]

Having 2 bins would also avoid the breaking change :). The cross-env bin would retain its current behavior, and we would have another bin for users who want less escaping in their commands. Maybe it could be named cross-env-raw or something of the like.

[kentcdodds]

I like the idea of 2 bins. But I want to preserve the original use-case with the main bin:

cross-env NODE_ENV=production webpack --config build/webpack.config.js

So, as long as we can preserve that then I'm good.

[hgwood]

Just to be sure we're on the same page, this particular use case would work with both bins. Differences show up only when there are escape sequences or quotes.

I agree the "right" thing would be to have to main bin do the v3 behavior and to other one the v4. What I was afraid of is that some users that have or will upgrade to v4 by changing their scripts, then will discover they have to revert everything for v5, but actually they would "just" have to use the other bin. Still a bit annoying. To avoid this situation, I suggest that we deprecate v4 on npm so that people don't upgrade to it and wait for v5. What do you think?

[kentcdodds]

Closed in favor of #104