Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bin into local grunt #943

Closed
shama opened this issue Oct 16, 2013 · 18 comments
Closed

Add bin into local grunt #943

shama opened this issue Oct 16, 2013 · 18 comments
Milestone

Comments

@shama
Copy link
Member

shama commented Oct 16, 2013

Reference TryGhost/Ghost#1053

I think it would be nice if npm install grunt included a bin script. Then Grunt users could recommend npm install && npm test without explaining the additional step to install the grunt cli globally and have the package dependencies completely self contained.

@vladikoff had the good idea of just adding grunt-cli as a peer dependency to Grunt.

The current solution is to npm install grunt-cli locally and reference in the package.json with:

{
  "scripts": {
    "test": "grunt test"
  }
}

in which npm will find the local node_modules/.bin/grunt binary installed by grunt-cli.

I think at the very least, this message https://github.com/gruntjs/grunt-cli#installing-grunt-cli-locally should be removed and a completely local install method should be officially supported.

@chrisenytc
Copy link

I agree with this. It would be really interesting this feature. 👍

@vladikoff
Copy link
Member

@shama and me had an IRC discussion regarding this.

A few reasons for this:

  1. A lot of node projects can get up and running with just npm install && npm start or get tests running using npm install && npm test. But if the tests rely on Grunt, then grunt-cli has to be present globally.
  2. node.js hosting and CI platforms, such as Nodejitsu, Travis CI and Heroku currently need to have grunt-cli either in developer dependencies or installed locally somehow.

If can add this npm start functionality I think it will be really useful, especially to node.js developers. This doesn't change the current global grunt-cli behavior. The local grunt-cli will ONLY work inside the current project.

Q: Why not just install grunt-cli locally?
A: 1) Feels wrong, you don't get CLI features this way. 2) Grunt is already in dev dependencies, why can't it take care of it.

