Instantiate MemoryFileSystem only once

[moljac024]

I wasn't sure if I should open an issue or a pull request, but since I already made some changes, here goes.

I have perhaps a peculiar use case: I want to use the middleware for multiple express apps that would share a single compiler but otherwise be independent and listen on different ports and serve different index files that would include different bundles (webpack is configured with multiple entry points).

The problem arises because the middleware mutates the compiler.outputFileSystem property. When setting up multiple middlewares only the last one will be valid and the ones before will hold a reference to a memory fs that the compiler is not using to write files to.

My solution was to detect if the outputFileSystem already was a MemoryFileSystem and if it was, just not overwrite it but use the existing one for the middleware. An alternative approach would be to take an optional file system as an option and use that, and then every middleware instance could be passed in the same fs reference. But this requires installing memory-fs as a peer so for my quick-fix I didn't do it that way.

There must be a better way to achieve this, and I also may be misunderstanding something and doing everything horribly wrong. I need some feedback on this and if changes in this direction are wanted, will fix up the commits and the code.

[moljac024]

This is an example of multiple express apps that would use the same compiler:

var path = require('path');
var express = require('express');

var webpack = require('webpack');
var webpackConfig = require('./webpack.config');
var devMiddleware = require('webpack-dev-middleware');

var compiler = webpack(webpackConfig);


function configureApp (app, indexFile) {
  app.use(devMiddleware(compiler, {
    noInfo: true,
    quiet: true,
    publicPath: webpackConfig.output.publicPath
  }));

  // For any route that isn't static file, serve the index.html, so we can have
  // client-side routing
  app.get('*', function(req, res) {
    res.sendFile(indexFile);
  });

  return app;
}

var ServerStartedCallback = function (port) {
  return function(err) {
    if (err) {
      console.log(err);
      return;
    }

    console.log('Listening at http://0.0.0.0:' + port);
  }
}

function listen (app, port) {
  app.listen(port, ServerStartedCallback(port));
}

function startServer (app, port, indexFile) {
  listen(configureApp(app, indexFile), port)
}

var apps = [
  { // App 1
    port: 3000,
    index: path.join(__dirname, 'src/public', 'index1.html')
  },
  { // App 2
    port: 3001,
    index: path.join(__dirname, 'src/public', 'index2.html')
  }
];

// Start dev servers
apps.forEach(function (app) {
  startServer(express(), app.port, app.index)
})

[SpaceK33z]

Does your usecase still work when you remove the options.fileSystem? It looks now as if you've implemented two ways of reusing the outputFileSystem; by allowing options.fileSystem AND by checking if compiler.outputFileSystem already exists.

[moljac024]

Yes, my usecase will work with the removal of options.fileSystem. I'm not even using it currently but I added it here because it might be a better solution than checking the outputFileSystem.

Or maybe we want both for flexibility. Like I said, I'm willing to make any changes necessary, so long as there is an option for not mutating the outputFileSystem.

[SpaceK33z]

Okay, could you remove options.fileSystem then? After that I'll do some tests and merge this in.

[moljac024]

Done.

EDIT: I have also changed the pull request title, removing the part that referenced the deleted code.

[SpaceK33z]

Cool, I'll try to test it in the upcoming days.

I've started writing some tests for this package since yesterday. If you have the time, it would be awesome if you could write a test for this. It would ensure this does not break in the future, so it's also in your own best interest :).

[SpaceK33z]

Thanks. I added a test for this.