fix: Ensure render errors are caught before success messaging sent (#49)

Author: jahredhope

.eslintrc.js

@@ -7,18 +7,18 @@ module.exports = {
       files: ["**/*.js"],
       rules: {
         "@typescript-eslint/no-var-requires": "off",
-        "@typescript-eslint/explicit-function-return-type": "off"
-      }
-    }
+        "@typescript-eslint/explicit-function-return-type": "off",
+      },
+    },
   ],
   parserOptions: {
     ecmaVersion: 2018,
-    sourceType: "module"
+    sourceType: "module",
   },
   env: {
     es6: true,
     jest: true,
-    node: true
+    node: true,
   },
   rules: {
     "@typescript-eslint/ban-ts-ignore": "warn",
@@ -27,6 +27,6 @@ module.exports = {
     "@typescript-eslint/no-non-null-assertion": "off",
     "@typescript-eslint/explicit-function-return-type": "off",
     "no-console": "off",
-    "no-inner-declarations": "off"
-  }
+    "no-inner-declarations": "off",
+  },
 };

jest.config.js

@@ -2,5 +2,5 @@ module.exports = {
   testEnvironment: "node",
   transform: {},
   watchPathIgnorePatterns: ["/dist/"],
-  preset: "ts-jest"
+  preset: "ts-jest",
 };

package.json

@@ -15,12 +15,12 @@
     "build": "tsc --declaration",
     "commit": "git-cz",
     "lint.eslint": "eslint src/**/*.ts",
-    "lint.prettier": "prettier --list-different \"**/*.js\"",
+    "lint.prettier": "prettier --check \"**/*.(js|ts)\"",
     "lint": "npm run lint.eslint && npm run lint.prettier",
     "jest": "jest",
     "prepublishOnly": "npm run build",
     "test": "npm run lint && npm run jest",
