Skip to content
This repository has been archived by the owner on Jun 3, 2019. It is now read-only.

Come up with better strategy for hot reloading of server #330

Open
Tracked by #428
bkniffler opened this issue Jan 16, 2017 · 22 comments
Open
Tracked by #428

Come up with better strategy for hot reloading of server #330

bkniffler opened this issue Jan 16, 2017 · 22 comments

Comments

@bkniffler
Copy link
Collaborator

Hey,
If disableSSR = true, changing code will still trigger a hot reload and server restart, since it will only affect the express servers response, not the server bundle building.

Is this the desired behavior?

@smirea
Copy link

smirea commented Jan 20, 2017

ping on this
reloading the server is causing issues in dev mode as the client sometimes resends requests to the server while it's being reloaded. There should be a mode to not hot reload the server with SSR disabled

EDIT: If anyone is having the same issue as I am with requests being sent while the dev server is hot reloading, I hacked it for now by adding a timeout to the client server. It ain't pretty, but gets the job done. @ctrlplusb plz halp

    // 👉  tools/development/hotNodeServer.js:57

    const waitForClientThenStartServer = () => {
      if (this.serverCompiling) {
        // A new server bundle is building, break this loop.
        return;
      }
      if (this.clientCompiling) {
        setTimeout(waitForClientThenStartServer, 50);
      } else {
        // 👉  TODO: HACK: remove this timeout when you can disable server hot reloading
        setTimeout(() => startServer(), 200);
      }
    };

@bkniffler
Copy link
Collaborator Author

Cool @smirea, I'm suffering similar issues, assets and data served by the express server will not load properly for the reason you stated. Your solution might work to get around this for now, but having the possibility to not restart the whole server if something in your react app changed while SSR is disabled would be very nice.

@ctrlplusb ctrlplusb added the bug label Jan 21, 2017
@ctrlplusb
Copy link
Owner

Agreed

@ctrlplusb
Copy link
Owner

I did look at this and it turned into a bigger pain than anticipated. I am going to give the tooling a once over for v13 and hopefully I can come up with a more elegant solution.

@ctrlplusb
Copy link
Owner

ctrlplusb commented Feb 26, 2017

Finally have a solution for this. 😀

It's perhaps not a perfect solution, however, I think it will acceptable. Basically, when ssr is disabled I configure the server's webpack watcher to ignore any paths in the shared folder. So the server will only rebuild for changes within it's own folder.

One case this would fail though: if a change is made to some shared code that isn't related to the frontend only (e.g. a util). This won't cause the server to reload. What are your guys thoughts on how to handle this case? We could beef up the CLI output a bit and allow users to manually trigger a server restart? Or somehow make the ignore flag a bit more clever?

This will be going into the next branch.

Here is a code snippet update on development/hotNodeServer.js:

    // Lets start the compiler.
    this.watcher = compiler.watch(
      {
        ignored: config('disableSSR')
          ? `${path.resolve(appRootDir.get(), config('sharedCodePath'))}/**/*`
          : undefined,
      },
      () => undefined,
    )

@ctrlplusb
Copy link
Owner

Perhaps something like this could make it a bit more intelligent?

https://github.com/dependents/node-dependency-tree

I just worry the additional complexity introduces flakiness.

@birkir
Copy link
Collaborator

birkir commented Feb 27, 2017

Good work there @ctrlplusb

So now we will have to stop / start the dev server if we change that disableSSR flag, right?

@ctrlplusb ctrlplusb added this to the 13.0.0 milestone Mar 5, 2017
@ctrlplusb
Copy link
Owner

@birkir

Yeah, but only for changes to the shared folder(s). Any change to the server folder itself would cause it to restart automatically.

I have been trying out this solution in a personal project of mine, to see if it is too confusing or not. We will have to output more appropriate messages to let the user know what is going on too, and ask them if they would like to restart the server. May need to buff up the the CLI interface/output. There are some cool tools for this, e.g. https://github.com/mattallty/Caporal.js

@ctrlplusb
Copy link
Owner

Thought of another cool possibility...

Only auto hot-reload client - however, when server changes send a message to the client. The client then displays a toast/notification informing them that the server code has changed, and that they can click the notification to restart the server whenever they would like. Avoids user having to switch back out to the console to do a manual restart and allows you a grace period where hot reloading the client with components hitting server APIs etc don't fall over.

Thoughts?

@ctrlplusb ctrlplusb removed this from the 13.0.0 milestone Apr 5, 2017
@birkir
Copy link
Collaborator

birkir commented Apr 5, 2017

I think I would use that feature very often. Lots of time I hot reload and the server takes too long to restart so when the client reloads and needs some local api endpoint, its not available.

