sezna / nps

NPM Package Scripts -- All the benefits of npm scripts without the cost of a bloated package.json and limits of json
MIT License
1.43k stars 93 forks source link

Revert 100 #139

Open kentcdodds opened 7 years ago

kentcdodds commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

kwelch commented 7 years ago

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 commented 7 years ago

Correct :+1:

kentcdodds commented 7 years ago

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

kwelch commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

FYI, I created https://github.com/yargs/yargs-parser/pull/130 to help with the parsing

wmertens commented 6 years ago

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

wmertens commented 5 years ago

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

sezna commented 5 years ago

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

wmertens commented 4 years ago

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?