-    "format": "prettier --write \"**/*.js\"",
+    "format": "prettier --write \"**/*.(js|ts)\"",
     "release": "semantic-release"
   },
   "repository": {
@@ -32,29 +32,29 @@
   },
   "devDependencies": {
     "@types/debug": "^4.1.5",
-    "@types/express": "^4.17.2",
-    "@types/jest": "^25.1.3",
+    "@types/express": "^4.17.3",
+    "@types/jest": "^25.1.4",
     "@types/memory-fs": "^0.3.2",
     "@types/schema-utils": "^2.4.0",
-    "@types/webpack": "^4.41.6",
-    "@typescript-eslint/eslint-plugin": "^2.20.0",
-    "@typescript-eslint/parser": "^2.20.0",
+    "@types/webpack": "^4.41.8",
+    "@typescript-eslint/eslint-plugin": "^2.25.0",
+    "@typescript-eslint/parser": "^2.25.0",
     "commitizen": "4.0.3",
     "cz-conventional-changelog": "3.1.0",
     "eslint": "6.8.0",
     "exception-formatter": "^2.1.2",
     "express": "^4.17.1",
-    "jest": "25.1.0",
+    "jest": "25.2.3",
     "memoizee": "^0.4.14",
     "memory-fs": "^0.5.0",
-    "prettier": "1.19.1",
-    "react": "^16.12.0",
-    "react-dom": "^16.12.0",
+    "prettier": "2.0.2",
+    "react": "^16.13.1",
+    "react-dom": "^16.13.1",
     "semantic-release": "17.0.4",
     "ts-jest": "^25.2.1",
-    "ts-node": "^8.6.2",
-    "typescript": "^3.8.2",
-    "webpack": "4.41.6",
+    "ts-node": "^8.8.1",
+    "typescript": "^3.8.3",
+    "webpack": "4.42.1",
     "webpack-merge": "4.2.2"
   },
   "dependencies": {

src/common-types.ts

@@ -17,7 +17,7 @@ export type Specifier = string;
 export type ExtraGlobals = object;
 export type RenderConcurrency = "parallel" | "serial";
 export type MapStatsToParams = ({
-  webpackStats
+  webpackStats,
 }: {
   webpackStats: WebpackStats;
 }) => object;

src/createDevRouter.ts

@@ -6,7 +6,7 @@ import {
   BaseRoute,
   WebpackStats,
   TransformExpressPath,
-  GetRouteFromRequest
+  GetRouteFromRequest,
 } from "common-types";
 import { Stats } from "webpack";
 
@@ -23,7 +23,7 @@ export = <Route extends BaseRoute>({
   getRouteFromRequest,
   onRendererReady,
   transformExpressPath,
-  getClientStats
+  getClientStats,
 }: Params<Route>) => {
   log("Create dev server");
   const formatErrorResponse = (
@@ -36,7 +36,8 @@ export = <Route extends BaseRoute>({
         const devServerAssets = webpackStats.entrypoints.main.assets;
 
         devServerScripts = devServerAssets.map(
-          asset => `<script src="${webpackStats.publicPath}${asset}"></script>`
+          (asset) =>
+            `<script src="${webpackStats.publicPath}${asset}"></script>`
         );
       } catch (err) {
         console.error("Unable to load Dev Server Scripts. Error: ", err);
@@ -51,7 +52,7 @@ export = <Route extends BaseRoute>({
   const routesByExpressPath: Record<string, Route> = {};
 
   // Deduplicate paths to avoid duplicated processing in Express
-  routes.forEach(route => {
+  routes.forEach((route) => {
     const expressPath = transformExpressPath(route);
     if (expressPath) {
       routesByExpressPath[expressPath] = route;
@@ -62,7 +63,7 @@ export = <Route extends BaseRoute>({
     devServerRouter.get(
       expressPath,
       async (req: Request, res: Response, next: NextFunction) => {
-        onRendererReady(async render => {
+        onRendererReady(async (render) => {
           const route = getRouteFromRequest
             ? getRouteFromRequest(req, routes)
             : defaultRoute;
@@ -84,7 +85,7 @@ export = <Route extends BaseRoute>({
                 exceptionFormatter(error, {
                   format: "html",
                   inlineStyle: true,
-                  basepath: "webpack://static/./"
+                  basepath: "webpack://static/./",
                 }),
                 getClientStats().toJson()
               )

src/createRenderer.ts

@@ -4,7 +4,7 @@ import { SourceModules, ExtraGlobals } from "common-types";
 export = function createRenderer({
   fileName,
   source,
-  extraGlobals
+  extraGlobals,
 }: {
   fileName: string;
   source: SourceModules;

src/evalutateFromSource.ts

@@ -28,7 +28,7 @@ function evalutateFromSource(
       console,
       process,
       ...(extraGlobals || {}),
-      require: createLinker(specifier, sourceModules, extraGlobals)
+      require: createLinker(specifier, sourceModules, extraGlobals),
     },
     /* includeGlobals: */ true
   );

src/index.ts

@@ -18,7 +18,7 @@ import {
   TransformExpressPath,
   TransformPath,
   WebpackStats,
-  GetRouteFromRequest
+  GetRouteFromRequest,
 } from "./common-types";
 import { Compiler, compilation, Stats } from "webpack";
 import getSourceFromCompilation from "./getSourceFromCompilation";
@@ -64,10 +64,10 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
       getRouteFromRequest,
       transformFilePath = defaultTransform,
       transformExpressPath = defaultTransform,
-      renderConcurrency = "serial"
+      renderConcurrency = "serial",
     } = options;
 
-    const routes: Route[] = (options.routes || [""]).map(route =>
+    const routes: Route[] = (options.routes || [""]).map((route) =>
       typeof route === "string" ? ({ route } as Route) : route
     );
 
@@ -82,7 +82,9 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
     const isBuildReady = () =>
       rendererCompilation &&
       rendererCompilation.isReady &&
-      clientCompilations.every(compilationStatus => compilationStatus.isReady);
+      clientCompilations.every(
+        (compilationStatus) => compilationStatus.isReady
+      );
     const isRendererReady = () => isBuildReady() && renderer;
 
     const renderQueue: Array<() => void> = [];
@@ -101,11 +103,11 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
         ...route,
         ...mapStatsToParams({
           ...route,
-          webpackStats
-        })
+          webpackStats,
+        }),
       };
       try {
-        const result = renderer(renderParams);
+        const result = await renderer(renderParams);
 
         log(
           `Successfully rendered ${route.route} (${timeSince(startRenderTime)})`
@@ -134,7 +136,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
           renderDirectory,
           renderEntry,
           routes,
-          transformFilePath
+          transformFilePath,
         });
       } catch (error) {
         logError("An error occured rendering HTML", error);
@@ -177,9 +179,9 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
           ? clientCompilations[0].compilation!.getStats()
           : new MultiStats(
               clientCompilations
-                .map(compilationStatus => compilationStatus.compilation)
+                .map((compilationStatus) => compilationStatus.compilation)
                 .filter(Boolean)
-                .map(compilation => compilation!.getStats())
+                .map((compilation) => compilation!.getStats())
             );
 
       lastClientStats = clientStats;
@@ -193,7 +195,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
       }
     };
 
-    const onRendererReady: OnRendererReady<Route> = cb => {
+    const onRendererReady: OnRendererReady<Route> = (cb) => {
       if (isRendererReady()) {
         cb(render);
       } else {
@@ -214,7 +216,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
       renderer = createRenderer({
         source,
         fileName: renderEntry,
-        extraGlobals
+        extraGlobals,
       });
 
       if (typeof renderer !== "function") {
@@ -234,7 +236,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
 
       const compilationStatus: CompilationStatus = {
         compilation: null,
-        isReady: false
+        isReady: false,
       };
 
       if (isRenderer) {
@@ -257,7 +259,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
         compilationStatus.isReady = false;
       });
 
-      compiler.hooks.afterEmit.tapPromise(pluginName, async compilation => {
+      compiler.hooks.afterEmit.tapPromise(pluginName, async (compilation) => {
         log(`Assets emitted for ${compilerName}.`);
         compilationStatus.compilation = compilation;
         lastClientStats = null;
@@ -289,7 +291,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
         getRouteFromRequest,
         onRendererReady,
         getClientStats,
-        routes
+        routes,
       });
 
     this.renderWhenReady = (route: Route) =>

src/renderRoutes.ts

@@ -20,7 +20,7 @@ export default async function renderRoutes<Route>({
   routes,
   renderDirectory,
   renderCompilation,
-  transformFilePath
+  transformFilePath,
 }: Params<Route>) {
   log(`Starting render of ${routes.length} routes`);
   async function emitFile(dir: string, content: string) {
@@ -40,7 +40,7 @@ export default async function renderRoutes<Route>({
       renderCompilation.compiler.outputFileSystem.writeFile(
         dir,
         content,
-        error => {
+        (error) => {
           if (error) {
             reject(error);
           }

tests/test-cases/async/async.test.js

@@ -9,7 +9,7 @@ const getDirContentsSync = require("../../utils/getDirContentsSync");
 describe("Render asyncronously", () => {
   const renderDirectory = path.join(process.cwd(), "dist", "render");
 
-  it("should render a HTML once resolved", async done => {
+  it("should render a HTML once resolved", async (done) => {
     const compiler = webpack(
       config(
         new HtmlRenderPlugin({ mapStatsToParams: () => ({}), renderDirectory })
@@ -25,23 +25,23 @@ describe("Render asyncronously", () => {
       done();
     });
   });
-  it("should render a multiple files at once", async done => {
+  it("should render a multiple files at once", async (done) => {
     jest.setTimeout(1000);
     const compiler = webpack(
       config(
         new HtmlRenderPlugin({
           mapStatsToParams: () => ({}),
           renderConcurrency: "parallel",
           routes: new Array(20).fill(null).map((_, i) => `page${i}`),
-          renderDirectory
+          renderDirectory,
         })
       )
     );
 
     const memoryFs = new MemoryFS();
     compiler.outputFileSystem = memoryFs;
 
-    compiler.run(error => {
+    compiler.run((error) => {
       expect(error).toBe(null);
       const contents = getDirContentsSync(renderDirectory, { fs: memoryFs });
       expect(contents).toMatchSnapshot();