Skip to content

Commit

Permalink
fix: Ensure render errors are caught before success messaging sent (#49)
Browse files Browse the repository at this point in the history
  • Loading branch information
jahredhope authored Mar 27, 2020
1 parent 400c8bf commit e1f9ef3
Show file tree
Hide file tree
Showing 46 changed files with 1,268 additions and 1,125 deletions.
14 changes: 7 additions & 7 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
},
};
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ module.exports = {
testEnvironment: "node",
transform: {},
watchPathIgnorePatterns: ["/dist/"],
preset: "ts-jest"
preset: "ts-jest",
};
28 changes: 14 additions & 14 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion src/common-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 7 additions & 6 deletions src/createDevRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
BaseRoute,
WebpackStats,
TransformExpressPath,
GetRouteFromRequest
GetRouteFromRequest,
} from "common-types";
import { Stats } from "webpack";

Expand All @@ -23,7 +23,7 @@ export = <Route extends BaseRoute>({
getRouteFromRequest,
onRendererReady,
transformExpressPath,
getClientStats
getClientStats,
}: Params<Route>) => {
log("Create dev server");
const formatErrorResponse = (
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -84,7 +85,7 @@ export = <Route extends BaseRoute>({
exceptionFormatter(error, {
format: "html",
inlineStyle: true,
basepath: "webpack://static/./"
basepath: "webpack://static/./",
}),
getClientStats().toJson()
)
Expand Down
2 changes: 1 addition & 1 deletion src/createRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { SourceModules, ExtraGlobals } from "common-types";
export = function createRenderer({
fileName,
source,
extraGlobals
extraGlobals,
}: {
fileName: string;
source: SourceModules;
Expand Down
2 changes: 1 addition & 1 deletion src/evalutateFromSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function evalutateFromSource(
console,
process,
...(extraGlobals || {}),
require: createLinker(specifier, sourceModules, extraGlobals)
require: createLinker(specifier, sourceModules, extraGlobals),
},
/* includeGlobals: */ true
);
Expand Down
32 changes: 17 additions & 15 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
TransformExpressPath,
TransformPath,
WebpackStats,
GetRouteFromRequest
GetRouteFromRequest,
} from "./common-types";
import { Compiler, compilation, Stats } from "webpack";
import getSourceFromCompilation from "./getSourceFromCompilation";
Expand Down Expand Up @@ -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
);

Expand All @@ -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> = [];
Expand All @@ -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)})`
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -214,7 +216,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
renderer = createRenderer({
source,
fileName: renderEntry,
extraGlobals
extraGlobals,
});

if (typeof renderer !== "function") {
Expand All @@ -234,7 +236,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {

const compilationStatus: CompilationStatus = {
compilation: null,
isReady: false
isReady: false,
};

if (isRenderer) {
Expand All @@ -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;
Expand Down Expand Up @@ -289,7 +291,7 @@ export = class HtmlRenderPlugin<Route extends BaseRoute = BaseRoute> {
getRouteFromRequest,
onRendererReady,
getClientStats,
routes
routes,
});

this.renderWhenReady = (route: Route) =>
Expand Down
4 changes: 2 additions & 2 deletions src/renderRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -40,7 +40,7 @@ export default async function renderRoutes<Route>({
renderCompilation.compiler.outputFileSystem.writeFile(
dir,
content,
error => {
(error) => {
if (error) {
reject(error);
}
Expand Down
8 changes: 4 additions & 4 deletions tests/test-cases/async/async.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion tests/test-cases/async/src/render.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export default async function exampleRender(...params) {
await new Promise(resolve => setTimeout(resolve, 300));
await new Promise((resolve) => setTimeout(resolve, 300));
return `<html>
<body>
Rendered with:&nbsp;
Expand Down
14 changes: 7 additions & 7 deletions tests/test-cases/async/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ const path = require("path");
const srcPath = path.resolve(__dirname, "./src");
const paths = {
renderEntry: path.resolve(srcPath, "render.js"),
clientEntry: path.resolve(srcPath, "client.js")
clientEntry: path.resolve(srcPath, "client.js"),
};

module.exports = plugin => [
module.exports = (plugin) => [
{
name: "client",
target: "web",
mode: "production",
entry: paths.clientEntry,
output: {
filename: "client-[name]-[contenthash].js"
filename: "client-[name]-[contenthash].js",
},
plugins: [plugin]
plugins: [plugin],
},
{
dependencies: ["client"],
Expand All @@ -27,8 +27,8 @@ module.exports = plugin => [
libraryExport: "default",
library: "static",
libraryTarget: "umd2",
filename: "render-[name]-[contenthash].js"
filename: "render-[name]-[contenthash].js",
},
plugins: [plugin.render()]
}
plugins: [plugin.render()],
},
];
2 changes: 1 addition & 1 deletion tests/test-cases/errors/errors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const MemoryFS = require("memory-fs");
const webpack = require("webpack");

describe("Render HTML from in-config Plugin", () => {
it("should render a HTML file", async done => {
it("should render a HTML file", async (done) => {
const compiler = webpack(require("./webpack.errors.config"));

const memoryFs = new MemoryFS();
Expand Down
Loading

0 comments on commit e1f9ef3

Please sign in to comment.