Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

npm run removes quotes from given parameters #7743

Closed
mantoni opened this issue Mar 25, 2015 · 5 comments
Closed

npm run removes quotes from given parameters #7743

mantoni opened this issue Mar 25, 2015 · 5 comments

Comments

@mantoni
Copy link
Contributor

mantoni commented Mar 25, 2015

I'm using npm as a build tool and ran into the following issue with npm run and quoted parameters.

This is the interesting part of the "scripts" section in my package.json:

"scripts": {
  "test": "mochify",
  "cover": "npm run test -- --plugin [ mochify-istanbul --report cobertura --exclude '**/*.mustache' ]"
}

Running npm run cover prints this:

> npm run test -- --plugin [ mochify-istanbul --report cobertura --exclude '**/*.mustache' ]
...
> mochify --plugin [ mochify-istanbul --report cobertura --exclude **/*.mustache ]

You can see that **/*.mustache is quoted on the first line, but the quotes are removed on the second line. This causes the script to fail because I have to pass the glob expression as a plain string.

If I change the cover script to this:

"cover": "mochify --plugin [ mochify-istanbul --report cobertura --exclude '**/*.mustache' ]"

... calling npm run cover works fine and npm prints the quotes as expected:

> mochify --plugin [ mochify-istanbul --report cobertura --exclude '**/*.mustache' ]

Thanks for looking into it!

@smikes
Copy link
Contributor

smikes commented Mar 29, 2015

Thanks for reporting this.

This is known behavior, though I don't know if there's another issue for it already. It happens because npm run retrieves the args after -- from the command line after the shell has unescaped them, and concatenates them without quotes.

I'm not sure there's a general way to solve the problem, though, as the quotes are gone by the time npm run can see the arguments, and it's not obvious to me that there's a safe way to re-quote them since an argument may have quotes in it due to escaping, or glob expansion, etc.

EDIT: to add
You seem to have found a functional workaround. Is that good enough for the present?

@mantoni
Copy link
Contributor Author

mantoni commented Mar 29, 2015

Well, the workaround is to not use the feature if I have command parameters that need to be quoted :)

I had a look and as a quick and dirty hack I replaced this line with this:

joinedArgs += " '" + arg + "'"

It fixes this specific issue, but I'm not sure whether this works as a valid solution in general or whether it will introduce other issues.

Thoughts?

@smikes
Copy link
Contributor

smikes commented Mar 29, 2015

Just off the top of my head, It will cause an error in the shell if the arg before shell expansion is * and the directory contains a file containing single-quote (eg., named Don't read me.md)

So that's probably not a viable long-term fix.

The workaround you have found is to denormalize the commands, repeating mochify in your cover command:

"scripts": {
  "test": "mochify",
  "cover": "mochify --plugin [ mochify-istanbul --report cobertura --exclude '**/*.mustache' ]"
}

That way the arguments don't get passed through npm run twice. The downside is that if you change your tests to use a different runner, you have to maintain two lines in your package.json.

@kenany
Copy link
Contributor

kenany commented Mar 29, 2015

Oddly enough, this results in no quotes being removed:

{
  "scripts": {
    "test": "mochify",
    "cover": "mochify --plugin [ mochify-istanbul --report cobertura --exclude \"'**/*.mustache'\" ]"
  }
}
$ npm run cover

> @ cover
> npm run test -- --plugin [ mochify-istanbul --report cobertura --exclude "'**/*.mustache'" ]


> @ test
> mochify --plugin [ mochify-istanbul --report cobertura --exclude "'**/*.mustache'" ]

@mantoni
Copy link
Contributor Author

mantoni commented Mar 30, 2015

@kenany You're just doing the escaping for npm ;-)

@smikes Good point. If an argument contains ' it should be escaped with \', just like " is escaped here. I'll make that a PR and see whether I can come up with some tests for that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants