js-dxtools / webpack-validator

Validates your webpack config with Joi
MIT License
295 stars 29 forks source link

Allow cli to be executed against arbitrary package.json scripts? #72

Closed bebraw closed 8 years ago

bebraw commented 8 years ago

Let's say I have package.json scripts declaration like this:

"scripts": {
  "stats": "webpack --profile --json > stats.json",
  "start": "webpack-dev-server",
  "deploy": "gh-pages -d build",
  "build": "webpack",
  "test": "karma start",
  "test:tdd": "karma start --auto-watch --no-single-run",
  "test:lint": "eslint . --ext .js --ext .jsx --ignore-path .gitignore --ignore-pattern dist --cache"
},

I would like to run webpack-validator cli against all of these (separate script) without having to spell each script explicitly.

My current tooling figures out which webpack configuration to run based on process.env.npm_lifecycle_event. I would need some neat way to run the validator while setting that against each script name.

How would you solve this? I could probably handle it outside of webpack-validator but some kind of integration might be cool as well. Then you would perhaps point the validator to package.json and it would just work.

If we can agree on a specification, I could implement it.

jonathanglasmeyer commented 8 years ago

Are we talking about all these scripts or just stats, start and build? I am a bit confused. :)

bebraw commented 8 years ago

It would have to validate stats, start, build, test, test:tdd. Those test parts are tricky as there's an indirect dependency. Basically how I would do it is roughly like this:

const pkg = require('./package.json');

Object.keys(pkg.scripts).forEach((script) => {
  // 1. set env now to match npm behavior
  ...  
  // 2. get configuration (nuke require cache for configuration require + require)
  ...
  // 3. validate now if an object was received
})

This sort of algorithm would work for an arbitrary scripts definition in my case.

nyrosmith commented 8 years ago

Is this behavior really in scope of webpack-validator? It sounds like a taskrunner for me and it would only work for npm-scripts. Why not use an existing taskrunner or just adding the CLI call to the scripts themselves?

bebraw commented 8 years ago

@nyrosmith Yeah, I could easily handle it on my own. That's the question really.

It depends on how people use the tool and how they configure webpack. Having a generic mechanism like this would be useful even if you don't rely on npm lifecycle and instead define a separate configuration file for each script. In any case we could end up with something like this:

"scripts": {
  "validate-webpack": "webpack-validator --all-npm-scripts"
}

You would then call npm run validate-webpack on pre-commit or pre-push hook or something like that.

I'm very curious to understand how people prefer to use this tool. This would be the ideal way to use the tool for me as I could keep the validator separate from my webpack configuration as it doesn't make sense to run it every time you run webpack.

jonathanglasmeyer commented 8 years ago

How should webpack-validator generally infer which webpack config the given script refers to?

nyrosmith commented 8 years ago

Sorry guys, now I got it! Right now I am on the misspelled Keys feature but after that I'll Start supporting Power users via .webpack- validatorrc. This would totally fit there

bebraw commented 8 years ago

How should webpack-validator generally infer which webpack config the given script refers to?

In my case it's just webpack.config.js always, but for the other one we would need to parse the filename from --config perhaps (if not set, assume webpack.config.js).

jonathanglasmeyer commented 8 years ago

That wouldn't work for cases where the webpack config is configured via NODE_ENV, would it? Generally I think the specific configuration setup can only be determined at runtime, which makes it a perfect fit for the programmatic approach. Analyzing script strings for pointers at which config is intended sounds quite brittle. :)

bebraw commented 8 years ago

Yeah, NODE_ENV would be yet another case to worry about. There are just so many ways to configure webpack. I suppose these three would cover majority of the cases, though.

kentcdodds commented 8 years ago

Generally I think the specific configuration setup can only be determined at runtime

Just a drive-by note. This is why we determined to not make this an ESLint effort.

jonathanglasmeyer commented 8 years ago

Tbh i can't quite imagine how you would even implement checking "over all npm scripts" - maybe it's best to discuss further on the basis of some prototype implementation?

bebraw commented 8 years ago

I can do a little standalone script, sure. We can then make some decisions based on that. It might take a day or two. Busy days.

jonathanglasmeyer commented 8 years ago

Sounds good. :+1:

bebraw commented 8 years ago

I wrote the script I promised. See https://gist.github.com/bebraw/436c2094afbfd08aed8609f000788bc4 .

It should handle the cases we discussed. The question is what to do with it.

jonathanglasmeyer commented 8 years ago

Ok, now I understand it much better. Well, it'll probably be really handy for 80 percent of people's setups. As long as we have an explicit description what it can and can't do in the README, I think we can put it on the cli, maybe with --all-scripts, --from-scripts or --scripts?

bebraw commented 8 years ago

--all-scripts would be cool, yeah.

The implementation takes a couple of further tweaks (need to do path.join against process.cwd() etc.) but I don't mind doing a PR.