Allow start command to be configurable

[ryanmoran]

Describe the Enhancement

The buildpack currently only detects in the case that it sees a start script. The buildpack should allow users to have their start script be something other than start either through configuration or more permissive detection logic.

Possible Solution

Should we detect on other common script names, like serve?

Or should we include an environment variable like BP_NPM_START_SCRIPT=serve?

Motivation

Sometimes developers have good reason to not use start as the command they want their image to run at boot. For instance, the start command might be what you run locally to boot a development server while serve might be what is expected to run in production.

[mhdawson]

If it detects on other common names then there would have be a defined priority or other way to choose the one to run right?

I suspect BP_NPM_START_SCRIPT might still be necessary to handle cases where the defined priority does not match what the developer needs. Starting by adding BP_NPM_START_SCRIPT might therefore be a good first step.

[benosman]

Having BP_NPM_START_SCRIPT would be something that I would find very handy.

[mhdawson]

@ryanmoran if using just BP_NPM_START_SCRIPT makes sense to you and I can put it on my list of TODOs to add support for it. I'm on holiday for a few weeks but would be able to look at it after that.

[ryanmoran]

@mhdawson I think the implementation should include BP_NPM_START_SCRIPT in any case. I'd be happy with a PR that only added that and ignored the whole priority lookup list concept.

[mhdawson]

This PR includes an update to libnodejs which adds support for BP_NPM_START_SCRIPT - paketo-buildpacks/libnodejs#9

I don't think any change will be need in npm-start. I would plan to add one or more tests to npm-start but #331 will need to land first as it makes npm-start use the shared function in libnodejs.

[loewenstein]

I am wondering if this has been done meanwhile and should be closed? Did we document the new environment variable already?

[loewenstein]

Another thought though, if I recall correctly we don't actually use npm run for this - pointing out that npm isn't supposed to be used in production.
If that is the case, how many users actually use npm scripts at all to run their apps in production?

This is with the background that I recently realized how the order of npm-start and node-start can lead to subtle differences when e.g. the start command is start: node app.js <param>. Both buildpacks will end up running app.js but one with and the other without parameter...
How can we be sure the buildpack magic is helping more than it adds confusion?

[mhdawson]

@loewenstein good point about the documentation, I think the option is in place. I'll add that to my tody list.

I agree we should help people avoid using npm to run their applications, lots of people do, but its not recommended. When I look at adding the doc for the option, I'll take a closer look at what is done in the buildpacks and see if I have any suggestions.

[mhdawson]

This dropped of my radar, will try to get back to it.

[mhdawson]

Confirmed it is already documented here - https://github.com/paketo-buildpacks/npm-start#specifying-a-custom-start-script

and there was one integration test that used BP_NPM_START_SCRIPT, but created PR to add one to the unit tests as well - #562

With that I think we can close this as complete.