60frames / webpack-hot-server-middleware

:fire: Hot reload webpack bundles on the server
MIT License
323 stars 50 forks source link

Add a way to manage state/resources #30

Open aflatter opened 7 years ago

aflatter commented 7 years ago

Hello,

first, thanks for webpack-hot-server-middleware - it's been a pleasure to use so far!

Today, I ran into the following problem: My server renderer needs to regularly pull resources from a 3rd-party API, which is then used to render the application. To achieve that, I added a simple function that is called using setInterval. Now when a new webpack build replaces the old one, I need to clear that timer again in order to 1) allow for garbage-collection of the old build 2) stop that old timer to avoid accumulating multiple versions.

I've solved the problem by allowing the serverRenderer-Module to export a dispose function, which is called before the new build is activated. A quick & dirty implementation is here: https://github.com/aflatter/webpack-hot-server-middleware/commit/754a049de2caca42738003c51bbd7b634d6f1832

Is there another way to solve this without touching webpack-hot-server-middleware? I've thought about passing in the timer logic as an option to the server renderer, but then I lose hot-reloading of that logic.

If there is no other way, I'd love to get input on a possible API that might also allow keeping state between two different builds:

// serverRenderer.js
export function serverRenderer({clientStats, serverStats, state}) {
  // Before this server renderer is replaced, this function is called and has the chance to return a new state that will be passed to the next server renderer.
  const replacementHandler = () => {
    const {versions} = state || {}
    return {versions: versions + 1}
  }

  // This is the actual function that handles incoming requests.
  const requestHandler = (req, res, next) => {
    res.status(200).send(`Hello world, I am server renderer #${state.versions}!`)
  }

  return {replacementHandler, requestHandler}
})

Would you accept that kind of functionality? Thanks!

richardscarrott commented 6 years ago

@aflatter apologies for the incredibly slow response -- I've been putting this issue off as in all honesty I'm on the fence re: whether it's worth the added complexity.

Personally I have done as you suggest, pass a getter as an option and fetch the data in the dev server but of course you lose hot reloading.

Rather than trying to maintain state I wonder if it'd be enough to just offer a dispose hook to keep things simpler, e.g.

let timerId;
let data;
const startUpdater = () => { timerId = setInterval(() => fetchData().then((nextData) => data = nextData), 1000);

export function serverRenderer({ clientStats, serverStats }) {
   startUpdater();
   return (req, res, next) => res.send(data);
}

serverRenderer.dispose = () => {
    clearTimeout(timerId);
}

EDIT: @aflatter I just realised this is exactly what you did in your fork 🤦‍♂️ -- I'd be happy to accept a PR for this.

aflatter commented 6 years ago

@richardscarrott Hello, thanks for your answer. I currently don't need this functionality (anymore), but I still think that it's worth adding. I agree that a dispose hook is much simpler and that it could be a good-enough solution 👍