shellscape/webpack-plugin-serve

HMR broken with historyFallback in Firefox

davidroeca opened this issue · 11 comments

  • Webpack Version: 4.23.1
  • Operating System (or Browser): Linux Mint 18.2 Cinammon 64-bit (Firefox 65.0 and 64.0.2)
  • Node Version: 11.3.0
  • webpack-plugin-serve Version: 0.4.0

How Do We Reproduce?

With the given Firefox versions, run the react recipe in this repo.

Expected Behavior

HMR works as expected with historyFallback on Firefox. This is especially important if I'm developing an app that relies on something like https://github.com/ReactTraining/react-router for SPA purposes.

Actual Behavior

Notice in the console that the following error occurs: Firefox can’t establish a connection to the server at ws://[::]:55555/wps..

Now kill the start script, remove the historyFallback option and restart. The bug now should be fixed.

Note that this problem does not exist on current versions of Chrome/Chromium.

By changing configuration to the following:

    new Serve({
      // note: this value is true by default
      hmr: true,
      historyFallback: {
        logger: console.log.bind(console),
        verbose: true,
      },
      static: [outputPath]
    })

I found that when loading localhost:55555, the following is logged in a Firefox page load:

Rewriting GET / to /index.html
Not rewriting GET /bundle.js because the client does not accept HTML.
Rewriting GET /wps to /index.html

While the following behavior happens in Chrome:

Rewriting GET / to /index.html
Not rewriting GET /bundle.js because the client does not accept HTML.
Not rewriting GET /wps because the client did not send an HTTP accept header.
Not rewriting GET /favicon.ico because the client does not accept HTML.

Not sure if this is the most optimal, but it can be fixed with the following configuration:

    new Serve({
      // note: this value is true by default
      hmr: true,
      historyFallback: {
        logger: console.log.bind(console),
        verbose: true,
      },
      middleware: (app, builtins) => app.use(async (ctx, next) => {
        if (ctx.path.match(/^\/wps/)) {
          const { accept, ...remainingHeaders } = ctx.request.header
          ctx.request.header = remainingHeaders
        }
        await next()
      }),
      static: [outputPath]
    })

EDIT: a simpler approach

    new Serve({
      // note: this value is true by default
      hmr: true,
      historyFallback: {
        // disable the plugin on /wps
        rewrites: [
          {
            from: '/wps',
            to: (context) => (context.parsedUrl.pathname),
          },
        ],
      },
      static: [outputPath]
    })

@davidroeca would you classify this as a problem with WPS or with http-proxy-middleware directly?

@shellscape well actually, it's most likely an issue with how connect-history-api-fallback deals with WebSocket requests from Firefox vs Chrome, because the logic relies on the non-existence of an Accept header, which Firefox provides but Chrome does not. My most recent work-around essentially disables connect-history-api-fallback on the /wps path.

See https://github.com/bripkens/connect-history-api-fallback/blob/e931cce468099b8d08d3e6115e3e2b8b598e11d1/lib/index.js#L19

Ah yeah, that's what I meant :) mental lapse there. Could this be related? bripkens/connect-history-api-fallback#60

Somewhat related, though Firefox sends along an Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8 header with websockets and I haven't seen chrome replicate this behavior with the same request.

So I guess for now it's a bug specific to websockets in firefox, which passes along an Accept header to connect-history-api-fallback, which then rewrites to /index.html.

Thanks for the explanation. Do you have any suggestions for how we should best handle this?

Maybe an option for the wps web sockets to be configured on a separate port, similar to webpack-serve? In that case, the websocket portion won't need to interact with the history fallback.

That would require using a separate WebSocket Server, something that we explicitly tried to avoid because of the complications and overhead it created. Unfortunately that's not an option for this plugin. I can definitely add a FAQ question for this. But I'd be curious to know about any other ideas you have. If adding the middleware from your reply above fixes the issue, I'm open to making that a permanent addition as well.

Yeah I mean implementing a middleware that gets around this definitely makes sense to me as long as it doesn't make the plugin code too confusing. Maybe by default this middleware behavior is turned on but there could be a configuration option to turn it off if it causes people problems? It would probably be worth documenting this behavior as it relates to connect-history-api-fallback because removing the accept header might cause confusion for some people

Sounds like a plan to me. I'll get this implemented.