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

Stop sirv before reloading rollup #213

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,39 @@ import resolve from '@rollup/plugin-node-resolve';
import livereload from 'rollup-plugin-livereload';
import { terser } from 'rollup-plugin-terser';
import css from 'rollup-plugin-css-only';
import { spawn } from 'child_process';

const production = !process.env.ROLLUP_WATCH;

function serve() {
let server;

function toExit() {
if (server) server.kill(0);
if (server) {
/* On linux, server.kill() only kills the parent shell (sh) process but not the child sirv instance
See https://nodejs.org/docs/latest-v14.x/api/child_process.html#child_process_subprocess_kill_signal
Passing the negation of PID of a detached process to 'kill' stops all its children */
try {
spawn('kill', ['--', `-${server.pid}`], { shell: true });
} catch (_) {
server.kill();
}
}
}

return {
writeBundle() {
if (server) return;
server = require('child_process').spawn('npm', ['run', 'start', '--', '--dev'], {
server = spawn('npm', ['start', '--', '--dev'], {
stdio: ['ignore', 'inherit', 'inherit'],
shell: true
detached: true,
});

process.on('SIGTERM', toExit);
process.on('exit', toExit);
}
},
/* Rollup restarts on detecting changes to this config file.
This hook makes sure the previously started sirv instance is stopped before starting a new one */
closeWatcher: toExit,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main fix here.

I think we can skip the kill workaround by just removing shell: true on the spawner

Copy link
Author

@ranjan-purbey ranjan-purbey Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried that, but a shell is spawned regardless. The shell is spawned by npm.

};
}

Expand Down