kentcdodds / nps-utils

Utilities for http://npm.im/nps (npm-package-scripts)
https://doclets.io/kentcdodds/nps-utils/master
MIT License
101 stars 23 forks source link

Methods should use config path defined in package.json #13

Open ghost opened 7 years ago

ghost commented 7 years ago

While refactoring my package.json to use nps I haven noticed that methods like:

ignore custom config path provided via "nps --config". It would be extremely easy to check if env contains config path e.g. process.argv.slice(3,4) & reuse it while creating new nps call in following locations (nps-utils/src/index.js):

I'm happy to make pull request but due to #12 I'm not confident that this repo is ready to be forked ;-/

kentcdodds commented 7 years ago

This makes sense to me. Let's do it. Could you please respond to my question in #12? Thanks!

kentcdodds commented 7 years ago

Note, unfortunately this won't be as simple as calling slice because the options and scripts can appear in any order. We'll need to parse things via yargs.

ghost commented 7 years ago

@kentcdodds sure - that make sense. If you can check file I have provided for #12 & help with this I can quickly add this feature.

BTW. nps is such an amazing tool - showed that to mates from my team & they were impressed how easy you can clean package.json xD

kentcdodds commented 7 years ago

I'm glad you like it! I really enjoy it as well :)

RichDonnellan commented 7 years ago

@kentcdodds Looks like the @ghost account has been deleted. I'm facing the same dilemma and can't seem to set the custom --config within these methods in the meantime. Am I missing something? I'm getting:

Unable to find a config file and none was specified.

Code

const npsUtils = require('nps-utils');

// Reference to config file
const configFile = __filename;

compile: {
  description: `Compile Sass and JavaScript for development`,
  script: npsUtils.series.nps(`sass.dev --config ${configFile}`, `js.dev --config ${configFile}`)
}

I'm only able to get things working if I write things out manually:

nps sass.dev --config ${configFile} && nps js.dev --config ${configFile}

Thanks for making! 👏

RichDonnellan commented 7 years ago

I dug into the source code and was able to run my compile script after making this change:

series.nps = function seriesNPS(...scriptNames) {
  return series(
    ...scriptNames
    .filter(Boolean)
    .map(scriptName => scriptName.trim())
    .filter(Boolean)
-  .map(scriptName => `nps ${quoteScript(scriptName)}`),
+  .map(scriptName => `nps ${scriptName}`,
  )
}

It's getting confused by me having to redeclare my --config. Perhaps a band-aid fix could be to check for the presence of this flag before setting shouldQuote? I haven't the slightest idea how to proceed other than ditch the method approach.

kentcdodds commented 7 years ago

This will actually be mostly a non-issue when this happens (hopefully soon).

RichDonnellan commented 7 years ago

Will that issue also address the custom --config being ignored from the package.json? I have it set as so, but still need to address each script included in my package-scripts.js. I'd also think that I shouldn't have to prepend each script with nps since my start already includes it.

Do note that I'm not using nps globally, only as a local dependency.

// package.json
"scripts": {
  "start": "nps --config ./node_modules/package-scripts/package-scripts.js"
}

// package-scripts.js in custom location
const configFile = __filename;

script: sass.dev && js.dev // expected
script: `nps sass.dev --config ${configFile} && nps js.dev --config ${configFile}` // actual
kentcdodds commented 7 years ago

It wont, but doing that will make solving this issue easier.

RichDonnellan commented 7 years ago

Cool; thanks for the quick replies.

Do you want me to open an issue to address this within nps? It's still a problem outside of using nps-utils.

kentcdodds commented 7 years ago

This is already tracked here: https://github.com/kentcdodds/nps/issues/139