Skip to content
This repository has been archived by the owner on Feb 15, 2023. It is now read-only.

Do not watch Rollup config file #199

Open
lukeed opened this issue Dec 15, 2020 · 3 comments · May be fixed by #213
Open

Do not watch Rollup config file #199

lukeed opened this issue Dec 15, 2020 · 3 comments · May be fixed by #213

Comments

@lukeed
Copy link
Member

lukeed commented Dec 15, 2020

Note: While this is problematic, we might be able to kick the can on it because of the upcoming Svelte Kit release.
Off hand, not even sure if we can tell Rollup to ignore its own file.

Any changes made to rollup.config.js automatically triggers Rollup to rebuild itself. This reinvokes the entire plugins[] list, which not only can be slow, but reinitializes a new sirv process.

Because Rollup clears reference to its previous rollup.config.js, the server reference resets to undefined, forcing a new server subprocess.

After a few modifications to my config, I saw I had 16 node processes running in Activity Monitor.
Then after shutting down dev script, 3 nodes remain, meaning that the Rollup resets make it possible for our npm start subprocesses to be detached.

Logs

$ rollup -c -w
rollup v2.35.1
bundles src/main.js → public/build/bundle.js...
created public/build/bundle.js in 211ms

[2020-12-14 22:17:43] waiting for changes...

> [email protected] start /Users/lukee/repos/testing/svelte-template
> sirv public "--dev"

  Your application is ready~! 🚀

  - Local:      http://localhost:5000
  - Network:    Add `--host` to expose

────────────────── LOGS ──────────────────

bundles src/main.js → public/build/bundle.js...
created public/build/bundle.js in 91ms

[2020-12-14 22:17:51] waiting for changes...
  [22:17:52] 200 ─ 2.64ms ─ /
  [22:17:52] 200 ─ 0.44ms ─ /global.css
  [22:17:52] 200 ─ 0.22ms ─ /build/bundle.css
  [22:17:52] 200 ─ 0.95ms ─ /build/bundle.js
  [22:17:52] 200 ─ 0.66ms ─ /favicon.png
bundles src/main.js → public/build/bundle.js...
created public/build/bundle.js in 65ms

[2020-12-14 22:17:57] waiting for changes...

Reloading updated config...
bundles src/main.js → public/build/bundle.js...
created public/build/bundle.js in 134ms

[2020-12-14 22:18:07] waiting for changes...

> [email protected] start /Users/lukee/repos/testing/svelte-template
> sirv public "--dev"

  Your application is ready~! 🚀

  ➡ Port 5000 is taken; using 64353 instead

  - Local:      http://localhost:64353
  - Network:    Add `--host` to expose

────────────────── LOGS ──────────────────
@ranjan-purbey
Copy link

Can confirm this on linux
Maybe related #143

@ranjan-purbey
Copy link

As a workaround on linux, you can change the serve() function to the following:

function serve() {
  let server;
  function toExit() {
    if (server) server.kill();
  }

  return {
    writeBundle() {
      if (server) return;
      server = spawn("bash", ["-c", "sirv public --dev"], {
        stdio: ["ignore", "inherit", "inherit"],
      });
      process.on("SIGTERM", toExit);
      process.on("exit", toExit);
    },
    closeWatcher: toExit,
  };
}

NB: You will need bash installed in order for this to work.

closeWatcher stops the server on rollup reload
npm run start needs to be replaced with bash -c sirv public because npm run internally uses sh -c to run the command. And sh -c for some reason doesn't seem to forward SIGTERM to its children, which makes me sad. See docs

I'll love to see a better solution

@ranjan-purbey
Copy link

ranjan-purbey commented Jan 22, 2021

This removes the need for bash:

function serve() {
  let server;
  function toExit() {
    if (server) {
      try {
        spawn("kill", ["--", `-${server.pid}`]);
      } catch (error) {
        console.log(error.message);
        server.kill();
      }
    }
  }

  return {
    writeBundle() {
      if (server) return;
      server = spawn("yarn", ["start", "--dev"], {
        stdio: ["inherit", "inherit", "inherit"],
        detached: true,
      });
      process.on("SIGTERM", toExit);
      process.on("exit", toExit);
    },
    closeWatcher: toExit,
  };
}

Starting the server in a detached process makes sure that the PID of server is also the PGID of all of it's children. Then passing the negation (-) of server PID to kill kills all the children

ranjan-purbey added a commit to ranjan-purbey/template that referenced this issue Jan 22, 2021
Fixes sveltejs#199

Rollup reloads the config on detecting changes in it. This should also restart the `sirv` server. But the current config keeps creating new instances of `sirv` on each reload without properly stopping the previous ones. This PR fixes that behaviour.
@ranjan-purbey ranjan-purbey linked a pull request Jan 22, 2021 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants