Would this post-install hook break things if there is nothing to Gulp?

Out of the box it works fine.It should only fail if it can't find the gulpfile.js file or if there are no tasks specified.

The postinstall script isn't necessary but convenient. You want me to remove it?

I'm ok with it.

Great!

One thing though is that you're doing gulp --production after the install. If I'm working locally, I don't want that. It takes longer, and removes important debugging stuff. Plus, it minifies code unnecessarily.

This also assumes you have the correct node_moduels/bin aliasing setup. That doesn't happen out of the box on windows machines, so this will break installs for people.

@GrahamCampbell it should be fine. I think Node handles the bin aliasing by itself.

Node handles the bin aliasing by itself.

Doesn't work for me.

-1: I had this very idea in mind for weeks, and for the reasons listed here never proposed it. Yes, it looks shiny and all, but again, doesn't serve anything spectacular except pointing devs-not-familiar-with-node to 'scripts' config in package.json. IMHO gulp watch is more than enough.

Btw, I think it's good idea to move all those dependencies to devDependencies as it was done in this PR.

The whole point is to now having to install gulp globally since that requires an extra step during the installation. If someone doesn't want to use the scripts they can simply ignore them.

Who doesn't install gulp globally these days?! The point of that tool is to be handy globally, across many projects. Yet if you choose not to, that's good: then let's replace all global invocations with node_modules/.bin/gulp and such and be done with it.

Who doesn't install gulp globally these days?!

That's not a valid argument. We can't just say "f**k those people".

Hahaha, @GrahamCampbell :) I provided workaround for those people. You don't have to install anything globally anyway.

Who doesn't install gulp globally these days?!

This is correct, it is very common. Though, this updated doesn't force you to stop using you globally installed version of gulp.

One problem with using the globally installed version of gulp is that the version could differ from the one that the project requires. Gulp 4.0 is just around the corner and could introduce breaking changes. Lets say Laravel Elixir updates to Gulp 4.0 and you have version 3.0 installed globally on your machine. It could prevent from building your assets. With npm scripts you'll always have the same version of Gulp that the project requires.

(apologies if I am misunderstanding the problem, I am not very familiar with laravel)

npm solves the problem of absent global deps by putting the devDependencies into the path for npm scripts: https://docs.npmjs.com/misc/scripts

So if you declare gulp as a devDependency in package.json, the gulp binary will be available to your scripts whether gulp is installed globally or not. And because it's a devDependency you don't needlessly install gulp on your production machines.

This also resolves the issue of different projects needing different versions of gulp.

This also resolves the issue of different projects needing different versions of gulp.

Yes, it does. Though, it requires the project to use npm scripts.

@nhowell and instead of polluting your git repo, you pollute your prod server with tons of needless tools used for deployment?! That's even worse, having all that node, npm, compass blah blah tools, totally unrelated to a PHP-run software. What happened to CI/CD in software development workflow?

@vinkla I see that, but other entries remain for the scripts key. My point was when I suggested post-install asset compiling I was flat out dismissed, yet this Pull Request seems to have had constructive feedback and ultimately merged.

@vinkla That’s what I mean: I would have been fine if that was mentioned in my PR instead of going “nope”.

@shehi Yes, that's a fair point - The "right way" being to use continuous integration & deployment. In my case, I wasn't afforded that luxury, and instead began using a simpler deployment tool (e.g. like http://deployer.org/).

Anyway, if the official stance is to keep this change to devDependencies then I'll just modify it for my use case and move on.