Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compileOnStartup implementation missing #1006

Merged
merged 2 commits into from
May 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 13 additions & 2 deletions packages/generator-react-server/test/app.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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...");
Expand All @@ -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);
Expand All @@ -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, {
Expand Down Expand Up @@ -149,3 +159,4 @@ function installDeps() {
});
});
}
*/
40 changes: 35 additions & 5 deletions packages/react-server-cli/src/commands/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__);

Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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...");

Expand Down
29 changes: 15 additions & 14 deletions packages/react-server-cli/src/webpack/webpack4.config.fn.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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`,
Expand All @@ -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(),
]),
Expand Down