Restart script always runs stop and start #1999

Closed
laggyluke opened this Issue Jan 6, 2012 · 14 comments

Comments

Projects
None yet
8 participants

As per npm help scripts:

Note: npm restart will run the stop and start scripts if no restart script is provided.

But in reality this package.json:

{
    "name": "example",
    "version": "0.0.1",
    "scripts": {
        "start": "echo 'start'",
        "stop": "echo 'stop'",
        "restart": "echo 'restart'"
    }
}

results in this behavior:

$ node -v
v0.6.6
$ npm -v
1.1.0-beta-4
$ npm restart

> example@0.0.1 stop /tmp/example
> echo 'stop'

stop

> example@0.0.1 restart /tmp/example
> echo 'restart'

restart

> example@0.0.1 start /tmp/example
> echo 'start'

start
Contributor

TrevorBurnham commented Jun 9, 2012

I don't see any inconsistency. The note says "If no restart script is provided"; your package.json does define a restart script: echo restart'.

I assume that the expected behavior or npm restart should be:

  • If restart script is provided, just run it.
  • If restart script is not provided, run stop and start scripts instead.

And I don't really see a point in running stop, restart, and start all together.

What does this mean for the (pre/post)(stop/start) scripts?

I agree that npm restart shouldn't also call npm stop; npm start, but I don't know the existing use cases for said pre/post scripts.

$ node -v
v0.10.13
$ npm -v
1.3.4
$ npm restart

> example@0.0.1 stop /tmp/example
> echo 'stop'

stop

> example@0.0.1 restart /tmp/example
> echo 'restart'

restart

> example@0.0.1 prestart /tmp/example
> mkdir -p log


> example@0.0.1 start /tmp/example
> echo 'start'

start

@eliasgs eliasgs added a commit to nosco/nodeploy that referenced this issue Nov 12, 2013

@eliasgs eliasgs Use custom run-script instead of restart
The restart run-script executes both start, stop and restart
[see](npm/npm#1999). We just use a custom
run-script until this maybe gets fixed.
c6f9dde

+1

This still seems to be an issue. I would really like to see this fixed cause it would make things with pm2 much easer. pm2 restart pm2.json does absolutely not work with the current way npm restartworks.

The workaround with npm run-script rs or similar is a little much to type in my opinion.

Contributor

othiym23 commented Sep 24, 2014

This is by design and is intended to give developers flexibility about which lifecycle scripts they use to solve their particular problems. I agree that it doesn't map perfectly to how restart works in other tools, but getting this perfect is an infinite bikeshed, and npm is not actually the best place to solve this problem. Patches for documentation or a really good use case to say why e.g. npm should only run the restart script if it's provided might be considered, but in general this functionality is done. @cspiegl's solution might be verbose, but it's a good one if you're trying to use pm2 or forever with your application.

othiym23 closed this Sep 24, 2014

vyp commented Oct 2, 2014

@TrevorBurnham:

Note: npm restart will run the stop and start scripts if no restart script is provided.

So if a restart script is provided, it will still run the start and stop scripts? Then why have the note? (i.e. Why not just say npm restart always runs the npm start and stop scripts?) Sorry for the bump @othiym23, but I think I might be missing something here?

edit: https://www.npmjs.org/doc/cli/npm-restart.html:

This runs a package's "restart" script, if one was provided. Otherwise it runs package's "stop" script, if one was provided, and then the "start" script.

Contributor

othiym23 commented Oct 3, 2014

The code in lib/run-script.js is pretty straightforward:

function run (pkg, wd, cmd, args, cb) {
  var cmds = []
  if (!pkg.scripts) pkg.scripts = {}
  if (cmd === "restart") {
    cmds = ["prestop","stop","poststop"
           ,"restart"
           ,"prestart","start","poststart"]
  } else {
    cmds = [cmd]
  }
  if (!cmd.match(/^(pre|post)/)) {
    cmds = ["pre"+cmd].concat(cmds).concat("post"+cmd)
  }
  // ...
}

Restart has the potential to do a lot of stuff (in execution order):

  1. prerestart
  2. prestop
  3. stop
  4. poststop
  5. restart
  6. prestart
  7. start
  8. poststart
  9. postrestart

If you're building something that involves all of those lifecycle stages, I definitely want to know about it, because that's somewhere between awe-inspiring and deranged.

That "otherwise" in the documentation is definitely confusing, and I would welcome a patch to the docs with clearer wording. I would also love to hear people's use cases around restart, because while the current behavior is the most general, I can see wanting a restart script to replace start and stop scripts, rather than supplementing them.

othiym23 reopened this Oct 3, 2014

Contributor

othiym23 commented Oct 3, 2014

(reopening to remember to get the docs cleaned up)

One use case of restart centers around supervisors like pm2 and
forever. Both of these excellent supervisors have a restart-specific
command. If I'm trying to abstract what supervisor we happen to be using
behind npm, I'd like to be able to map npm run-script restart to pm2 restart (for example) without running pm2 start (mapped as the start
script) and pm2 stop (mapped as the stop script) as well.

Is my explanation of this use case clear? It's the reason I originally
chimed in; I was really surprised by the current behaviour.

Contributor

othiym23 commented Oct 3, 2014

@Schoonology Crystal clear. I'd like to find somebody who actually relies on the current behavior, because I agree that your use case is the most logical.

vyp commented Oct 4, 2014

If you're building something that involves all of those lifecycle stages, I definitely want to know about it, because that's somewhere between awe-inspiring and deranged.

No :). Merely trying to understand what seems to be an inconsistency between docs and behaviour.

I'd like to find somebody who actually relies on the current behavior, because I agree that your use case is the most logical.

Although I agree with @Schoonology's use case, this behaviour has existed already for quite some time and no doubt people have developed use cases which rely on the current behaviour. It's just a question of whether those use cases can adapt after any switch, or how good of a use case they are if they can't.

Contributor

othiym23 commented Oct 4, 2014

Although I agree with @Schoonology's use case, this behaviour has existed already for quite some time and no doubt people have developed use cases which rely on the current behaviour.

Yes, which makes changing this a breaking change / a candidate for deprecation + a new major version. That said, if it's not in use by many people, it's worth contemplating making the change.

othiym23 removed the easy label Oct 7, 2014

aripalo commented Oct 28, 2014

Just to weigh in:

I am also using npm scripts to abstract away as much "details" as possible. Because when you have many different kinds of NodeJS projects it's nice to have a "unified command line API" to use manage them. Otherwise the way npm's scripts work is really, really nice way of doing things, but that npm restart just frustrates me a lot.

Just to give an example, our current setup does away with basically 3 main commands:

  • npm start - install everything, run build steps and finally starts the server daemon (with forever)
  • npm stop - stops the server daemon
  • npm run development - actives development mode with gulpjs and file watching etc etc

Now only if npm restart would not repeat everything in npm start I could just map it to forever restart and have lighter & faster restart process.

othiym23 closed this in 2f6a1df Feb 27, 2015

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