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

added functional-scripts feature #197

Closed liron-navon closed 5 years ago

liron-navon commented 5 years ago

multiple files have been changed, but 6 of them are only for tests and documentation that I added. What**: Added a feature that allows functions with scripts, this allows us to call scripts like this:

{
  myScript: ([message]) => `echo "hello ${message}"`,
  myOtherScript() {
    console.log('myOtherScript is called');
    return 'echo "hello myOtherScript"';
  },
 getTime: function() {
   const date = new Date();
   return 'echo "${date.toISOString()}"';
 }
}

Why: I wanted this feature implemented, I need to be able to log stuff into a file, or change the command dynamically, and using getters make them being called multiple times (every time the value is checked/cloned). I believe there is a demand for this feature based on multiple issues: https://github.com/kentcdodds/nps/issues/196 https://github.com/kentcdodds/nps/issues/184 https://github.com/kentcdodds/nps/issues/165 https://github.com/kentcdodds/nps/issues/62 https://github.com/kentcdodds/nps/issues/6

How: I added a function that resolves a string or a function as a script:

function resolveFunctionalScript(scriptToRun, args) {
  if (!isUndefined(scriptToRun) && isFunction(scriptToRun.script)) {
    scriptToRun.script = scriptToRun.script(args)
  }
  return scriptToRun
}

And a check to resolve-script-object-to-string.js, the name will now be confusing, so I'm thinking of changing it to resolve-script-object-to-script.js but I didn't since I don't want to do a large scale refactor wherever it's used currently.

else if (isString(script) || isFunction(script)) {
    return {script}
  }

I also added a third parameter to getScriptToRun so it can pass arguments to the functions that result to a runnable script. I also added a new argument for the CLI, named "args" or "-a" so users can pass arguments to the function (calling it "props" might be better?).

I also thought if I should spread the arguments into the function instead of passing them as an array - which will result in a much nicer api, but I don't want to break functionality for older versions of node, so it could be something like this:

scriptToRun.script = scriptToRun.script(...args)

And called like this:

myScript: (message) => `echo "hello ${message}"`

Checklist:

codecov[bot] commented 5 years ago

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #197   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines         389    390    +1     
  Branches       94     95    +1     
=====================================
+ Hits          389    390    +1
Impacted Files Coverage Δ
src/resolve-script-object-to-string.js 100% <100%> (ø) :arrow_up:
src/get-script-to-run.js 100% <100%> (ø) :arrow_up:

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 a0c961a...4509c56. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #197   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines         389    394    +5     
  Branches       94     95    +1     
=====================================
+ Hits          389    394    +5
Impacted Files Coverage Δ
src/bin-utils/parser.js 100% <ø> (ø) :arrow_up:
src/get-script-to-run.js 100% <100%> (ø) :arrow_up:
src/resolve-script-object-to-string.js 100% <100%> (ø) :arrow_up:
src/index.js 100% <100%> (ø) :arrow_up:

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 a0c961a...f1916a2. Read the comment docs.

aparajita commented 4 years ago

Hey @liron-navon, https://github.com/Mehuge/nps-plus implements what we both want, supports Promises too!