yet more

kethinov · Jan 10, 2025 · cc4fb86 · cc4fb86
1 parent df23fb4
commit cc4fb86
Show file tree

Hide file tree

Showing 10 changed files with 22 additions and 208 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 - Breaking: Removed `app.get('routes')`.
 - Breaking: `app.httpServer` is now `app.get('httpServer')` and `app.httpsServer` is now `app.get('httpsServer')`.
+- Breaking: Removed `onReqAfterRoute`, `onReqBeforeRoute`, `onReqStart`.
+- Added `start` and `stop` method shorthands for `startServer` and `stopServer` respectively.
 - Stopped the `statics/pages` directory from being created automatically if it doesn't exist.
 - Refactored various things under the hood to improve code quality, performance, and reduce unnecessary dependencies.
 - Updated various dependencies.

diff --git a/README.md b/README.md
@@ -1025,17 +1025,6 @@ require('roosevelt')({
   - `app`: The [Express app](http://expressjs.com/api.html#express) created by Roosevelt.
 - `onAppExit(app)`: Fired when the app recieves a kill signal.
   - `app`: The [Express app](http://expressjs.com/api.html#express) created by Roosevelt.
-- `onReqStart(req, res, next)`: Fired at the beginning of each new request.
-  - `req`: The [request object](http://expressjs.com/api.html#req.params) created by Express.
-  - `res`: The [response object](http://expressjs.com/api.html#res.status) created by Express.
-  - `next`: Callback to continue with the request. Must be called to continue the request.
-- `onReqBeforeRoute(req, res, next)`: Fired just before executing the controller.
-  - `req`: The [request object](http://expressjs.com/api.html#req.params) created by Express.
-  - `res`: The [response object](http://expressjs.com/api.html#res.status) created by Express.
-  - `next`: Callback to continue with the request. Must be called to continue the request.
-- `onReqAfterRoute(req, res)`: Fired after the request ends.
-  - `req`: The [request object](http://expressjs.com/api.html#req.params) created by Express.
-  - `res`: The [response object](http://expressjs.com/api.html#res.status) created by Express.
 - `onClientViewsProcess(template)`: Fired to preprocess templates before being exposed to the client.
   - `template`: A string containing a template written in any JS-based templating engine (e.g. Teddy, Pug, ejs, etc)
 
@@ -1312,12 +1301,12 @@ Additionally the Roosevelt constructor returns the following object:
 | Roosevelt constructor returned object members | Description                                                  |
 | ------------------------ | ------------------------------------------------------------ |
 | `expressApp`             | *[Object]* The [Express app](http://expressjs.com/api.html#express) created by Roosevelt. |
-| `httpServer`             | *[Object]* The [http server](https://nodejs.org/api/http.html#http_class_http_server) created by Roosevelt. |
-| `httpsServer`            | *[Object]* The [https server](https://nodejs.org/api/https.html#https_class_https_server) created by Roosevelt. |
 | `initServer(callback)`   | *[Method]* Starts the HTML validator, sets up some middleware, runs the CSS and JS preprocessors, and maps routes, but does not start the HTTP server. Call this method manually first instead of `startServer` if you need to setup the Express app, but still need to do additional setup before the HTTP server is started. This method is automatically called by `startServer` once per instance if it has not yet already been called. Takes an optional callback. |
 | `init`                   | *[Method]* Shorthand for `initServer`. |
 | `startServer`            | *[Method]* Calls the `listen` method of `http`, `https`, or both (depending on your configuration) to start the web server with Roosevelt's config. |
+| `start`                  | *[Method]* Shorthand for `startServer`. |
 | `stopServer(params)`     | *[Method]* Stops the server from accepting new connections before exiting and takes an optional argument `stopServer({persistProcess: true})` which will allow the process to remain active after the server has closed. |
+| `stop`                   | *[Method]* Shorthand for `stopServer`. |
 
 ## Supplying your own CSS preprocessor
 

diff --git a/lib/injectReload.js b/lib/injectReload.js
@@ -1,5 +1,4 @@
 // injects <script> tag containing "reload.js"
-// TODO: does this really need to be its own file?
 module.exports = app => {
   const reloadParams = app.get('params').frontendReload
 

diff --git a/lib/mapRoutes.js b/lib/mapRoutes.js
@@ -36,28 +36,11 @@ module.exports = app => {
     params.errorPages.serviceUnavailable = path.join(__dirname, '../defaultErrorPages/controllers/503')
   }
 
-  // bind user-defined middleware which fires just before executing the controller if supplied
-  if (params.onReqBeforeRoute && typeof params.onReqBeforeRoute === 'function') {
-    app.use(params.onReqBeforeRoute)
-  }
-
   // enable multipart
   if (typeof params.formidable === 'object') {
-    // TODO: why was app being redefined here?
     require('./enableMultipart.js')(app)
   }
 
-  // bind user-defined middleware which fires after request ends if supplied
-  // TODO: investigate if these are being bound correctly or if they will fire multiple times
-  if (params.onReqAfterRoute && typeof params.onReqAfterRoute === 'function') {
-    app.use((req, res, next) => {
-      res.on('finish', function () { params.onReqAfterRoute(req, res) })
-      res.on('close', function () { params.onReqAfterRoute(req, res) })
-      res.on('error', function () { params.onReqAfterRoute(req, res) })
-      next()
-    })
-  }
-
   // generate mvc directories
   if (params.makeBuildArtifacts && params.makeBuildArtifacts !== 'staticsOnly') {
     fsr.ensureDirSync(app.get('modelsPath'))
@@ -71,7 +54,7 @@ module.exports = app => {
   }
 
   // optional require designed to fail silently allowing || chaining
-  router.isoRequire = (module) => {
+  router.isoRequire = module => {
     try {
       // the paths added by appModulePath in sourceParams aren't automatically available here because of https://github.com/patrick-steele-idem/app-module-path-node/issues/17
       // thus we have to examine each path manually so the shorthands still work in isoRequire the same as they would in require
@@ -96,7 +79,7 @@ module.exports = app => {
         return {}
       }
     } catch (e) {
-      console.error(e)
+      logger.error(e)
       return () => {
         logger.error(`isoRequire was unable to load module: ${module}`)
         return {}
@@ -116,7 +99,7 @@ module.exports = app => {
   }
 
   // convenience method to determine if it's a server-side render
-  router.ssr = router.serverSideRender = (req) => {
+  router.ssr = router.serverSideRender = req => {
     if (req.headers && req.headers['content-type'] === 'application/json') {
       return false
     } else {

diff --git a/lib/preprocessCss.js b/lib/preprocessCss.js
@@ -214,7 +214,6 @@ module.exports = async app => {
     if (file !== '.' && file !== '..' && !gitignoreFiles.includes(path.basename(file)) && !gitignoreFiles.includes(file) && !fs.lstatSync(usingAllowlist ? path.join(cssPath, file.split(':')[0]) : file).isDirectory()) {
       // generate a promise for each file
       promises.push(
-        // TODO: consider refactoring this confusing data structure
         new Promise((resolve, reject) => {
           (async () => {
             let split

diff --git a/lib/roosevelt-router.js b/lib/roosevelt-router.js
@@ -70,7 +70,7 @@ module.exports = (params = {}) => {
   })
 
   // optional require designed to fail silently allowing || chaining
-  router.isoRequire = (module) => {
+  router.isoRequire = module => {
     try {
       return require(module)
     } catch (e) {
@@ -82,7 +82,7 @@ module.exports = (params = {}) => {
   }
 
   // convenience method for handling form submits to the server in a SPA-friendly way; boilerplate written for you
-  router.onSubmit = (cb) => {
+  router.onSubmit = cb => {
     if (!document.routerOnSubmitAlreadyAdded) {
       document.routerOnSubmitAlreadyAdded = true
       document.addEventListener('submit', function (e) {

diff --git a/lib/sourceParams.js b/lib/sourceParams.js
@@ -380,15 +380,6 @@ module.exports = (params, appSchema) => {
     onAppExit: {
       default: {}
     },
-    onReqStart: {
-      default: {}
-    },
-    onReqBeforeRoute: {
-      default: {}
-    },
-    onReqAfterRoute: {
-      default: {}
-    },
     onClientViewsProcess: {
       default: {}
     },

diff --git a/roosevelt.js b/roosevelt.js
@@ -212,11 +212,6 @@ const roosevelt = (options = {}, schema) => {
       else logger.warn(`Favicon ${params.favicon} does not exist. Please ensure the "favicon" param is configured correctly.`)
     }
 
-    // bind user-defined middleware which fires at the beginning of each request if supplied
-    if (params.onReqStart && typeof params.onReqStart === 'function') {
-      app.use(params.onReqStart)
-    }
-
     // configure express, express-session, and csrf
     require('./lib/setExpressConfigs')(app)
 
@@ -330,20 +325,23 @@ const roosevelt = (options = {}, schema) => {
     }
 
     if (params.makeBuildArtifacts !== 'staticsOnly') {
-      try {
-        if (!params.https.force || !params.https.enable) {
+      if (!params.https.force || !params.https.enable) {
+        try {
           await httpServer.listen(params.port, (params.localhostOnly ? 'localhost' : null), startupCallback('HTTP', params.port))
+        } catch (err) {
+          logger.error(err)
+          logger.error(`Another process is using port ${params.port}. Either kill that process or change this app's port number.`.bold)
+          process.exit(1)
         }
-        if (params.https.enable) {
+      }
+      if (params.https.enable) {
+        try {
           await httpsServer.listen(params.https.port, (params.localhostOnly ? 'localhost' : null), startupCallback('HTTPS', params.https.port))
+        } catch (err) {
+          logger.error(err)
+          logger.error(`Another process is using port ${params.https.port}. Either kill that process or change this app's port number.`.bold)
+          process.exit(1)
         }
-      } catch (err) {
-        logger.error(err)
-        if (err.message.includes('EADDRINUSE')) {
-          // TODO: figure out how to tell which server crashes and print the correct port
-          logger.error(`Another process is using port ${params.port}. Either kill that process or change this app's port number.`.bold)
-        }
-        process.exit(1)
       }
     }
 
@@ -410,9 +408,6 @@ const roosevelt = (options = {}, schema) => {
 
   return {
     expressApp: app,
-    // TODO: Since both of these server objects are stored in express variables we may be able to remove them from here
-    httpServer,
-    httpsServer,
     initServer,
     init: initServer,
     startServer,

diff --git a/test/paramFunctions.js b/test/paramFunctions.js
@@ -99,147 +99,6 @@ describe.skip('Parameter Function Tests', function () {
     })
   })
 
-  it('should execute what is in onReqStart', function (done) {
-    // bool var to hold whether or not the app had used its body parser middleware yet
-    let bodyParserNotUsedBool = false
-
-    // make options method go back to what it once was
-    options.method = 'startServer'
-
-    // generate the app
-    generateTestApp({
-      appDir,
-      makeBuildArtifacts: true,
-      csrfProtection: false,
-      onReqStart: '(req, res, next) => {console.log("body: " + JSON.stringify(req.body)); next()}',
-      onServerStart: '(app) => {process.send(app.get("params"))}'
-    }, options)
-
-    // fork the app and run it as a child process
-    const testApp = fork(path.join(appDir, 'app.js'), { stdio: ['pipe', 'pipe', 'pipe', 'ipc'] })
-
-    // check the console logs to see if the req body that will be logged out has a value or if its undefined
-    testApp.stdout.on('data', (data) => {
-      if (data.includes('body: undefined')) {
-        bodyParserNotUsedBool = true
-      }
-    })
-
-    // when the app starts and sends the message that "ServerStart", send a request and see if I get another message saying "ReqStart"
-    testApp.on('message', (params) => {
-      // send a http request
-      request(`http://localhost:${params.port}`)
-        .post('/paramPost')
-        .send({ name: 'Bob' })
-        .send({ age: '3' })
-        .expect(200, (err, res) => {
-          if (err) {
-            testApp.send('stop')
-            assert.fail(err)
-          }
-          testApp.send('stop')
-        })
-    })
-
-    // when the app is about to exit, check if the bool is true
-    testApp.on('exit', () => {
-      assert.strictEqual(bodyParserNotUsedBool, true, 'The Response that we got back from onReqStart shows that middleware was used before it was hit')
-      done()
-    })
-  })
-
-  it('should execute what is in onReqBeforeRoute', function (done) {
-    // bool var to hold whether or not the app had used its body parser middleware yet
-    let bodyParserUsedBool = false
-
-    // generate the app
-    generateTestApp({
-      appDir,
-      makeBuildArtifacts: true,
-      csrfProtection: false,
-      onReqBeforeRoute: '(req, res, next) => {console.log("body: " + JSON.stringify(req.body)); next()}',
-      onServerStart: '(app) => {process.send(app.get("params"))}'
-    }, options)
-
-    // fork the app and run it as a child process
-    const testApp = fork(path.join(appDir, 'app.js'), { stdio: ['pipe', 'pipe', 'pipe', 'ipc'] })
-
-    // check the console logs to see if the req body that will be logged out has a value or if its undefined
-    testApp.stdout.on('data', (data) => {
-      if (data.includes('body: {"name":"Bob","age":"3"}')) {
-        bodyParserUsedBool = true
-      }
-    })
-
-    testApp.on('message', (params) => {
-      // send a http request
-      request(`http://localhost:${params.port}`)
-        .post('/paramPost')
-        .send({ name: 'Bob' })
-        .send({ age: '3' })
-        .expect(200, (err, res) => {
-          if (err) {
-            testApp.send('stop')
-            assert.fail(err)
-          }
-          testApp.send('stop')
-        })
-    })
-
-    // when the child process exits, check assertions and finish the test
-    testApp.on('exit', () => {
-      assert.strictEqual(bodyParserUsedBool, true, 'The Response that we got back from onReqStart shows that middleware was not used before it was hit')
-      done()
-    })
-  })
-
-  it('should execute what is in onReqAfterRoute', function (done) {
-    // two bool to check if res.text has a value in it
-    let resHeaderUndefinedBool = false
-    let resHeaderValueBool = false
-
-    // generate the app
-    generateTestApp({
-      appDir,
-      makeBuildArtifacts: true,
-      csrfProtection: false,
-      onReqAfterRoute: '(req, res) => {console.log("Testing after: " + res.Testing)}',
-      onServerStart: '(app) => {process.send(app.get("params"))}'
-    }, options)
-
-    // fork the app and run it as a child process
-    const testApp = fork(path.join(appDir, 'app.js'), { stdio: ['pipe', 'pipe', 'pipe', 'ipc'] })
-
-    testApp.stdout.on('data', (data) => {
-      if (data.includes('Testing before: undefined')) {
-        resHeaderUndefinedBool = true
-      }
-      if (data.includes('Testing after: someValue')) {
-        resHeaderValueBool = true
-      }
-    })
-
-    testApp.on('message', (params) => {
-      // send a http request
-      request(`http://localhost:${params.port}`)
-        .get('/paramPostAfter')
-        .accept('application/json')
-        .expect(200, (err, res) => {
-          if (err) {
-            assert.fail(err)
-          }
-          testApp.send('stop')
-        })
-    })
-
-    // when the app is about to exit, check if both bool values are true
-    testApp.on('exit', () => {
-      assert.strictEqual(resHeaderUndefinedBool, true, 'response header should have no value as it was not assigned yet')
-      assert.strictEqual(resHeaderValueBool, true, 'response heade should have a value since it was assigned in route')
-      done()
-    })
-  })
-
   it('should throw an error if there is a controller that is not coded properly in the mvc', function (done) {
     // bool var to hold whether or not the controller errors logs are outputted
     let controllerErrorLogBool = false

diff --git a/test/sourceParams.js b/test/sourceParams.js
@@ -25,9 +25,6 @@ describe.skip('sourceParams', () => {
       'appDir',
       'cssCompiler',
       'onClientViewsProcess',
-      'onReqAfterRoute',
-      'onReqBeforeRoute',
-      'onReqStart',
       'onServerInit',
       'onStaticAssetsGenerated',
       'onServerStart',