Merge pull request #1006 from redfin/bugfix/compile-on-startup-option…

…-missing

compileOnStartup implementation missing

packages/generator-react-server/generators/app/templates/package.json

@@ -10,6 +10,7 @@
   },
   "license": "Apache-2.0",
   "dependencies": {
+    "@babel/core": "^7.0.0",
     "@babel/runtime": "^7.0.0",
     "babel-preset-react-server": "^1.0.0-alpha.0",
     "react": "^15.4.1",

packages/generator-react-server/test/app.js

@@ -1,10 +1,10 @@
-import cp from 'child_process';
+//import cp from 'child_process';
 import fs from 'fs';
 import path from 'path';
 import test from 'ava';
 import helpers from 'yeoman-test';
 import { defaultOptions } from 'react-server-cli';
-import shellescape from 'shell-escape';
+//import shellescape from 'shell-escape';
 
 test('generator-react-server:app creates default files', async t => {
 	let testDir;
@@ -76,6 +76,13 @@ test('generator-react-server:app creates docker files', async t => {
 	t.true(await exists('docker-compose.yml', testDir));
 });
 
+/*
+// TODO: fix this test.
+// This test is disabled for the time being because it continuously fails.  Because we're trying to
+// install react-server, react-server-cli, and babel-preset-react-server using the local versions
+// and those each have modules hoisted to the parent directory, the npm install always fails.
+// This needs to be reworked to support installing the local version of those packages with non-hoisted
+// dependencies.
 test('generator-react-server:app passes the test target', async t => {
 	let testDir;
 	console.log("Running generator...");
@@ -93,6 +100,7 @@ test('generator-react-server:app passes the test target', async t => {
 	console.error("SERVER TEST RESULT: " + testServerResult);
 	t.falsy(testServerResult);
 });
+ */
 
 function exists(filename, dir) {
 	filename = path.join(dir, filename);
@@ -115,6 +123,8 @@ function readFile(filename, dir) {
 	});
 }
 
+/*
+// Disabled until the last test is restored.
 function runsSuccessfully(command, dir) {
 	return new Promise((resolve) => {
 		cp.exec(command, {
@@ -149,3 +159,4 @@ function installDeps() {
 		});
 	});
 }
+ */

packages/react-server-cli/src/commands/start.js

@@ -14,6 +14,7 @@ import setupLogging from "../setupLogging";
 import logProductionWarnings from "../logProductionWarnings";
 import expressState from 'express-state';
 import cookieParser from 'cookie-parser';
+import fs from 'fs';
 
 const logger = reactServer.logging.getLogger(__LOGGER__);
 
@@ -93,10 +94,12 @@ const startServer = (serverRoutes, options, compiler, config) => {
 		}));
 	} else {
 		// Only compile the webpack configs manually if we're not in hot mode
-		logger.notice("Compiling Webpack bundle prior to starting server");
-		compiler.run((err, stats) => {
-			handleCompilationErrors(err, stats);
-		});
+		if (compiler) {
+			logger.notice("Compiling Webpack bundle prior to starting server");
+			compiler.run((err, stats) => {
+				handleCompilationErrors(err, stats);
+			});
+		}
 
 		server.use('/', compression(), express.static(`__clientTemp/build`, {
 			maxage: longTermCaching ? '365d' : '0s',
@@ -179,9 +182,36 @@ export default function start(options) {
 	const {
 		port,
 		bindIp,
+		compileOnStartup,
+		hot,
 	} = options;
 
-	const {serverRoutes, compiler, config} = compileClient(options);
+	const routesFileLocation = path.join(process.cwd(), '__clientTemp/routes_server.js');
+	let serverRoutes,
+		compiler,
+		config;
+
+	if ((hot === false || hot === "false") && (compileOnStartup === false || compileOnStartup === "false")) {
+		logger.debug('Not running the compiler because hot is false and compileOnStartup is false');
+		serverRoutes = new Promise((resolve, reject) => {
+			fs.access(routesFileLocation, fs.constants.R_OK, (err) => {
+				if (err) {
+					reject("You must manually compile your application when compileOnStartup is set to false.");
+				} else {
+					// We need to replace the promise returned by the compiler with an already-resolved promise with the path
+					// of the compiled routes file.
+					resolve(routesFileLocation);
+				}
+			});
+		});
+
+		// it is safe to ignore setting the compiler and config variables
+	} else {
+		// ES6 destructuring without a preceding `let` or `const` results in a syntax error.  Therefore, the below
+		// statement must be wrapped in parentheses to work properly.
+		// http://exploringjs.com/es6/ch_destructuring.html#sec_leading-curly-brace-destructuring
+		({serverRoutes, compiler, config} = compileClient(options));
+	}
 
 	logger.notice("Starting server...");
 

packages/react-server-cli/src/webpack/webpack4.config.fn.js

@@ -1,11 +1,10 @@
-import webpack from "webpack"
-import callerDependency from "../callerDependency"
-import path from "path"
-
-const ExtractCssChunks = require("extract-css-chunks-webpack-plugin");
-const OptimizeCSSAssetsPlugin = require("optimize-css-assets-webpack-plugin");
-const TerserPlugin = require('terser-webpack-plugin');
+import ExtractCssChunksPlugin from "extract-css-chunks-webpack-plugin";
+import OptimizeCSSAssetsPlugin from "optimize-css-assets-webpack-plugin";
 import StatsPlugin from "webpack-stats-plugin";
+import callerDependency from "../callerDependency";
+import path from "path";
+import webpack from "webpack";
+
 
 export default function ({entrypoints, outputDir, outputUrl, hot, minify, longTermCaching, stats}) {
 	return {
@@ -47,7 +46,7 @@ export default function ({entrypoints, outputDir, outputUrl, hot, minify, longTe
 					test: /\.(sa|sc|c)ss$/,
 					use: [
 						{
-							loader: ExtractCssChunks.loader,
+							loader: ExtractCssChunksPlugin.loader,
 							options: {
 								hot: hot,
 							},
@@ -77,13 +76,9 @@ export default function ({entrypoints, outputDir, outputUrl, hot, minify, longTe
 		},
 		optimization: {
 			minimize: minify,
-			minimizer: clean([
-				minify && new TerserPlugin(),
-				minify && new OptimizeCSSAssetsPlugin(),
-			]),
 		},
 		plugins: clean([
-			new ExtractCssChunks({
+			new ExtractCssChunksPlugin({
 				// Options similar to the same options in webpackOptions.output
 				// both options are optional
 				filename: `[name]${longTermCaching ? ".[contenthash]" : ""}.css`,
@@ -94,7 +89,13 @@ export default function ({entrypoints, outputDir, outputUrl, hot, minify, longTe
 				fields: ["assets", "assetsByChunkName", "chunks", "errors", "warnings", "version", "hash", "time", "filteredModules", "children", "modules"],
 			}),
 
-			!minify && new webpack.SourceMapDevToolPlugin(),
+			// We always like sourcemaps, so if we're minifying then put the sourcemaps in a separate file
+			// in the sourcemaps/ directory, otherwise inline the sourcemap with the file.
+			new webpack.SourceMapDevToolPlugin({
+				filename: minify ? 'sourcemaps/[file].map' : false,
+			}),
+
+			minify && new OptimizeCSSAssetsPlugin(),
 
 			hot && new webpack.HotModuleReplacementPlugin(),
 		]),