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

Revert 100 #139

Open
kentcdodds opened this issue May 17, 2017 · 16 comments
Open

Revert 100 #139

kentcdodds opened this issue May 17, 2017 · 16 comments

Comments

@kentcdodds
Copy link
Collaborator

Ok, so #100 had good intentions, but it turns out that it's a little more frustrating to deal with things now that you have to wrap things in quotes. I don't know about anyone else, but it's been annoying for me. Also it's pretty rare that I want to run a series of scripts in a single nps invocation. And if I wanted to do that I could just do nps script1 && nps script2 for the rare cases I even want to do that. It's much more common for me to want to do: nps script1 arg1. So I'd like to basically revert what we did in #101, but with a slight change to things...

We still need to deal with this:

nps foo --silent --bar

Does the --silent flag get forwarded to the foo script or used as the --silent flag for nps? Here's my proposal:


nps foo --silent --bar

nps will forward --silent and --bar to foo. --silent will not be used as a flag for nps.

nps --silent foo --bar

nps will forward only --bar to foo. --silent will be used as a flag to nps.

nps --silent foo bar baz

nps will forward bar and baz to foo. --silent will be used as a flag to nps. This differs from the current situation in which nps will attempt to run three scripts serially: foo, bar, baz. As mentioned, it's not common to want to even do this. Making this change will make things much more usable/predictable.


I think that's pretty straightforward. I would love to see some emoji reactions to this before I start working on it...

@kentcdodds
Copy link
Collaborator Author

ping @kwelch who did some work to make things a little easier. Your work is still appreciated, but I think it will be unnecessary after these changes.

@kentcdodds
Copy link
Collaborator Author

That said, if anyone would like to work on implementing this, please, be my guest!

@kwelch
Copy link
Contributor

kwelch commented May 17, 2017

No worries. I actually thought about building a variation of this last night. I think this proposal works better than I was thinking.

To be clear, there will only ever be one script called? So usage would look like:
nps [--flags] (script|command) [script options/flags]

@kentcdodds
Copy link
Collaborator Author

Correct 👍

@kentcdodds
Copy link
Collaborator Author

I'm not sure how to accomplish this with yargs, but I hope it's straightforward!

@kwelch
Copy link
Contributor

kwelch commented May 17, 2017

Is yargs a requirement? I think using it for the simplified help and simple commands as valid, but the arguments come in as an array already and if order is key then I would map or reduce through the args making a single command. Lets see how the emoji vote goes and if there is enough in favor I can try to take a look at this.

@kentcdodds
Copy link
Collaborator Author

I like yargs a lot, but what you're saying makes sense. We don't need most of the features of yargs anyway. One thing that yargs made easier was autocomplete. However, that feature has been broken for some time now and nobody's complained, so I don't think that anyone's using it 😅

I wouldn't worry about the emoji vote count. I've been thinking that this would be better for a while. The more I think about it the more I think that this should happen anyway. Thanks for whatever you can do :)

@kentcdodds
Copy link
Collaborator Author

Ok, so I reeeally want this now. In fact I also really want to get the --parallel flag back. The ergonomics of nps took a bit of a downturn when I removed those two things. I'd like to get them back now. Anyone wanna work on this?

@kwelch
Copy link
Contributor

kwelch commented Jun 2, 2017

Sorry, I want to but I had a few things come up and have not been able to get to it.

If anything changes I will let you know. 👍

@istrau2
Copy link

istrau2 commented Nov 24, 2017

@kentcdodds Why not just use environment variables for this? I've been doing this:

server: {
     default: `webpack-dev-server -d --devtool '#eval' --inline --env.server ${process.env.REMOTE_BACKEND ? '--env.remoteBackend' : ''} ${process.env.PUBLIC_NETWORK ? '--env.publicNetwork' : ''}`,
     extractCss: `webpack-dev-server -d --devtool '#eval' --inline --env.server --env.extractCss ${process.env.REMOTE_BACKEND ? '--env.remoteBackend' : ''} ${process.env.PUBLIC_NETWORK ? '--env.publicNetwork' : ''}`,
     hmr: `webpack-dev-server -d --devtool '#eval' --inline --hot --env.server ${process.env.REMOTE_BACKEND ? '--env.remoteBackend' : ''} ${process.env.PUBLIC_NETWORK ? '--env.publicNetwork' : ''}`
}

The environment variables are automatically passed through the process (and they can be used in multiple steps that are run in series or parallel).

@wmertens
Copy link

wmertens commented Aug 14, 2018

Another thing that would be useful is having the parsed arguments available in the package-scripts.js file, for example

nps --silent build.doc --fast

Could define a global nps variable that package-scripts.js can use to do some setup, or skip extra work done to define the scripts

global.nps = {script: 'build.doc', args: ['--fast'], options: {verbose: 0}}

@wmertens
Copy link

FYI, I created yargs/yargs-parser#130 to help with the parsing

@wmertens
Copy link

That PR seems like it will have to wait a while though - last activity in yargs-parser was june :-(

@wmertens
Copy link

In the meantime the PR got merged and yargs-parser has halt-at-non-option :)

@sezna
Copy link
Owner

sezna commented Aug 28, 2019

@wmertens would you like to take the lead on this? 😄

@wmertens
Copy link

wmertens commented Feb 4, 2020

To do this in a backwards-compatible way, how about having to set a flag in package-scripts.js?

E.g. if the returned object contains _passArgs: true, it works as explained here, otherwise retain the current behavior?

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

No branches or pull requests

5 participants