Add "grunt-cli" package to "devDependencies" at package.json

[kirillrogovoy]

So contibutors don't have to install grunt globally to run tests which is a good tone.

[kirillrogovoy]

I feel like it also allows us to remove this line: https://github.com/less/less.js/blob/3.x/.travis.yml#L22
But I'm not quiet sure how to test it right.

[bd82]

grunt-cli is meant to be installed globally.

From my understanding: What grunt-cli does is add a grunt wrapper executable to the PATH. but this executable is just a wrapper, which seeks the specific executable for the specific project.

This means you can have many versions of grunt in different projects on the same machine
without version conflicts.

So if my understand is correct, adding the wrapper to a specific project (not globally) makes little sense.

as if it is not installed globally it won't be added to the global PATH.

[kirillrogovoy]

@bd82 There are two ways you can think about it:

1. Install all CLI binaries globally, install all code dependencies locally. It's fast and might be convenient when you and your team have convention about it and nobody minds installing required packages globally to work on the project.
2. Install everything locally. This means, you don't force your contributors to install any packages globally, which is good tone in open-source projects. Here's a README block from grunt-cli about this case. It's always convenient when you use npm scripts as the only entry point in your app via CLI. Anyway, you can always use the local grunt-cli dynamically like this npm run grunt -- [grunt_task].

My point is that it's not really kind to make your contributors install any global packages in order to make contributions. Should agree It's not that bad with grunt-cli, but it hurts when it comes to testing frameworks, linting tools, helper tools, etc. So, why to make exceptions?

[bd82]

you can always use the local grunt-cli dynamically like this npm run grunt -- [grunt_task].

This Does not work for me.
It complains about "missing" script grunt.

Afaik you must have the command you are trying to run listed in the package.json under the scripts section for npm run, which makes this a lot less convenient.

Anyway, I just checked and realized that npm install grunt does not provide the executable.
so installing grunt-cli locally is indeed good.

But to make it more useful I think the common development workflow items will have to be wrapped in npm scripts currently I only see an npm test script. in the package.json

or does "npm test" cover most development workflows for Less.js?

[kirillrogovoy]

This Does not work for me.

You can checkout my repo, run npm i, and then run npm test to ensure it actually works. Remember, that ./node_modules/.bin always gets automatically appended to your $PATH when running npm run [script]. See docs.

But to make it more useful I think the common development workflow items will have to be wrapped in npm scripts currently I only see an npm test script

We can always provide an NPM-way to run local grunt-cli like this:

// package.json
"scripts": {
  ...
  "grunt": "grunt"
}

Then run npm run grunt -- lint. No global modules required.

or does "npm test" cover most development workflows for Less.js?

Well, I don't think I can answer this question precisely right now. However, as far as I understand, you don't always need to run the whole npm test process (which takes about 1 minute on my computer btw), but only to run a specific grunt task. In this case, adding npm run grunt would be a good idea.

Anyway, applying this changes would satisfy both those who don't care about installing global modules and those who really do (like me). Even after these changes nothing stops you from using your own globally installed grunt-cli.

[bd82]

The trick with wrapping grunt in an npm script and passing additional argument looks nice. I will try it out later.

[kirillrogovoy]

I decided to make that alias as well.
Available here: 26e5583

[kirillrogovoy]

@bd82 Any updates on this.

What do core guys think? @matthew-dean

[matthew-dean]

Hmm... Maybe it's alright to have as a devDependency, but then you'd need to update Contributing docs to make sure it's clear how to run all the grunt tasks when grunt-cli is not global. If they're running as NPM commands, then those commands would need to be added. Simply adding "grunt": "grunt" to scripts is not sufficient, since there are so many parameters and sub-commands you can use when running tests or building that are defined in the Gruntfile.js.

Which ALSO means that we have to maintain both the Grunt task list and make sure those are all mirrored in the NPM "scripts" key.

So.... I dunno.... Seems like it could cause an additional maintenance burden, while running Grunt and Gulp as globals is pretty standard practice. I agree it's nice if packages don't require global modules, but in this case there's a lot more work that you probably haven't factored in than simply adding a devDependency.

Such as: many of the Grunt tasks are actually added programmatically. You can't add NPM script tasks programmatically (and I don't know that you can send parameters to them?), so I don't even think parity with the current Gruntfile is even possible.

[kirillrogovoy]

@matthew-dean Thanks for your thoughts. It seems to me that you could get my proposal all wrong.

"grunt": "grunt" to scripts is not sufficient, since there are so many parameters and sub-commands you can use when running tests or building that are defined in the Gruntfile.js.

I addressed this above, such as here: #2950 (comment).

I'm totally not suggesting maintaining both Grunt and NPM scripts. It would indeed be a burden. No, Grunt is the king here and NPM would be just a "proxy".

I just wanted to elaborate on the fact that, when you have grunt-cli installed locally, grunt [task_name_and_params] and npm run grunt -- [task_name_and_params] are virtually the same.

Thus, you don't need to replicate all the Grunt commands in the package.json. Basically, nothing else changes.

Moreover, you still can run grunt [task_name_and_params] if you have it installed globally, but you don't anymore force your contributors to install it, they can just clone the repo and start running Grunt tasks right away.

[matthew-dean]

@kirillrogovoy Okay, I didn't know you could send params to NPM scripts.

Can you open a PR for updating the Contributing in Less docs that reflects these changes? When that's done I'll merge both.

[kirillrogovoy]

Done.

Updated CONTRIBUTING.md in this PR and the docs here: less/less-docs#461.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[kirillrogovoy]

Please, don't! Merciless can.

[matthew-dean]

Sorry, I missed this.