ericclemmons / start-server-webpack-plugin

Automatically start your server once Webpack's build completes.
MIT License
158 stars 26 forks source link

Send a signal to the server process for HMR #17

Closed tikotzky closed 6 years ago

tikotzky commented 6 years ago

This will send a SIGUSR2 signal to the server process in the after-emit hook. This allows you to use webpack/hot/signal instead of webpack/hot/poll so that changes should be instant.

This PR enables the signal by default, but it should not break anything since most apps will just ignore SIGUSR2. If you'd like I can make it opt in only. Let me know if you'd like me to change it.

fixes: #9

tikotzky commented 6 years ago

I changed the logic to what you suggested which allows passing anything as a signal...

If you are planning on making a major bump, I have another feature I'd like to add as well... I would like to enable manually restarting the server by typing rs similiar to how nodemon works today. The idea is to allow the user to restart if they make external changes which affect the app (for example to env variables)

I'll try to get a basic PR opened today.

wmertens commented 6 years ago

Cool, the we could also do q for graceful quit and maybe k for killing the process tree etc.

I'm working on using childprocess.fork to fix #12, I'll set up a branch later today. Then it really will merit a major bump…

On Thu, Feb 15, 2018, 3:05 PM Mordy Tikotzky, notifications@github.com wrote:

I changed the logic to what you suggested which allows passing anything as a signal...

If you are planning on making a major bump, I have another feature I'd like to add as well... I would like to enable manually restarting the server by typing rs similiar to how nodemon works today. The idea is to allow the user to restart if they make external changes which affect the app (for example to env variables)

I'll try to get a basic PR opened today.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/ericclemmons/start-server-webpack-plugin/pull/17#issuecomment-365936480, or mute the thread https://github.com/notifications/unsubscribe-auth/AADWlsIczPSCzXFuL-TpA1kT1mCzxmuUks5tVDmbgaJpZM4SFv4r .

wmertens commented 6 years ago

So, in https://github.com/ericclemmons/start-server-webpack-plugin/tree/use-the-fork I'm trying to make it work with messaging, maybe that's better? Thoughts?

ericclemmons commented 6 years ago

@wmertens I agree with a major version bump. I'm also really impressed with the direction y'all are going! I've always struggled with signals in the past :(

Recently I've experienced a problem where the server crashes, and I've had to do a full close & restart of the webpack process as a result.

Would this lay the groundwork for the plugin to restart failed processes automatically, or being able to do a r for restarting a process (in case HMR causes some issue)?

wmertens commented 6 years ago

@ericclemmons, check out the use-the-force branch, it does this…

On Fri, Feb 16, 2018, 10:49 PM Eric Clemmons, notifications@github.com wrote:

@wmertens https://github.com/wmertens I agree with a major version bump. I'm also really impressed with the direction y'all are going! I've always struggled with signals in the past :(

Recently I've experienced a problem where the server crashes, and I've had to do a full close & restart of the webpack process as a result.

Would this lay the groundwork for the plugin to restart failed processes automatically, or being able to do a r for restarting a process (in case HMR causes some issue)?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/ericclemmons/start-server-webpack-plugin/pull/17#issuecomment-366368194, or mute the thread https://github.com/notifications/unsubscribe-auth/AADWlsvSljiRKZBkstpjs5iZPDvOCmNUks5tVff_gaJpZM4SFv4r .

ericclemmons commented 6 years ago

@wmertens Whoa, you're right. That's going to be a fantastic addition! 😍

wmertens commented 6 years ago

@ericclemmons I just need to figure out how to add the monitor to the entry and then it's good to go.

Ideally, it would be by providing string contents so that there is no complexity from loaders and plugins processing the monitor. Tips welcome.

ericclemmons commented 6 years ago

Hmmm, I'm not sure how to add the monitor entry dynamically, but since it's server-side, it should be largely bypassed. Or, perhaps it could be added as an external?

In the past I've rewritten entry to be myCustomScript.js?pathToOriginalEntry and embedded it via __resourceQuery. The problem with that approach is that it messes up source maps :(

wmertens commented 6 years ago

I have decided that this should be merged, and then use-the-fork can be the next major release once fully hammered out.

That way both options are available, and we can update both versions as we see fit.