Skip to content
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

Try to catch "npm run" arguments without "--" #3470

Closed
bahmutov opened this issue Feb 14, 2019 · 4 comments
Closed

Try to catch "npm run" arguments without "--" #3470

bahmutov opened this issue Feb 14, 2019 · 4 comments
Labels
cli stage: proposal 💡 No work has been done of this issue stale no activity on this issue for a long period

Comments

@bahmutov
Copy link
Contributor

Sometimes our users forget that NPM run script needs command line arguments to the tool separated from the NPM's own arguments with --. For example, see #3467

We have a warning in our docs about it, and should put the warning in more places cypress-io/cypress-documentation#1418 but I think we can help users even in the current situation.

Here is how. Imagine we have index.js that just prints command line arguments

console.log('all args', process.argv)

and we have NPM script in package.json

{
  "scripts": {
    "test": "node ."
  }
}

Here is what happens when running script directly vs through npm run test

Calling directly with arguments

$ node . --foo bar
all args [ '/Users/gleb/.nvm/versions/node/v10.13.0/bin/node',
  '/Users/gleb/git/training/node/test-npm-params',
  '--foo',
  'bar' ]

Calling via NPM script npm test

{
  "scripts": {
    "test": "node ."
  }
}
$ npm test --foo bar

> test-npm-params@1.0.0 test /Users/gleb/git/training/node/test-npm-params
> node . "bar"

all args [ '/Users/gleb/.nvm/versions/node/v10.13.0/bin/node',
  '/Users/gleb/git/training/node/test-npm-params',
  'bar' ]

So we are losing "run" command.

But

If we prince process.env we can find the original command line arguments in NPM variable npm_config_argv passed as a string

npm_config_argv:
   '{"remain":["bar"],"cooked":["test","--foo","bar"],
   "original":["test","--foo","bar"]}',

Idea

In our Cypress CLI module, whenever we get an unknown command (because instead of open or run we see something like --record), we take a look at npm_config_argv original list of arguments, load user's package.json, if the first argument matches the script name like "test" we know the user forgot to add --, and in that case we can take the rest of the npm_config_argv original arguments as the real CLI arguments the user meant to use.

@cypress-bot cypress-bot bot added the stage: needs information Not enough info to reproduce the issue label Feb 14, 2019
@brian-mann
Copy link
Member

Yarn also does this by default, it doesn't need the -- and we could actually do the same without even warning the user.

@bahmutov
Copy link
Contributor Author

I would still warn the user since this so custom and against what the NPM does, they should be aware that this is non-standard feature we are doing.

@cypress-app-bot
Copy link
Collaborator

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

@cypress-app-bot cypress-app-bot added the stale no activity on this issue for a long period label May 18, 2023
@cypress-app-bot
Copy link
Collaborator

This issue has been closed due to inactivity.

@cypress-app-bot cypress-app-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli stage: proposal 💡 No work has been done of this issue stale no activity on this issue for a long period
Projects
None yet
Development

No branches or pull requests

4 participants