scripts/hooks functionality added

[cgross]

This is a PR for the postinstall (and more) hooks as mentioned in #249.

One single commit, tests included, rebased on latest. I included a small bit of documentation in a separate HOOKS.md.

Let me know if you think anything needs work or updating.

Thanks.

[satazor]

Wow, thanks for the PR.
Will review it as soon as I got some time.

[wibblymat]

Good work! Haven't finished reviewing yet, but with #723 now being merged the build will fail on Travis because there are linting errors. Mostly just code style stuff. Run grunt test and you should get a big list of minor errors to correct.

I'll keep reading code in the meantime.

[cgross]

Thanks. I'll talk a look at the lint problems tonight and update.

I tried to make the changes to existing code pretty minimal. The only change of any significance to existing code was to Project.js where I had to put some existing code inside a then(). The diff makes it looks worse than it actually is. Otherwise, it was just adding new promises from the scripts.js to the promise chain where appropriate.

[cgross]

Added new commit for the lint issue fixes.

[cgross]

Any time for this PR? Really hope this stuff can go out in a release sometime soon. Thanks.

[satazor]

@cgross I think we have an issue that will be very hard to solve. Let's take this dep tree:

- app
  - foo
    - bar

As you see foo depends on bar and has a postinstall script that takes some of the bar css files to build its own css file. If bar is updated, the foo postinstall script won't be run again. This might be an edge case because ideally a postinstall script should only use files of itself and not from dependencies but it came to mind. And we could say, ok we can just run postinstall script of foo again. But if the postinstall script of foo changes the directory structure of the itself somehow, the next time it runs it will not have the expected directory structure (mout npm module) does that. Any suggestions?

[cgross]

@satazor Thanks for taking the time to look at this.

I think there's two use-cases for post_install and there was a bit of a debate about that in #249:

• Build artifacts in the just installed package (ex. compile some coffeescript or something like that - sounds like what you're talking about above).
• Update the project that the package was just installed in with references or configuration of the new package, primarily by using the main property (ex. inject script tags, update require.js config, etc)

I think those debating against post_install are worried about the first use-case and that its an anti-pattern in npm and we should not bring it over into bower. FWIW I tend to agree.

This PR attempts to solve the second use-case. I built bowinst to be used with this new feature to get that last mile out of bower. So the work and thinking about what kinds of hooks were necessary and when to call them was done thinking about how bowinst or a similar dependency wiring tool would need to work. For example, the configuration for the hooks is done in the .bowerrc in the project instead of bower.json.

tl;dr IMO its not a use-case that I think is worth supporting.

[cgross]

hi @satazor, any thoughts on this PR?

[satazor]

@cgross overall is good, I have not merged yet because I've not yet found some spare time to prepare 1.3.0 release. I've been focusing mainly in solving bugs in 1.2.x. Though, it will happen sometime soon.

[cgross]

Awesome. Thanks!

[svnlto]

this is pretty sweet!

[LaurentGoderre]

👍 for this! Can't wait for this to land

/cc @nschonni @masterbee

[sindresorhus]

👍

[sindresorhus]

@cgross can you fix the merge conflict?

[cgross]

Done. Also found and fixed a bug. Squashed the commits as well.

[nschonni]

Awesome 👍
Is there a reason that this setting is going in .bowerrc instead of the bower.json?

[benschwarz]

@nschonni bower.json is to define your package, .bowerrc configures bower for package consumption.

[nschonni]

OK, I was just used to the package.json for packaging settings and rc files being cumulative CLI settings 😄

[joefiorini]

Thanks for putting this patch together. It definitely solves a problem!

I agree with @nschonni. It is a little confusing since bower already matches npm in a lot of ways, using the same config key ("scripts") but putting it in a completely different file than npm seems like one of those decisions everyone will still be wondering about in a couple years.

[cgross]

They should take pause ... because the scripts in npm and bower are different and should not be confused.

NPM:
-Component author specifies scripts in his package.
-Scripts are run when any user installs his package in any project.

Bower
-Project (the thing consuming the components) author specifies the scripts when he's setting up his project.
-Scripts are run all packages installed in this project.

Different use-cases. Different implementations. Somewhat similar and could be confused.

I intentionally put the config in .bowerrc to try to make this more obvious.

@nschonni 's statement on package settings vs CLI settings is apt. These are settings for the user running bower not the package author or the package itself.

[itay]

@cgross: is there a way to get the npm style scripts? e.g. component author specified, rather than component consumer specified?

[cgross]

@itay No. For all the reasons talked about in this thread.

I'm interested to know what your use-case is for it? In NPM its commonly used to compile native code. Since nobody's distributing native code through bower...?

[joefiorini]

@cgross Thank you for explaining. Your logic definitely makes sense. Maybe the problem is just that the configuration looks so similar to the npm scripts configuration. Would naming the key something more indicative of this difference (e.g. "userScripts" or "tasks") make this clearer?

[nschonni]

I'm interested to know what your use-case is for it? In NPM its commonly used to compile native code. Since nobody's distributing native code through bower...?

I was looking to see if stuff like Sass and CoffeeScript could be complied, but this doesn't seem to be the use case here. In NPM stuff like a "dist" folder can be added when publishing, but since bower only does the clone, those types of files currently need to get checked it to be distributed.

[itay]

@cgross similar to what @nschonni said, but we also want to be able to distribute some aspects of the component outside of its containing directory possibly, and a script would make this easier to do. I realize it's a narrow and specific use case, but it would be nice to have such a feature.

[cgross]

Ping. Really hoping this can get merged sometime soon.

[Padam87]

👍

[tkfm-yamaguchi]

👍

[wibblymat]

Epic.

[cgross]

🎉 w00t