feat: getRouter argument for app function (`({ app, getRouter }) =>…

… {}`) (#1406)

docs/http.md

@@ -4,12 +4,12 @@ next: docs/simulating-webhooks.md
 
 # HTTP routes
 
-Calling `app.route('/my-app')` will return an [express](http://expressjs.com/) router that you can use to expose HTTP endpoints from your app.
+Calling `getRouter('/my-app')` will return an [express](http://expressjs.com/) router that you can use to expose HTTP endpoints from your app.
 
 ```js
-module.exports = ({ app }) => {
+module.exports = ({ app, getRouter }) => {
   // Get an express router to expose new HTTP endpoints
-  const router = app.route("/my-app");
+  const router = getRouter("/my-app");
 
   // Use any middleware
   router.use(require("express").static("public"));
@@ -23,6 +23,6 @@ module.exports = ({ app }) => {
 
 Visit https://localhost:3000/my-app/hello-world to access the endpoint.
 
-It is strongly encouraged to use the name of your package as the prefix so none of your routes or middleware conflict with other apps. For example, if [`probot/owners`](https://github.com/probot/owners) exposed an endpoint, the app would call `app.route('/owners')` to prefix all endpoints with `/owners`.
+It is strongly encouraged to use the name of your package as the prefix so none of your routes or middleware conflict with other apps. For example, if [`probot/owners`](https://github.com/probot/owners) exposed an endpoint, the app would call `getRouter('/owners')` to prefix all endpoints with `/owners`.
 
 See the [express documentation](http://expressjs.com/en/guide/routing.html) for more information.

docs/logging.md

@@ -20,7 +20,7 @@ module.exports = ({ app }) => {
     }
   });
 
-  app.route().get("/hello-world", (req, res) => {
+  route().get("/hello-world", (req, res) => {
     req.log.info("Someone is saying hello");
   });
 };

src/application.ts

@@ -1,6 +1,7 @@
-import express from "express";
+import { Router } from "express";
 import Redis from "ioredis";
 import LRUCache from "lru-cache";
+import { Deprecation } from "deprecation";
 
 import type { Webhooks } from "@octokit/webhooks";
 import type { Logger } from "pino";
@@ -19,7 +20,7 @@ import { webhookEventCheck } from "./helpers/webhook-event-check";
 import { aliasLog } from "./helpers/alias-log";
 import { getWebhooks } from "./octokit/get-webhooks";
 import { load } from "./load";
-import { route } from "./route";
+import { getRouter } from "./get-router";
 import { auth } from "./auth";
 
 export interface Options {
@@ -37,6 +38,7 @@ export interface Options {
   cache?: LRUCache<number, string>;
   octokit?: InstanceType<typeof ProbotOctokit>;
   webhooks?: Webhooks;
+  router?: Router;
 
   /**
    * @deprecated set `Octokit` to `ProbotOctokit.defaults({ throttle })` instead
@@ -52,21 +54,20 @@ export type OnCallback<T> = (context: Context<T>) => Promise<void>;
  * @property {logger} log - A logger
  */
 export class Application {
-  public router: express.Router;
   public log: DeprecatedLogger;
   public on: ProbotWebhooks["on"];
   public receive: ProbotWebhooks["receive"];
   public load: (
     appFn: ApplicationFunction | ApplicationFunction[]
   ) => Application;
-  public route: (path?: string) => express.Router;
   public auth: (
     installationId?: number,
     log?: Logger
   ) => Promise<InstanceType<typeof ProbotOctokit>>;
 
   private webhooks: ProbotWebhooks;
   private state: State;
+  private internalRouter: Router;
 
   constructor(options: Options) {
     this.log = aliasLog(options.log || getLog());
@@ -110,8 +111,6 @@ export class Application {
       },
     };
 
-    this.router = express.Router();
-
     this.webhooks = options.webhooks || getWebhooks(this.state);
 
     this.on = (eventNameOrNames, callback) => {
@@ -130,8 +129,56 @@ export class Application {
     };
     this.receive = this.webhooks.receive;
 
-    this.load = load.bind(null, this);
-    this.route = route.bind(null, this);
+    const router = options.router || Router();
+    this.load = load.bind(null, this, router);
     this.auth = auth.bind(null, this.state);
+
+    this.internalRouter = router;
+  }
+
+  /**
+   * @deprecated "app.router" is deprecated, use "getRouter()" from the app function instead: "({ app, getRouter }) => { ... }"
+   */
+  public get router() {
+    this.log.warn(
+      new Deprecation(
+        `[probot] "app.router" is deprecated, use "getRouter()" from the app function instead: "({ app, getRouter }) => { ... }"`
+      )
+    );
+
+    return this.internalRouter;
+  }
+
+  /**
+   * Get an {@link http://expressjs.com|express} router that can be used to
+   * expose HTTP endpoints
+   *
+   * ```
+   * module.exports = ({ app, getRouter }) => {
+   *   // Get an express router to expose new HTTP endpoints
+   *   const router = getRouter('/my-app');
+   *
+   *   // Use any middleware
+   *   router.use(require('express').static(__dirname + '/public'));
+   *
+   *   // Add a new route
+   *   router.get('/hello-world', (req, res) => {
+   *     res.end('Hello World');
+   *   });
+   * };
+   * ```
+   *
+   * @param path - the prefix for the routes* @param path
+   *
+   * @deprecated "app.route()" is deprecated, use the "getRouter()" argument from the app function instead: "({ app, getRouter }) => { ... }"
+   */
+  route(path?: string) {
+    this.log.warn(
+      new Deprecation(
+        `[probot] "app.route()" is deprecated, use the "getRouter()" argument from the app function instead: "({ app, getRouter }) => { ... }"`
+      )
+    );
+
+    return getRouter(this.internalRouter, path);
   }
 }

src/apps/default.ts

@@ -1,10 +1,17 @@
 import path from "path";
+import express from "express";
 import { Application } from "../application";
 
-export = ({ app }: { app: Application }) => {
-  const route = app.route();
+export = ({
+  app,
+  getRouter,
+}: {
+  app: Application;
+  getRouter: () => express.Router;
+}) => {
+  const router = getRouter();
 
-  route.get("/probot", (req, res) => {
+  router.get("/probot", (req, res) => {
     let pkg;
     try {
       pkg = require(path.join(process.cwd(), "package.json"));
@@ -14,5 +21,5 @@ export = ({ app }: { app: Application }) => {
 
     res.render("probot.hbs", pkg);
   });
-  route.get("/", (req, res, next) => res.redirect("/probot"));
+  router.get("/", (req, res, next) => res.redirect("/probot"));
 };

src/get-router.ts

@@ -0,0 +1,18 @@
+import { Router } from "express";
+
+/**
+ * Get an {@link http://expressjs.com|express} router that can be used to
+ * expose HTTP endpoints
+ *
+ * @param path - the prefix for the routes
+ * @returns an [express.Router](http://expressjs.com/en/4x/api.html#router)
+ */
+export function getRouter(router: Router, path?: string): Router {
+  if (path) {
+    const newRouter = Router();
+    router.use(path, newRouter);
+    return newRouter;
+  }
+
+  return router;
+}

src/index.ts

@@ -270,6 +270,7 @@ export class Probot {
       appFn = resolveAppFunction(appFn) as ApplicationFunction;
     }
 
+    const router = express.Router();
     const app = new Application({
       id: this.state.id,
       privateKey: this.state.privateKey,
@@ -280,10 +281,11 @@ export class Probot {
       octokit: this.state.octokit,
       throttleOptions: this.throttleOptions,
       webhooks: this.webhooks,
+      router,
     });
 
     // Connect the router from the app to the server
-    this.server.use(app.router);
+    this.server.use(router);
 
     // Initialize the ApplicationFunction
     app.load(appFn);

src/load.ts

@@ -1,24 +1,18 @@
 import { Deprecation } from "deprecation";
+import { Router } from "express";
 
 import { Application } from "./application";
 import { ApplicationFunction, ApplicationFunctionOptions } from "./types";
+import { getRouter } from "./get-router";
 
-type DeprecatedKey =
-  | "auth"
-  | "load"
-  | "log"
-  | "on"
-  | "receive"
-  | "route"
-  | "router";
+type DeprecatedKey = "auth" | "load" | "log" | "on" | "receive" | "router";
 
 const DEPRECATED_APP_KEYS: DeprecatedKey[] = [
   "auth",
   "load",
   "log",
   "on",
   "receive",
-  "route",
   "router",
 ];
 let didDeprecate = false;
@@ -29,6 +23,7 @@ let didDeprecate = false;
  */
 export function load(
   app: Application,
+  router: Router | null,
   appFn: ApplicationFunction | ApplicationFunction[]
 ) {
   const deprecatedApp = DEPRECATED_APP_KEYS.reduce(
@@ -54,11 +49,12 @@ export function load(
   );
 
   if (Array.isArray(appFn)) {
-    appFn.forEach((fn) => load(app, fn));
+    appFn.forEach((fn) => load(app, router, fn));
   } else {
     appFn(
       (Object.assign(deprecatedApp, {
         app,
+        getRouter: getRouter.bind(null, router || app.router),
       }) as unknown) as ApplicationFunctionOptions
     );
   }

src/types.ts

@@ -1,3 +1,4 @@
+import express from "express";
 import { WebhookEvent, Webhooks } from "@octokit/webhooks";
 import LRUCache from "lru-cache";
 
@@ -41,5 +42,5 @@ export type ApplicationFunctionOptions = {
    * @deprecated "(app) => {}" is deprecated. Use "({ app }) => {}" instead.
    */
   [K in deprecatedKeys]: Application[K];
-} & { app: Application };
+} & { app: Application; getRouter: (path?: string) => express.Router };
 export type ApplicationFunction = (options: ApplicationFunctionOptions) => void;