fix(vite): close server and exit if stdin ends

[nullpilot]

Description

When running Vite from a parent process (Elixir Phoenix dev watcher in my case) the process doesn't shut down and keeps running in the background after the parent process is stopped.

Example

Starting and then stopping mix phx.server with watchers set up to run npx vite leaves two ghost processes still running:

# ps aux | grep vite  ### simplified output
nullpilot   ?   S   14:31   0:00 sh -c vite
nullpilot   ?   Sl  14:31   0:00 node /home/nullpilot/demo_project/assets/node_modules/.bin/vite

Solution

In vue-cli the webpack approach was used, and a flag was added to address this (vuejs/vue-cli#1597)

Here I am instead using the same approach used in rollup (rollup/rollup#3493) to do it without adding an extra flag.

With the change, running the same as above no longer leaves any dangling processes running and everything shuts down as expected.

[LostKobrakai]

This PR was merged, but the underlying problem still persists as the following commit reverts what this PR fixed. In the current state vite still doesn't stop when stdin is closed and creates zombie processes when used with phoenix.

[nullpilot]

@LostKobrakai For Phoenix/Elixir specifically there is a wrapper script you can use, the Port docs talk about Zombie processes and how to prevent them.

As for the PR itself, I would have liked to get the support directly into Vite before the v2 release of course, but the solution might be more complex than a quick PR after all. Sorry for causing the need to roll back the change. The tests passed and I didn't have the reference frame to notice something's not right beyond that.

[jonathanstiansen]

@nullpilot I get weird error when trying to use that work around - using exactly the script they talk about (I called it start.sh) I use these watcher args:

  watchers: [
    node: [
      "start.sh",
      "npx",
      "vite",
      cd: Path.expand("../assets", __DIR__)
    ]

However, it gives me errors as if it's not even interpreting it with bash, if I remove all comments I still get unexpected token with some valid (and needed bash syntax):

/Users/jono/projects/phx-vite-demo/demo/assets/start.sh:3
# Start the program in the background
^

SyntaxError: Invalid or unexpected token
at wrapSafe (internal/modules/cjs/loader.js:979:16)
    at Module._compile (internal/modules/cjs/loader.js:1027:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)

[jonathanstiansen]

For anyone who lands here I fixed it with js instead of bash:
assets/start.js

const { execSync } = require("child_process");

// Exit the process when standard input closes due to:
//   https://hexdocs.pm/elixir/1.10.2/Port.html#module-zombie-operating-system-processes
//
process.stdin.on("end", function() {
    console.log("standard input end");
    process.exit();
});

process.stdin.resume();

execSync("./node_modules/.bin/vite")

dev.exs

...
config :demo, DemoWeb.Endpoint,
  http: [port: 4000],
  debug_errors: true,
  code_reloader: true,
  check_origin: false,
  watchers: [
    node: [
      "start.js",
      cd: Path.expand("../assets", __DIR__)
    ]
  ]
...

[1player]

I could not make @jonathanstiansen script work. vite keeps running on CTRL-D/stdin close.

This one works for me, by killing the vite process when receiving a CTRL-D:

const { spawn } = require("child_process");

const child = spawn("./node_modules/.bin/vite", { stdio: "inherit" });

process.stdin.on("end", function () {
  console.log("stdin close");
  child.kill();
  process.exit();
});

process.stdin.resume();

[50kudos]

Hi, just to add more option for folks here. I use the following, and it terminates properly:

--- a/assets/package.json
+++ b/assets/package.json
@@ -3,8 +3,8 @@
   "scripts": {
+    "build": "vite build",
+    "watch": "vite build --watch --minify false --emptyOutDir false --clearScreen false --mode development"
   },
   "devDependencies": {
+    "vite": "^2.2.4"
   }
 }
 
--- a/config/dev.exs
+++ b/config/dev.exs
@@ -23,7 +23,7 @@ config :fset, FsetWeb.Endpoint,
   watchers: [
-    node: ["whatever", "ever", cd: Path.expand("../assets", __DIR__)]
+    yarn: ["run", "watch", cd: Path.expand("../assets", __DIR__)]
   ]

---

For full integration (least work), check this out: https://github.com/50kudos/vite-phx

[IanVS]

This seems to cause problems with the vite web-test-runner plugin: material-svelte/vite-web-test-runner-plugin#6.

[thiagomajesk]

@nullpilot @IanVS I've been noticing a weird amount of memory usage while using Vite in my WSL machine while starting/ stoping phoenix while using Vite. Is this a regression? I looked at this comment over here: #3659 (review) which seems to suggest that this is the case. Is anyone else having problems with this?

[IanVS]

@thiagomajesk I attempted to add process.stdin.resume() to allow streams to drain, but it caused other problems. (see #4082). I'm no expert in node process management, but I'd be curious if you tried adding the resume back in locally in your node_modules, if you'd see any difference. It might be worth opening an issue with any details you can collect.

[nullpilot]

@thiagomajesk This PR caused some other side effects (sorry about that!) and was partially rolled back - removing the line that was the actual fix for this particular issue. I haven't kept up and don't know what the current situation on this is.

The Elixir docs provide an example of a wrapper script that can be used to avoid this issue, as far as I understand. I haven't tried since my setup changed, but that's what I would go with now.

[thiagomajesk]

Hey, @IanVS I'm no expert either, so I don't know how far I can contribute to this one. However, I read a post recently that uses a similar approach. It looks a bit hackish but it seems to work: https://moroz.dev/blog/integrating-vite-js-with-phoenix-1-6.

[thiagomajesk]

@nullpilot I was wondering if a --watch-stdin flag or something similar would solve this in the same way it did for Webpack.

[nullpilot]

@thiagomajesk It would make sense and I think it would be the most reliable solution at this point. When I made this PR I specifically looked for an option that didn't require a new flag, instead of proposing the solution from vue-cli, as it was stated somewhere that it's the preferred solution over extra flags if they can be avoided.

Given that the no-flag solution hasn't really worked out, it may be worth sticking with the proven method and adding it as a flag after all.