diff --git a/.eslintrc b/.eslintrc index c2df7a28093..59cb6f27b7f 100644 --- a/.eslintrc +++ b/.eslintrc @@ -4,10 +4,7 @@ }, "extends": "airbnb", "globals": { - "CLIENT": true, "CLIENT_CONFIG": true, - "DEVELOPMENT": true, - "SERVER": true, "assert": true, "sinon": true, "webpackIsomorphicTools": true diff --git a/config/default.js b/config/default.js index 547e0e669ff..a13437c5fa8 100644 --- a/config/default.js +++ b/config/default.js @@ -24,6 +24,9 @@ module.exports = { cookieMaxAge: 2592000, cookieName: 'jwt_api_auth_token', + isDeployed: true, + isDevelopment: false, + // The canonical list of enabled apps. validAppNames, @@ -46,6 +49,8 @@ module.exports = { 'apiBase', 'cookieName', 'cookieMaxAge', + 'isDeployed', + 'isDevelopment', 'startLoginUrl', ], diff --git a/config/development.js b/config/development.js index cf5d013e209..03985babdb0 100644 --- a/config/development.js +++ b/config/development.js @@ -8,6 +8,9 @@ module.exports = { apiHost: 'https://addons-dev.allizom.org', amoCDN: 'https://addons-dev-cdn.allizom.org', + isDeployed: false, + isDevelopment: true, + webpackServerHost: '127.0.0.1', webpackServerPort: 3001, webpackHost: defer((cfg) => `http://${cfg.webpackServerHost}:${cfg.webpackServerPort}`), diff --git a/karma.conf.js b/karma.conf.js index 26261deec74..d7a28b1b365 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -20,9 +20,6 @@ const coverageReporters = [{ const newWebpackConfig = Object.assign({}, webpackConfigProd, { plugins: [ new webpack.DefinePlugin({ - DEVELOPMENT: false, - CLIENT: true, - SERVER: false, CLIENT_CONFIG: JSON.stringify(clientConfig), }), new webpack.NormalModuleReplacementPlugin(/config$/, 'client-config.js'), diff --git a/package.json b/package.json index f8960853dcf..3c60960660e 100644 --- a/package.json +++ b/package.json @@ -14,8 +14,9 @@ "eslint": "npm run lint", "lint": "eslint .", "servertest": "npm run build && better-npm-run servertest", - "start:disco": "better-npm-run start:disco", - "start:search": "better-npm-run start:search", + "start": "npm run version-check && NODE_PATH='./:./src' node bin/server.js", + "start:disco": "NODE_APP_INSTANCE=disco npm start", + "start:search": "NODE_APP_INSTANCE=search npm start", "test": "better-npm-run test", "unittest": "better-npm-run unittest", "unittest:dev": "better-npm-run unittest:dev", @@ -68,24 +69,6 @@ "NODE_ENV": "production" } }, - "start": { - "command": "npm run version-check && node bin/server.js", - "env": { - "NODE_PATH": "./:./src" - } - }, - "start:search": { - "command": "better-npm-run start", - "env": { - "NODE_APP_INSTANCE": "search" - } - }, - "start:disco": { - "command": "better-npm-run start", - "env": { - "NODE_APP_INSTANCE": "disco" - } - }, "test": { "command": "npm run version-check && npm run unittest && npm run servertest && npm run lint", "env": { @@ -127,9 +110,9 @@ "homepage": "https://github.com/mozillla/addons-frontend#readme", "dependencies": { "better-npm-run": "0.0.8", + "bunyan": "1.8.1", "camelcase": "2.1.1", "classnames": "2.2.5", - "common-tags": "0.0.3", "config": "1.20.1", "express": "4.13.4", "extract-text-webpack-plugin": "1.0.1", diff --git a/src/core/server/base.js b/src/core/server/base.js index 7b23d3b3548..5181c6bc12b 100644 --- a/src/core/server/base.js +++ b/src/core/server/base.js @@ -6,6 +6,7 @@ import cookie from 'react-cookie'; import React from 'react'; import ReactDOM from 'react-dom/server'; import ReactHelmet from 'react-helmet'; +import bunyan from 'bunyan'; import { Provider } from 'react-redux'; import { match } from 'react-router'; @@ -19,20 +20,32 @@ import config from 'config'; import { setJWT } from 'core/actions'; const env = config.util.getEnv('NODE_ENV'); -const isDeployed = ['stage', 'dev', 'production'].indexOf(env) > -1; +const isDeployed = config.get('isDeployed'); +const isDevelopment = config.get('isDevelopment'); const errorString = 'Internal Server Error'; const appName = config.get('appName'); -// Globals (these are set by definePlugin for client-side builds). -global.CLIENT = false; -global.SERVER = true; -global.DEVELOPMENT = env === 'development'; +const log = bunyan.createLogger({ + name: 'server', + app: appName, + serializers: bunyan.stdSerializers, +}); + +function logRequests(req, res, next) { + const start = new Date(); + next(); + const finish = new Date(); + const elapsed = finish - start; + log.info({req, res, start, finish, elapsed}); +} function baseServer(routes, createStore, { appInstanceName = appName } = {}) { const app = new Express(); app.disable('x-powered-by'); + app.use(logRequests); + // Sets X-Frame-Options app.use(helmet.frameguard('deny')); @@ -45,8 +58,8 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { // CSP configuration. app.use(helmet.contentSecurityPolicy(config.get('CSP'))); - if (env === 'development') { - console.log('Running in Development Mode'); // eslint-disable-line no-console + if (isDevelopment) { + log.info('Running in Development Mode'); // clear require() cache if in development mode webpackIsomorphicTools.refresh(); @@ -67,7 +80,7 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { cookie.plugToRequest(req, res); if (err) { - console.error(err); // eslint-disable-line no-console + log.error({err, req}); return res.status(500).end(errorString); } @@ -109,8 +122,7 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { res.send(`${HTML}`); }) .catch((error) => { - // eslint-disable-next-line no-console - console.error(error.stack); + log.error({err: error}); res.status(500).end(errorString); }); }); @@ -118,8 +130,7 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { // eslint-disable-next-line no-unused-vars app.use((err, req, res, next) => { - // eslint-disable-next-line no-console - console.error(err.stack); + log.error({err}); res.status(500).end(errorString); }); @@ -128,8 +139,7 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { export function runServer({listen = true, app = appName} = {}) { if (!app) { - // eslint-disable-next-line no-console - console.log(`Please specify a valid appName from ${config.get('validAppNames')}`); + log.fatal(`Please specify a valid appName from ${config.get('validAppNames')}`); process.exit(1); } @@ -137,7 +147,7 @@ export function runServer({listen = true, app = appName} = {}) { const host = config.get('serverHost'); const isoMorphicServer = new WebpackIsomorphicTools(WebpackIsomorphicToolsConfig); - return isoMorphicServer.development(env === 'development') + return isoMorphicServer.development(isDevelopment) .server(config.get('basePath')) .then(() => { global.webpackIsomorphicTools = isoMorphicServer; @@ -154,10 +164,9 @@ export function runServer({listen = true, app = appName} = {}) { if (err) { reject(err); } - // eslint-disable-next-line no-console - console.log(`🔥 Addons-frontend server is running [ENV:${env}] [APP:${app}]`); - // eslint-disable-next-line no-console - console.log(`👁 Open your browser at http://${host}:${port} to view it.`); + log.info(`🔥 Addons-frontend server is running [ENV:${env}] [APP:${app}] ` + + `[isDevelopment:${isDevelopment}] [isDeployed:${isDeployed}]`); + log.info(`👁 Open your browser at http://${host}:${port} to view it.`); resolve(server); }); } else { @@ -166,8 +175,7 @@ export function runServer({listen = true, app = appName} = {}) { }); }) .catch((err) => { - // eslint-disable-next-line no-console - console.error(err); + log.error({err}); }); } diff --git a/src/core/store.js b/src/core/store.js new file mode 100644 index 00000000000..fceff693921 --- /dev/null +++ b/src/core/store.js @@ -0,0 +1,10 @@ +import { applyMiddleware } from 'redux'; +import createLogger from 'redux-logger'; +import config from 'config'; + +export function middleware({ __config = config } = {}) { + if (__config.get('isDevelopment')) { + return applyMiddleware(createLogger()); + } + return undefined; +} diff --git a/src/disco/store.js b/src/disco/store.js index 1d7fba679ff..1a930a685c1 100644 --- a/src/disco/store.js +++ b/src/disco/store.js @@ -1,11 +1,11 @@ -import { applyMiddleware, createStore as _createStore, combineReducers } from 'redux'; +import { createStore as _createStore, combineReducers } from 'redux'; import { reducer as reduxAsyncConnect } from 'redux-async-connect'; -import createLogger from 'redux-logger'; +import { middleware } from 'core/store'; export default function createStore(initialState = {}) { return _createStore( combineReducers({reduxAsyncConnect}), initialState, - applyMiddleware(createLogger()), + middleware(), ); } diff --git a/src/search/store.js b/src/search/store.js index b27dffc006c..5c706dbae89 100644 --- a/src/search/store.js +++ b/src/search/store.js @@ -1,16 +1,16 @@ -import { applyMiddleware, createStore as _createStore, combineReducers } from 'redux'; +import { createStore as _createStore, combineReducers } from 'redux'; import { reducer as reduxAsyncConnect } from 'redux-async-connect'; -import createLogger from 'redux-logger'; import addons from 'core/reducers/addons'; import api from 'core/reducers/api'; import auth from 'core/reducers/authentication'; import search from 'search/reducers/search'; +import { middleware } from 'core/store'; export default function createStore(initialState = {}) { return _createStore( combineReducers({addons, api, auth, search, reduxAsyncConnect}), initialState, - applyMiddleware(createLogger()), + middleware(), ); } diff --git a/tests/client/core/test_store.js b/tests/client/core/test_store.js new file mode 100644 index 00000000000..e14e69aa8d0 --- /dev/null +++ b/tests/client/core/test_store.js @@ -0,0 +1,21 @@ +import { middleware } from 'core/store'; + +describe('core store middleware', () => { + it('includes the middleware in development', () => { + const config = { + get() { + return true; + }, + }; + assert.isFunction(middleware({__config: config})); + }); + + it('is undefined when not in development', () => { + const config = { + get() { + return false; + }, + }; + assert.strictEqual(undefined, middleware({__config: config})); + }); +}); diff --git a/webpack.dev.config.babel.js b/webpack.dev.config.babel.js index 65027cfc8aa..a3219b9bb13 100644 --- a/webpack.dev.config.babel.js +++ b/webpack.dev.config.babel.js @@ -74,10 +74,7 @@ export default Object.assign({}, webpackConfig, { }, plugins: [ new webpack.DefinePlugin({ - CLIENT: true, CLIENT_CONFIG: JSON.stringify(clientConfig), - DEVELOPMENT: true, - SERVER: false, }), new webpack.NormalModuleReplacementPlugin(/config$/, 'client-config.js'), new webpack.HotModuleReplacementPlugin(), diff --git a/webpack.prod.config.babel.js b/webpack.prod.config.babel.js index 13249547d64..71c6c885d3b 100644 --- a/webpack.prod.config.babel.js +++ b/webpack.prod.config.babel.js @@ -49,9 +49,6 @@ export default { }, plugins: [ new webpack.DefinePlugin({ - DEVELOPMENT: false, - CLIENT: true, - SERVER: false, CLIENT_CONFIG: JSON.stringify(clientConfig), }), // Replaces server config module with the subset clientConfig object.