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

Cli commands #490

Merged
merged 16 commits into from
Aug 3, 2016
Merged
Show file tree
Hide file tree
Changes from 10 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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ yo react-server
# compile and run the new app
npm run compile
npm run start
# go to http://localhost:3010
# go to http://localhost:3000
```

That hooks you up with [`react-server-cli`](packages/react-server-cli), which
Expand Down
283 changes: 197 additions & 86 deletions docs/guides/react-server-cli.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/testing-ports.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ simultaneously, so port conflicts will fail the tests. Make sure
to update this manifest when adding a test that starts a server.

## generator-react-server
3010, 3011
3000, 3001
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't 3001 conflict with react-server-integration-test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. I think react-server-integration-test uses a dummy static asset server.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, you're right. 😞
image


## react-server-integration-test
8771, 3001
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "A react-server instance",
"main": "HelloWorld.js",
"scripts": {
"start": "react-server --port 3010 --js-port 3011",
"start": "react-server start",
"test": "xo && nsp check && ava test.js"
},
"license": "Apache-2.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function getResponseCode(url) {
return new Promise((resolve, reject) => {
const req = http.get({
hostname: 'localhost',
port: 3010,
port: 3000,
path: url
}, res => {
resolve(res.statusCode);
Expand Down
1 change: 1 addition & 0 deletions packages/react-server-cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"json-loader": "^0.5.4",
"less": "^2.7.1",
"less-loader": "^2.2.3",
"lodash": "^4.14.1",
"mkdirp": "~0.5.1",
"node-libs-browser": "~0.5.2",
"null-loader": "~0.1.1",
Expand Down
13 changes: 13 additions & 0 deletions packages/react-server-cli/src/ConfigurationError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Can't use `instanceof` with babel-ified subclasses of builtins.
//
// https://phabricator.babeljs.io/T3083
//
// Gotta do this the old-fashioned way. :p
//
export default function ConfigurationError(message) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to consider using VError for our custom error classes

this.name = 'ConfigurationError';
this.message = message;
this.stack = (new Error()).stack;
}
ConfigurationError.prototype = Object.create(Error.prototype);
ConfigurationError.prototype.constructor = ConfigurationError;
4 changes: 2 additions & 2 deletions packages/react-server-cli/src/cli.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require("babel-core/register");

const {start, parseCliArgs} = require(".");
const {run, parseCliArgs} = require(".");

start(parseCliArgs());
run(parseCliArgs());
53 changes: 53 additions & 0 deletions packages/react-server-cli/src/commands/add-page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import _ from "lodash";
import fs from "fs";
import {join} from "path";
import chalk from "chalk";
import mkdirp from "mkdirp";
import fileExists from "../fileExists";
import ConfigurationError from "../ConfigurationError";

const PAGE_SOURCE = _.template(`
import React from "react";

export default class <%= className %> {
handleRoute(next) {
// Kick off data requests here.
return next();
}

getElements() {
return <div>This is <%= className %>.</div>
}
}
`);

export default function addPage(options){
const {routesFile, routesPath, routes} = options;

const path = options._[3];
const className = options._[4];

if (!path || !className) {
throw new ConfigurationError("Usage: react-server add-page <urlPath> <ClassName>");
}

const page = join("pages", className + ".js");

if (fileExists(page)) {
throw new ConfigurationError(`Found a pre-existing ${page}. Aborting.`);
}

mkdirp("pages");

console.log(chalk.yellow("Generating " + page));

fs.writeFileSync(page, PAGE_SOURCE({className}));

routes.routes[className] = { path, page };

console.log(chalk.yellow("Updating " + routesFile));

fs.writeFileSync(routesPath, JSON.stringify(routes, null, " ") + "\n");

console.log(chalk.green("All set!"));
}
23 changes: 23 additions & 0 deletions packages/react-server-cli/src/commands/compile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import compileClient from "../compileClient"
import handleCompilationErrors from "../handleCompilationErrors";
import setupLogging from "../setupLogging";
import logProductionWarnings from "../logProductionWarnings";

const logger = require("react-server").logging.getLogger(__LOGGER__);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we were trying to set the pattern of import {logging} from 'react-server'; const logger = loggin.getLogger(__LOGGER__);? Or is that a non-goal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.


export default function compile(options){
setupLogging(options);
logProductionWarnings(options);

const {compiler} = compileClient(options);

logger.notice("Starting compilation of client JavaScript...");
compiler.run((err, stats) => {
const error = handleCompilationErrors(err, stats);
if (!error) {
logger.notice("Successfully compiled client JavaScript.");
} else {
logger.error(error);
}
});
}
71 changes: 71 additions & 0 deletions packages/react-server-cli/src/commands/init.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import _ from "lodash";
import fs from "fs";
import {spawnSync} from "child_process";
Copy link
Collaborator

@doug-wade doug-wade Aug 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer spawn, personally -- we're using babel afaict, so you can use async/await w/ promises to smooth out the pyramid of doom.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no concurrency among subprocesses here. Easier flow of control this way.

import chalk from "chalk";
import fileExists from "../fileExists";
import ConfigurationError from "../ConfigurationError";

const DEPENDENCIES = [
"react-server",
"babel-preset-react-server",

// TODO: Modernize our peer deps and remove versions here.
Copy link
Collaborator

@doug-wade doug-wade Aug 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes please

"superagent@1.2.0",
"react@~0.14.2",
"react-dom@~0.14.2",
]

const DEV_DEPENDENCIES = [

// TODO: These, too.
"webpack-dev-server@~1.13.0",
"webpack@^1.13.1",
]

const CONFIG = {
"routes.json": {
middleware: [],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we init with a single page/route?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? My thought was if you want a page you can generate it with react-server add-page. If you don't then this way you won't have to go in and delete something you didn't ask for.

routes: {},
},
".reactserverrc": {
routesFile: "routes.json",
port: 3000,
env: {
production: {
port: 80,
},
},
},
".babelrc": {
presets: ["react-server"],
},
}

export default function init(){

if (!fileExists("package.json")) {
throw new ConfigurationError("Missing package.json. Please run `npm init` first.");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spawnSync('npm init --yes') seems more reasonable than a config error

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... yeah, I like it.

}

Object.keys(CONFIG).forEach(fn => {
if (fileExists(fn)) {
throw new ConfigurationError(`Found a pre-existing ${fn}. Aborting.`);
}
});

console.log(chalk.yellow("Installing dependencies"));

spawnSync("npm", ["install", "--save", ...DEPENDENCIES], {stdio: "inherit"});

console.log(chalk.yellow("Installing devDependencies"));

spawnSync("npm", ["install", "--save-dev", ...DEV_DEPENDENCIES], {stdio: "inherit"});

_.forEach(CONFIG, (config, fn) => {
console.log(chalk.yellow("Generating " + fn));

fs.writeFileSync(fn, JSON.stringify(config, null, " ") + "\n");
});

console.log(chalk.green("All set!"));
}
Loading