@ctrlplusb ctrlplusb changed the title disableSSR = true will still hot reload server Come up with better strategy for hot reloading of server Apr 21, 2017
@ctrlplusb ctrlplusb mentioned this issue Apr 21, 2017
3 tasks
@strues
Copy link
Collaborator

strues commented Apr 21, 2017

I've been thinking about a solution to this issue for a while now. Rather than creating the server bundle during development, what about running the server as is. By reconfiguring the server entry point and all of the development scripts this could work.

I dont have much time so pardon the pseudo-code

  1. yarn develop => babel-node server/index.js
  2. logic in index.js pulls in our listener & express app() from say, app.js
  3. index.js imports our hotreload and passes it the app instance
  4. Hot reloading files condensed below:
 // basically what we have now, but additionally including server
const serverConfigFiles = [ /\/server\//, /\/config\//, /\/internal\// ]

// client/shared files in addition to server/config
const allWatchFiles = [ /\/client\//, /\/shared\//, ...serverConfigFiles ]

// regex to match I'd use Ramda, but maybe someone 
// smarter (not as lazy) could write it (R.any) and we wouldnt need to add ramda
const fileMatch = (regexs, id) => R.any((regex) => regex.test(id))(regexs)

const compilerPlugin = webpack(config)
const watcher = chokidar.watch(SERVER PATH aka server/**)

compiler.plugin('compile', () => log...)
compiler.plugin('compilation', () => log...)

// add webpack-dev-mw  & pass it compiler
app.use( webpack-dev-mw(compiler, {devMWoptions })

watcher.on('ready', () => {
    watcher.on('all', (event, file) => {
      Object.keys(require.cache).forEach((id) => {
        if (fileMatch(serverConfigFiles, id)) {
          delete require.cache[id]
        }
      })
    })
  })
  compiler.plugin('done', () => {
    Object.keys(require.cache).forEach((id) => {
      if (fileMatch(allWatchFiles, id)) {
        delete require.cache[id]
      }
    })
  })

Also looking at the Webpack-Dev-Middleware docs, it might be worthwhile to experiment with the SSR features, see the bottom of their docs

@peshi
Copy link

peshi commented May 7, 2017

I am using this in my current project,
https://github.com/60frames/webpack-hot-server-middleware

@birkir
Copy link
Collaborator

birkir commented May 8, 2017

@peshi implementation details? It's pretty hard to use with react-universally because of the split server/client thing.

@peshi
Copy link

peshi commented May 8, 2017

@birkir Well, I came across this project (react-universally) yesterday. I started today and spent a couple of hours to rip it apart and managed to get and compile both configs in memory. But there's still a lot of magic and code to throw in the trashcan. I might re-visit that code when I have more free time.

@birkir
Copy link
Collaborator

birkir commented May 8, 2017

Cool, thanks. I got it working, but it was always reloading my server. Still need to work on it.

@ctrlplusb
Copy link
Owner

ctrlplusb commented May 8, 2017

All, I have a new effort I am building on the side. It includes a significantly simplified (and enhanced) developer toolchain. I hope to have some prototypes up soon, after which we could look at migrating it into the project.

The webpack-hot-server-middleware looks interesting too!

@Aetherall
Copy link

Aetherall commented Mar 28, 2018

Actually I was using webpack-hot-server-middleware before switching over this boilerplate because errors are swallowed by the webpack plugin hook

Maybe this is misconfiguration though, but take care of checking how errors behave if you try to implement that

@mschipperheyn
Copy link

Any news on this? In my case, I've got quite a bit going on the server, making shared code changes a slow process

@grahamd711
Copy link

grahamd711 commented Dec 19, 2018

I've recently had the below issue when the dev server reloads due to a minor change. Thinking it is related to the new bundle being built. I'm returning data on http://localhost:7331/__webpack_hmr so I was not sure if this was related. Anyone else experiencing similar issues?

SERVER: Error in server execution, check the console for more info. Error: listen EADDRINUSE :::3000

@birkir did you run into any issues like this when setting .env variables for the mP site?

@oyeanuj
Copy link
Contributor

oyeanuj commented Dec 25, 2018

@grahamd711 Also, seeing this suddenly. Did you find a fix for this?

@grahamd711
Copy link

grahamd711 commented Jan 3, 2019

@oyeanuj , yes still occurring on my end. shutting down the server and running yarn dev again works, but the hot reloading is still an issue. Any luck on your side?

@oyeanuj
Copy link
Contributor

oyeanuj commented Jan 3, 2019

@grahamd711 None yet :( I upgraded RHL, Webpack, etc but no luck yet. Searching online it suggested that it might have something to do with nodemon but that isn't something that I've installed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants