Separate server and app initialization into two parts (optional)

[alallier]

Opening a new issue for this idea. This conversation originally started on #56. Relevant comment below.

@peterwillis

The way I have middleware setup also makes it difficult for the same reason. I totally understand why you have done it the way you have for the most convenient case. In the middleware chain I have, there is no good place where server and app are available together at a point where the routing can be changed. All the middleware for the app is setup in advance including the error handling and this is a separate module. This module is then required by the serving module, which decides whether it should start an HTTP or HTTPS server for the app. Is it possible to separate out where the route is added so that I can do it in two steps? So I would have to add code in two places for reload to work in this way:

during routing:
reload.configureRoute(app)

then another function that wouldn't need the app:
reload.websocketServer(server)

I think the module would need another way to instantiate so it wasn't always returning the single function. I can put together a PR if you think it is feasible to include it?

[alallier]

@peterwillis you mentioned in #56 that you would be interested in writing a PR for this. Are you still interested in writing a PR for this I think this would be a great idea?

Can we do?

For routing: reload.configureApp(app)
And for the socket: reload.configureServer(server)

[peterwillis]

Yes I think I can something submitted today or tomorrow

[AckerApple]

reload has a fork that has middleware definition support. See for yourself : https://www.npmjs.com/package/ack-reload#middlware

[peterwillis]

If I needed it to work that way, I could just use the venerable webpack-dev-server. The way that fork works is hijacking the way static content is served which won't work for my middleware chain. Using this package means you can use the express static middleware.

[AckerApple]

I like sassy replies! Most all of mine are sassy too!!!

Here, it seems again I have to do the leg work to inspire the people using reload:

UPDATE : The following example is considered weak in comparison to NEXT COMMENT by Acker Apple with isRequestForReqload

app.js

const express = require('express');
const reload = require('ack-reload');

const app = express();
// LOTS AND LOTS OF MIDDLEWARE HERE...
// server is not created since in this module.
app.use((err, req, res, next) => {
  // final error handler
});

app.use( reload.middleware(app, false) )

module.exports = app;

serve.js

const app = require('./app');
const http = require('http');
const reload = require('ack-reload');

// we actually have http and https switched on configuration, but this example is for brevity
const server = http.createServer(app);
// at this point we have server and app, but the middleware chain setup is completed so we can't add reload.

const pathToWwwFiles = require('path').join(__dirname,'www')
reload.reloadSocketByHttp(pathToWwwFiles, http)

server.listen(8888, () => { console.log('started!') });

Let me know if this works for you. Otherwise make another sassy reply and I'll provide code for you to sit and spin on ;)

[AckerApple]

Hey, smart guy, my dumb ass came up with another simplified approach.

app.js (untouched - needs no changes)

const express = require('express');

const app = express();
// LOTS AND LOTS OF MIDDLEWARE HERE...
// server is not created since in this module.
app.use((err, req, res, next) => {
  // final error handler
});

module.exports = app;

serve.js

const app = require('./app');
const http = require('http');
const reload = require('ack-reload');

//tell ack-reload folder to put watch on
const pathToWwwFiles = require('path').join(__dirname,'www');

// we actually have http and https switched on configuration, but this example is for brevity
const server = http.createServer(function(req,res){
  if( reload.isRequestForReload(req) ){
    midware(req, res)//request is html OR reload.js file
  }else{
    app(req, res)
  }
});

//watch files, create websocket and return request processing function
const midware = reload.middleware(pathToWwwFiles, server)

// at this point we have server and app, but the middleware chain setup is completed so we can't add reload.
server.listen(8888, () => { console.log('started!') });

The function isRequestForReload was created, today, to properly handle existing app integration: ack-reload

[peterwillis]

Sorry I wasn't intending to be smart or sassy - sorry if it came across that way. That doesn't solve the problem at all. You are still delegating the handling of static content to your middleware. If you want to have a conversation about it, perhaps do it on your repos issues not here?

[AckerApple]

Sass and read worthy is my intent. I like to create buzz to gain and keep interest. Just letting you know so nothing is taken to heart on either end... Hey good luck solving your issue. I created a new function of isRequestForReload that I think may hit the spot more for your needs. See below

const server = http.createServer(function(req,res){
  if( reload.isRequestForReload(req) ){
    midware(req, res)//request is html OR reload.js file
  }else{
    app(req, res)
  }
});

This new method ensures reload middleware only gets involved when it should and not delegate ALL request. Please review and reply as I think this is your fix.

[peterwillis]

It does not - I don't know how I can be clearer about this. Please use your own repository for issues about it.

[AckerApple]

Heard loud and extra clear. Intent is absolutely not to harass. Farewell

[peterwillis]

Resolved by c00ac1a in pull request #71

[alallier]

Closes by #118