@cowboy thoughts? (also see TryGhost/Ghost#1053 ) There could be different ways to achieve this functionality, peer dependency was a rough idea.

(also really happy to see that @chrisenytc is interested in this feature)

@cowboy
Copy link
Member

cowboy commented Oct 16, 2013

Ok. I really like the idea of the grunt lib including a barebones bin for idiomatic Node.js npm test usage and grunt-cli being globally installable for a grunt command in the path with shell autocompletion rules. It seems like a legitimate pattern to me.

Keep in mind that we'll have to tell people they have a few ways of running Grunt:

  1. via a globally installed grunt-cli module
  2. via alias grunt=./node_modules/.bin/grunt for bash users
  3. via function grunt() { "$(npm bin)"/grunt "$@"; } for smarter bash users
  4. via DOSKEY grunt=.\node_modules\.bin\grunt $* for Windows users

Notes:

  • 1 allows you to receive more user-friendly errors and allows shell autocompletion
  • 1 and 3 allow you to run grunt from any subdirectory of your project
  • 2, 3, 4 won't work while devving grunt itself, since the bin path will be different

Other possible caveats here:

  • If the user has both grunt and grunt-cli already installed in a project (which some people do), I believe they will get an npm error because 2 libs have the same named bin. This might require us to wait until 0.5.0 to make this change.
  • What happens when someone tries to install grunt globally? I know we can have npm warn, but is there any way for Grunt to prevent itself from being installed globally or fail to run if installed that way? support nightmare?!
  • The "Getting Started" guide may need to be restructured to better explain how to "get the grunt command itself".
  • We'll need very clear migration guide notes regarding this change.
  • We'll need to find a place to explain what npm install && npm test means. Maybe in a "Continuous Integration with Grunt" docs page.

Does the grunt bin need to be anything more than this?

#!/usr/bin/env node

'use strict';

require('../lib/grunt').cli();

@shama
Copy link
Member Author

shama commented Oct 16, 2013

If the user has both grunt and grunt-cli already installed in a project (which some people do), I believe they will get an npm error because 2 libs have the same named bin. This might require us to wait until 0.5.0 to make this change.

I just tested this on both v0.8 and v0.10. If grunt-cli is locally and globally installed, typing grunt will run the global grunt-cli. Typing npm test to run "scripts": { "test": "grunt test" } will run the locally installed grunt-cli.

What happens when someone tries to install grunt globally? I know we can have npm warn, but is there any way for Grunt to prevent itself from being installed globally or fail to run if installed that way? support nightmare?!

AFAIK, this is unavoidable. Even with the current way grunt is built, people can place any module into a global or semi-global folder and node will find it: http://nodejs.org/api/modules.html#modules_loading_from_node_modules_folders

Maybe a note on the getting started guide instructing people to avoid global setups would be a good idea.

@tkellen
Copy link
Member

tkellen commented Oct 16, 2013

It is possible to detect that a module is being installed globally & force it to fail. I've never seen it in the wild, but it can be done.

@nschonni
Copy link

Maybe a preinstall script that sniffs out the NPM params. @shama's point is true that anyone determined would be able to get around this.

@tkellen
Copy link
Member

tkellen commented Oct 16, 2013

we had a pre-install script for a few seconds that did this:

if (process.env.npm_config_global === 'true') {
  console.error('###################################################################\n');
  console.error('Please use `npm install -g grunt-cli` to install the grunt command.');
  console.error('Grunt itself must be defined as a local dependency of your project.');
  console.error('For more information, read this: http://bit.ly/fyW0Fo\n');
  console.error('###################################################################\n');
  process.exit(1);
}

@cowboy
Copy link
Member

cowboy commented Oct 16, 2013

I wish @isaacs could weigh in on this.

@vladikoff
Copy link
Member

Keep in mind that we'll have to tell people they have a few ways of running Grunt:

  1. via a globally installed grunt-cli module
  2. via alias grunt=./node_modules/.bin/grunt for bash users
  3. via function grunt() { "$(npm bin)"/grunt "$@"; } for smarter bash users
  4. via DOSKEY grunt=.\node_modules.bin\grunt $* for Windows users

Method 1 is for beginners and basic usage. 2,3,4 are advanced / custom usages

If the user has both grunt and grunt-cli already installed in a project (which some people do), I believe they will get an npm error because 2 libs have the same named bin. This might require us to wait until 0.5.0 to make this change.

I just tested this on both v0.8 and v0.10. If grunt-cli is locally and globally installed, typing grunt will run the global grunt-cli. Typing npm test to run "scripts": { "test": "grunt test" } will run the locally installed grunt-cli.

If there are not edge cases to this, then this sounds good

What happens when someone tries to install grunt globally? I know we can have npm warn, but is there any way for Grunt to prevent itself from being installed globally or fail to run if installed that way? support nightmare?!

@tkellen's script should take care of it. It might make sense to be able to force it, but I'm not sure why would anyone really need to have it globally. The goal here is to prevent mistakes for those who don't read the install docs.

We'll need very clear migration guide notes regarding this change.

What migration though? Local CLI should be a feature that doesn't affect the current global CLI setups in any way

We'll need to find a place to explain what npm install && npm test means. Maybe in a "Continuous Integration with Grunt" docs page.

👍

@cowboy
Copy link
Member

cowboy commented Oct 16, 2013

@vladikoff some people have already installed grunt-cli locally beside grunt, even though it is not supported. Those people will need to remove grunt-cli from their project. It's not something we support, but I believe enough people do it that it warrants an explanation in a migration guide.

@cowboy
Copy link
Member

cowboy commented Oct 16, 2013

Because adding a bin to grunt will cause errors when npm install is run in a project that contains both grunt and grunt-cli, even though that is not supported... it seems like this should be an 0.5.0 change.

@sindresorhus
Copy link
Member

Huge 👍

@tkellen
Copy link
Member

tkellen commented Nov 5, 2013

If you make grunt-cli a dep (or devDep) of Grunt, the grunt bin will be in the environment for things executed from package.json scripts entries (e.g. npm test & npm start) but not available for general use when installed globally. This seems like the best of both worlds, but it could definitely introduce confusion without the right documentation.

@tkellen
Copy link
Member

tkellen commented Nov 5, 2013

Hah, ignore me, that's basically what @shama said in the OP. Signing off now!

@remy
Copy link

remy commented Jan 7, 2014

+1 would like to see this land. Seems weird to install grunt-cli in my package deps.

@vladikoff vladikoff removed this from the 0.5.0 milestone Jan 19, 2016
@vladikoff
Copy link
Member

@JKAussieSkater this is gonna be a change for the "docs" if #1438 lands, for your information.

@shama
Copy link
Member Author

shama commented Jan 30, 2016

This is done on fb9dd08. When grunt-cli@1.0.0-rc1 is released, I'll ask @vladikoff to update his PR GH-1438 to point to that version.

@shama shama closed this as completed Jan 30, 2016
@JKVeganAbroad
Copy link

@vladikoff, noted!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants