Skip to content

Commit

Permalink
Web dev improvements (Graylog2#4388)
Browse files Browse the repository at this point in the history
* Customize port number in dev-server

Let users customize port number by passing `--port=<port>` as argument.
If the port is already taken, we will start the web interface in a
random one.

* Remove alternative dev setup

* Revert dev server port

Changed it accidentally during some tests.

* Remove react-hot-loader dev build

The `start` build using HMR alongside react-hot-loader was already not
working as it should with webpack-dev-server. Modules got reloaded, but the
application state was reset on every change.

I have been trying a couple of times to make HMR work with the new express
dev server, but I just managed to reproduce what it achieved in the
previous build (reloading the whole component tree, but losing the state).
All of that making the current webpack configuration more complicated.

I think react-hot-loader is a good idea that can be helpful specially when
styling components, but currently I see that it produces way too maintenance
burden for the benefits it provides. Also, nobody in the team seems to be
using it for development.

Long story short, this commit removes react-hot-loader from our dev build. We
still use Webpack HMR to reload the browser.

The commit also replaces the `start` script with `start-nohmr`, so
running `yarn start` will use the no react-hot-loader dev server.

* Update README file

- Include details to use yarn in development
- Remove old warnings and information
- Remove mentions of the react-hot-loader build
- Update information on upgrade packages
- Update information on IntelliJ setup

* Improve some texts in the README

* Add documentation on how to customize port number

* Fix linter errors
  • Loading branch information
edmundoa authored and bernd committed Dec 5, 2017
1 parent 84843a8 commit be7a102
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 215 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// eslint-disable-next-line no-unused-vars
import webpackEntry from 'webpack-entry';
import 'webpack-entry';

import { PluginManifest, PluginStore } from 'graylog-web-plugin/plugin';

Expand All @@ -25,8 +24,3 @@ const manifest = new PluginManifest(packageJson, {
});

PluginStore.register(manifest);

if (module.hot) {
module.hot.accept();
module.hot.dispose(() => PluginStore.unregister(manifest));
}
2 changes: 1 addition & 1 deletion graylog2-web-interface/.babelrc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"presets": ["es2015", "react", "stage-0"],
"plugins": ["add-module-exports", "react-hot-loader/babel"]
"plugins": ["add-module-exports"]
}
64 changes: 23 additions & 41 deletions graylog2-web-interface/README.md
Original file line number Diff line number Diff line change
@@ -1,55 +1,37 @@
# Graylog Web Interface

## Development Setup

* Install [node.js](http://nodejs.org/) and npm.
* Run `npm install`
* Run `npm start` (if you don't want to include plugins in the build while developing, simply run `disable_plugins=true npm start`)
* Open http://localhost:8080

The `npm start` (or `disable_plugins=true npm start`) command will run the `webpack-dev-server`, which allows in-browser hot reloading.
In order to make switching between different branches faster, we use a script to store all `node_modules` folders
into `.node_cache` and then symlink the folder for the current branch to `node_modules`.
## Requirements
- [Node.js](https://nodejs.org/), at this time we use v8.9.1
- [Yarn](https://yarnpkg.com/)

When using IntelliJ or WebStorm, be sure to enable `JSX harmony` (available in IntelliJ 14 and WebStorm 9)
as JavaScript language version to properly support react templates.
**Note:** NPM v5 changed completely the way it builds local modules, breaking the Graylog web interface build. Please use Yarn instead of NPM v5.

You might get an error message during `npm install` from `gyp` because the installed (default) Python version is too recent (sic!):

```
gyp ERR! stack Error: Python executable "python" is v3.4.2, which is not supported by gyp.
```

In this case just set the correct (installed!) Python binary before running `npm install`:
## Development Setup

```
npm config set python python2.7
```
* Install the requirements listed above
* Run `yarn install`
* Run `yarn start` to build the project for development and start the development server. You can exclude any Graylog frontend plugins from the build by running `disable_plugins=true npm start` instead
* Open http://localhost:8080 in your browser to access the Graylog web interface

### Alternative Development Setup
The `yarn start` (or `disable_plugins=true yarn start`) command will run an [Express](http://expressjs.com) web server which is configured to do a full page reload in your browser every time that you save a file. This ensures that you will always use the latest version of your code.

Due to problems with webpack-dev-server there is another way to run the development setup.
You can start the development server in any other port that you like. To do so, use the `--port=<port>` option, e.g. `yarn start --port=8000` will start the development server in port 8000 instead of the default 8080. The server will also pick a random port if the port is already taken, so you don't need to worry if another process is already using that port.

* Install [devd](https://github.com/cortesi/devd)
* Install [node.js](http://nodejs.org/) and npm.
* Run `npm install`
* Run `npm run watch` and **keep it running** to start webpack in watch mode so it rebuilds on source changes
* Run `npm run devd` and **keep it running** once the `build/` directory exists
* Open http://localhost:8080
We mainly develop using IntelliJ or WebStorm. If you also decide to use them to work in Graylog, enable `React JSX` as Javascript language version to support the JSX language extension. This setting was called `JSX harmony` before, and it is available in one or the other form since IntelliJ 14 and WebStorm 9.

## Update Javascript dependencies

#### Update Javascript dependencies
1. Update a single dependency

a. Update a single dependency
* Run `yarn upgrade <package>@<version>`
* Commit any changes in both `package.json` and `yarn.lock` files
* Do any changes required to adapt the code to the upgraded modules

* Update `package.json` file with the new dependency
* `npm update <npm-package>`
* `npm shrinkwrap --dev` to save the whole dependency tree into the `npm-shrinkwrap.json` file
1. Update many dependencies

b. Update devDependencies
* It may be dangerous updating many dependencies at the same time, so be sure you checked the upgrade notes of all modules before getting started. Once you are ready to upgrade the modules, Yarn provides a few options to do it:
* You can pass all packages you want to upgrade to Yarn: `yarn upgrade <package1> <package2>...`
* Yarn also supports upgrading packages matching a pattern, so you can execute `yarn upgrade --pattern <pattern>`
* You could execute `yarn upgrade` if you really want to upgrade all packages
* After doing the upgrade, remember to commit both the `package.json` and `yarn.lock` files

* `npm shinkwrap` to keep the dependency tree (without devDependencies) into `npm-shrinkwrap.json`
* Update `package.json` file with the new devDependencies
* `npm install`
* Do more work with the new devDependencies
* `npm shrinkwrap --dev` to export the whole dependency tree with the new devDependencies into `npm-shrinkwrap.json`
23 changes: 20 additions & 3 deletions graylog2-web-interface/devServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@ const express = require('express');
const webpack = require('webpack');
const compress = require('compression');
const history = require('connect-history-api-fallback');
const http = require('http');
const yargs = require('yargs');
const webpackDevMiddleware = require('webpack-dev-middleware');
const webpackHotMiddleware = require('webpack-hot-middleware');
const webpackConfig = require('./webpack.bundled');

const DEFAULT_PORT = 8080;

const app = express();
const vendorConfig = webpackConfig[0];
const appConfig = webpackConfig[1];
Expand All @@ -32,6 +36,19 @@ app.use(webpackDevMiddleware(appCompiler, {

app.use(webpackHotMiddleware(appCompiler));

app.listen(8080, () => {
console.log('Graylog web interface listening on port 8080!\n');
});
const server = http.createServer(app);

const argv = yargs.argv;

server
.listen(argv.port || DEFAULT_PORT, () => {
console.log(`Graylog web interface listening on port ${server.address().port}!\n`);
})
.on('error', (error) => {
if (error.code === 'EADDRINUSE') {
console.error(`Port ${argv.port || DEFAULT_PORT} already in use, will use a random one instead...`);
server.listen(0);
} else {
throw error;
}
});
9 changes: 3 additions & 6 deletions graylog2-web-interface/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
"scripts": {
"docs:build": "styleguidist build",
"docs:server": "styleguidist server",
"start": "webpack-dev-server --config webpack.bundled.js --history-api-fallback --hot --inline",
"start-nohmr": "node devServer.js",
"watch": "webpack --watch --config webpack.bundled.js",
"devd": "devd --livewatch --port=8080 --address=127.0.0.1 --notfound=index.html build/",
"start": "node devServer.js",
"build": "disable_plugins=true webpack --config webpack.bundled.js",
"lint": "eslint --ext js,jsx src",
"test": "jest"
Expand Down Expand Up @@ -120,7 +117,6 @@
"less": "^2.5.3",
"less-loader": "^4.0.5",
"phantomjs-prebuilt": ">=1.9",
"react-hot-loader": "^3.0.0-beta.6",
"react-proxy-loader": "^0.3.4",
"react-styleguidist": "^6.0.33",
"react-test-renderer": "^15.6.1",
Expand All @@ -134,7 +130,8 @@
"webpack-dev-middleware": "^1.12.0",
"webpack-dev-server": "^2.6.1",
"webpack-hot-middleware": "^2.20.0",
"webpack-merge": "^4.1.0"
"webpack-merge": "^4.1.0",
"yargs": "^10.0.3"
},
"resolutions": {
"@types/jquery": "2.0.48",
Expand Down
18 changes: 3 additions & 15 deletions graylog2-web-interface/src/index.jsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
/* global REPLACE_MODULES */

// We need to set the app prefix before doing anything else, so it applies to styles too.
// eslint-disable-next-line no-unused-vars
import webpackEntry from 'webpack-entry';
import 'webpack-entry';

import React from 'react';
import ReactDOM from 'react-dom';
import Promise from 'bluebird';
import Reflux from 'reflux';
import { AppContainer } from 'react-hot-loader';
import AppFacade from 'routing/AppFacade';

Promise.config({ cancellation: true });
Reflux.setPromiseFactory(handlers => new Promise(handlers));

function renderAppContainer(appContainer) {
// eslint-disable-next-line global-require
const AppFacade = require('routing/AppFacade');
ReactDOM.render(
REPLACE_MODULES ? <AppContainer><AppFacade /></AppContainer> : <AppFacade />,
<AppFacade />,
appContainer,
);
}
Expand All @@ -27,11 +22,4 @@ window.onload = () => {
document.body.appendChild(appContainer);

renderAppContainer(appContainer);

if (module.hot && REPLACE_MODULES) {
console.log('HMR enabled');
module.hot.accept('routing/AppFacade', () => {
renderAppContainer(appContainer);
});
}
};
6 changes: 3 additions & 3 deletions graylog2-web-interface/webpack.combined.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@ const webpack = require('webpack');
const glob = require('glob');
const path = require('path');
const fs = require('fs');
const merge = require('webpack-merge');

const ROOT_PATH = path.resolve(__dirname);
const MANIFESTS_PATH = path.resolve(ROOT_PATH, 'manifests');
const VENDOR_MANIFEST_PATH = path.resolve(MANIFESTS_PATH, 'vendor-manifest.json');
const TARGET = process.env.npm_lifecycle_event;

const pluginPrefix = '../../graylog-plugin-*/**/';
const pluginConfigPattern = pluginPrefix + 'webpack.config.js';
const pluginConfigPattern = `${pluginPrefix}webpack.config.js`;

const pluginConfigs = process.env.disable_plugins === 'true' ? [] : glob.sync(pluginConfigPattern);

process.env.web_src_path = path.resolve(__dirname);

// eslint-disable-next-line import/no-dynamic-require
const webpackConfig = require(path.resolve(__dirname, './webpack.config.js'));

function getPluginName(pluginConfig) {
Expand Down Expand Up @@ -44,7 +44,7 @@ pluginConfigs.filter(isNotDependency).forEach((pluginConfig) => {
});

// We need to inject webpack-hot-middleware to all entries, ensuring the app is able to reload on changes.
if (TARGET === 'start-nohmr') {
if (TARGET === 'start') {
const hmrEntries = {};
const webpackHotMiddlewareEntry = 'webpack-hot-middleware/client?reload=true';

Expand Down
82 changes: 15 additions & 67 deletions graylog2-web-interface/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ process.env.BABEL_ENV = TARGET;
const BABELRC = path.resolve(ROOT_PATH, '.babelrc');
const BABELOPTIONS = {
cacheDirectory: 'cache',
'extends': BABELRC,
extends: BABELRC,
};

const BABELLOADER = { loader: 'babel-loader', options: BABELOPTIONS };

// eslint-disable-next-line import/no-dynamic-require
const BOOTSTRAPVARS = require(path.resolve(ROOT_PATH, 'public', 'stylesheets', 'bootstrap-config.json')).vars;

const webpackConfig = {
Expand All @@ -41,16 +42,19 @@ const webpackConfig = {
{ test: /\.ts$/, use: [BABELLOADER, { loader: 'ts-loader' }], exclude: /node_modules|\.node_cache/ },
{ test: /\.(woff(2)?|svg|eot|ttf|gif|jpg)(\?.+)?$/, use: 'file-loader' },
{ test: /\.png$/, use: 'url-loader' },
{ test: /bootstrap\.less$/, use: [
'style-loader',
'css-loader',
{
loader: 'less-loader',
options: {
modifyVars: BOOTSTRAPVARS,
{
test: /bootstrap\.less$/,
use: [
'style-loader',
'css-loader',
{
loader: 'less-loader',
options: {
modifyVars: BOOTSTRAPVARS,
},
},
},
] },
],
},
{ test: /\.less$/, use: ['style-loader', 'css-loader', 'less-loader'], exclude: /bootstrap\.less$/ },
{ test: /\.css$/, use: ['style-loader', 'css-loader'] },
],
Expand Down Expand Up @@ -93,39 +97,6 @@ const webpackConfig = {
};

if (TARGET === 'start') {
console.error('Running in development mode');
module.exports = merge(webpackConfig, {
entry: {
reacthot: 'react-hot-loader/patch',
},
devtool: 'eval',
devServer: {
historyApiFallback: true,
hot: true,
inline: true,
lazy: false,
watchOptions: {
ignored: /node_modules/,
},
},
output: {
path: BUILD_PATH,
filename: '[name].js',
publicPath: '/',
hotUpdateChunkFilename: '[id].hot-update.js',
hotUpdateMainFilename: 'hot-update.json',
},
plugins: [
new webpack.NamedModulesPlugin(),
new webpack.DefinePlugin({
DEVELOPMENT: true,
REPLACE_MODULES: true,
}),
],
});
}

if (TARGET === 'start-nohmr') {
console.error('Running in development (no HMR) mode');
module.exports = merge(webpackConfig, {
devtool: 'eval',
Expand All @@ -137,43 +108,20 @@ if (TARGET === 'start-nohmr') {
plugins: [
new webpack.DefinePlugin({
DEVELOPMENT: true,
REPLACE_MODULES: false, // We don't intend to use HMR but for reloading the browser window
}),
new CopyWebpackPlugin([{ from: 'config.js' }]),
new webpack.HotModuleReplacementPlugin(),
],
});
}

if (TARGET === 'watch') {
console.error('Running in development (watch) mode');
module.exports = merge(webpackConfig, {
devtool: 'eval',
output: {
path: BUILD_PATH,
filename: '[name].js',
publicPath: '/',
},
plugins: [
new webpack.DefinePlugin({
DEVELOPMENT: true,
REPLACE_MODULES: false, // We don't intend to use HMR but for reloading the browser window
}),
// We need config.js in the "build/" folder. No idea how webpack-dev-server
// handles that, I found nothing in the config. (bernd)
new CopyWebpackPlugin([{ from: 'config.js' }]),
],
});
}

if (TARGET === 'build') {
console.error('Running in production mode');
process.env.NODE_ENV = 'production';
module.exports = merge(webpackConfig, {
plugins: [
new webpack.DefinePlugin({
'process.env.NODE_ENV': JSON.stringify('production'),
REPLACE_MODULES: false, // Do not use HMR in production
}),
new webpack.optimize.UglifyJsPlugin({
minimize: true,
Expand All @@ -197,7 +145,7 @@ if (TARGET === 'test') {
module.exports = merge(webpackConfig, {
module: {
rules: [
{ test: /\.js(x)?$/, enforce: 'pre', loader: 'eslint-loader', exclude: /node_modules|public\/javascripts/ }
{ test: /\.js(x)?$/, enforce: 'pre', loader: 'eslint-loader', exclude: /node_modules|public\/javascripts/ },
],
},
});
Expand Down
Loading

0 comments on commit be7a102

Please sign in to comment.