From f7b48abe1235f8e9e7beeaee0b193494fc12aa23 Mon Sep 17 00:00:00 2001 From: Siddharth Doshi Date: Fri, 12 May 2017 22:23:58 +0530 Subject: [PATCH 1/5] Add custom eslint formatter --- .../react-dev-utils/formatWebpackMessages.js | 49 --------------- packages/react-dev-utils/formatter.js | 63 +++++++++++++++++++ packages/react-dev-utils/package.json | 1 + .../config/webpack.config.dev.js | 2 + 4 files changed, 66 insertions(+), 49 deletions(-) create mode 100644 packages/react-dev-utils/formatter.js diff --git a/packages/react-dev-utils/formatWebpackMessages.js b/packages/react-dev-utils/formatWebpackMessages.js index ad12e0ac931..6ed6bd090d3 100644 --- a/packages/react-dev-utils/formatWebpackMessages.js +++ b/packages/react-dev-utils/formatWebpackMessages.js @@ -85,55 +85,6 @@ function formatMessage(message, isError) { ); } - // TODO: Ideally we should write a custom ESLint formatter instead. - - // If the second line already includes a filename, and it's a warning, - // this is likely coming from ESLint. Skip it because Webpack also prints it. - // Let's omit that in this case. - var BEGIN_ESLINT_FILENAME = String.fromCharCode(27) + '[4m'; - // Also filter out ESLint summaries for each file - var BEGIN_ESLINT_WARNING_SUMMARY = String.fromCharCode(27) + - '[33m' + - String.fromCharCode(27) + - '[1m' + - String.fromCharCode(10006); - var BEGIN_ESLINT_ERROR_SUMMARY = String.fromCharCode(27) + - '[31m' + - String.fromCharCode(27) + - '[1m' + - String.fromCharCode(10006); - // ESLint puts separators like this between groups. We don't need them: - var ESLINT_EMPTY_SEPARATOR = String.fromCharCode(27) + - '[22m' + - String.fromCharCode(27) + - '[39m'; - // Go! - lines = lines.filter(function(line) { - if (line === ESLINT_EMPTY_SEPARATOR) { - return false; - } - if ( - line.indexOf(BEGIN_ESLINT_FILENAME) === 0 || - line.indexOf(BEGIN_ESLINT_WARNING_SUMMARY) === 0 || - line.indexOf(BEGIN_ESLINT_ERROR_SUMMARY) === 0 - ) { - return false; - } - return true; - }); - - var ESLINT_WARNING_LABEL = String.fromCharCode(27) + - '[33m' + - 'warning' + - String.fromCharCode(27) + - '[39m'; - // If there were errors, omit any warnings. - if (isError) { - lines = lines.filter(function(line) { - return line.indexOf(ESLINT_WARNING_LABEL) === -1; - }); - } - // Prepend filename with an explanation. lines[0] = // Underline diff --git a/packages/react-dev-utils/formatter.js b/packages/react-dev-utils/formatter.js new file mode 100644 index 00000000000..dde9527d59b --- /dev/null +++ b/packages/react-dev-utils/formatter.js @@ -0,0 +1,63 @@ +'use strict'; + +const chalk = require('chalk'); +const table = require('text-table'); + +function isError(message) { + if (message.fatal || message.severity === 2) { + return true; + } + return false; +} + +function formatter(results) { + let output = '\n'; + + results.forEach(result => { + let messages = result.messages; + if (messages.length === 0) { + return ''; + } + + let thereAreErrors = false; + messages = messages.map(message => { + let messageType; + if (isError(message)) { + messageType = 'error'; + thereAreErrors = true; + } else { + messageType = 'warn'; + } + let line = message.line || 0; + let column = message.column || 0; + let position = chalk.dim(`${line}:${column}`); + return [ + '', + position, + messageType, + message.message.replace(/\.$/, ''), + chalk.dim(message.ruleId || ''), + ]; + }); + // if there are error messages, we want to show only errors + if (thereAreErrors) { + messages = messages.filter(m => m[2] === 'error'); + } + // add color to messageTypes + messages = messages.map(m => { + m[2] = m[2] === 'error' ? chalk.red(m[2]) : chalk.yellow(m[2]); + return m; + }); + let outputTable = table(messages, { + align: ['l', 'l', 'l'], + stringLength(str) { + return chalk.stripColor(str).length; + }, + }); + output += `${outputTable}\n\n`; + }); + + return output; +} + +module.exports = formatter; diff --git a/packages/react-dev-utils/package.json b/packages/react-dev-utils/package.json index 8fe7f53b37c..1b11bfffabc 100644 --- a/packages/react-dev-utils/package.json +++ b/packages/react-dev-utils/package.json @@ -11,6 +11,7 @@ "node": ">=6" }, "files": [ + "formatter.js", "ansiHTML.js", "checkRequiredFiles.js", "clearConsole.js", diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js index e7ab84a2ee1..2f817de63ac 100644 --- a/packages/react-scripts/config/webpack.config.dev.js +++ b/packages/react-scripts/config/webpack.config.dev.js @@ -19,6 +19,7 @@ const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin'); const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin'); const getClientEnvironment = require('./env'); const paths = require('./paths'); +const formatter = require('react-dev-utils/formatter'); // Webpack uses `publicPath` to determine where the app is being served from. // In development, we always serve from the root. This makes config easier. @@ -123,6 +124,7 @@ module.exports = { // @remove-on-eject-begin // Point ESLint to our predefined config. options: { + formatter, baseConfig: { extends: ['react-app'], }, From 4bb2bf02823ba4c1ae801052561cc9833bc554e5 Mon Sep 17 00:00:00 2001 From: Siddharth Doshi Date: Sat, 13 May 2017 00:16:07 +0530 Subject: [PATCH 2/5] Add formatter docs --- packages/react-dev-utils/README.md | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/react-dev-utils/README.md b/packages/react-dev-utils/README.md index 26d762c3597..ba7740c8e96 100644 --- a/packages/react-dev-utils/README.md +++ b/packages/react-dev-utils/README.md @@ -166,6 +166,43 @@ compiler.plugin('done', function(stats) { }); ``` +#### `formatter(results: Object): string` + +This is an eslint formatter that takes the result generated from eslint and formats the output string + +```js +const formatter = require('react-dev-utils/formatter'); +const webpack = require('webpack'); + +module: { + strictExportPresence: true, + rules: [ + { parser: { requireEnsure: false } }, + { + test: /\.(js|jsx)$/, + enforce: 'pre', + use: [ + { + // @remove-on-eject-begin + // Point ESLint to our predefined config. + options: { + formatter, + baseConfig: { + extends: ['react-app'], + }, + ignore: false, + useEslintrc: false, + }, + // @remove-on-eject-end + loader: 'eslint-loader', + }, + ], + include: paths.appSrc, + } + ] +} +``` + #### `getProcessForPort(port: number): string` Finds the currently running process on `port`. From d24ef7d4365c48725fb5fbfddd88373d02fc4482 Mon Sep 17 00:00:00 2001 From: Siddharth Doshi Date: Sat, 13 May 2017 00:23:48 +0530 Subject: [PATCH 3/5] Update formatter docs --- packages/react-dev-utils/README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/react-dev-utils/README.md b/packages/react-dev-utils/README.md index ba7740c8e96..976d8ae05b7 100644 --- a/packages/react-dev-utils/README.md +++ b/packages/react-dev-utils/README.md @@ -168,12 +168,13 @@ compiler.plugin('done', function(stats) { #### `formatter(results: Object): string` -This is an eslint formatter that takes the result generated from eslint and formats the output string +This is our custom ESLint formatter that integrates well with Create React App console output. You can remove it and use the default one instead ```js const formatter = require('react-dev-utils/formatter'); const webpack = require('webpack'); +// Eslint loader in your webpack config module: { strictExportPresence: true, rules: [ @@ -183,7 +184,6 @@ module: { enforce: 'pre', use: [ { - // @remove-on-eject-begin // Point ESLint to our predefined config. options: { formatter, @@ -193,7 +193,6 @@ module: { ignore: false, useEslintrc: false, }, - // @remove-on-eject-end loader: 'eslint-loader', }, ], From 4bf55ded3f40a54c263f11a1358aa2b2afcb36d4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 15 May 2017 00:22:29 +0100 Subject: [PATCH 4/5] Slightly tweak it --- .../{formatter.js => eslintFormatter.js} | 20 +++++++++++++------ .../react-dev-utils/formatWebpackMessages.js | 11 ++-------- packages/react-dev-utils/package.json | 2 +- .../config/webpack.config.dev.js | 9 ++++----- .../config/webpack.config.prod.js | 8 +++++--- .../scripts/utils/createWebpackCompiler.js | 14 ++++++------- 6 files changed, 33 insertions(+), 31 deletions(-) rename packages/react-dev-utils/{formatter.js => eslintFormatter.js} (86%) diff --git a/packages/react-dev-utils/formatter.js b/packages/react-dev-utils/eslintFormatter.js similarity index 86% rename from packages/react-dev-utils/formatter.js rename to packages/react-dev-utils/eslintFormatter.js index dde9527d59b..dd971077639 100644 --- a/packages/react-dev-utils/formatter.js +++ b/packages/react-dev-utils/eslintFormatter.js @@ -13,21 +13,26 @@ function isError(message) { function formatter(results) { let output = '\n'; + let hasErrors = false; + let hasWarnings = false; + results.forEach(result => { let messages = result.messages; if (messages.length === 0) { - return ''; + return; } - let thereAreErrors = false; + let hasErrors = false; messages = messages.map(message => { let messageType; if (isError(message)) { messageType = 'error'; - thereAreErrors = true; + hasErrors = true; } else { messageType = 'warn'; + hasWarnings = true; } + let line = message.line || 0; let column = message.column || 0; let position = chalk.dim(`${line}:${column}`); @@ -39,21 +44,24 @@ function formatter(results) { chalk.dim(message.ruleId || ''), ]; }); + // if there are error messages, we want to show only errors - if (thereAreErrors) { + if (hasErrors) { messages = messages.filter(m => m[2] === 'error'); } + // add color to messageTypes - messages = messages.map(m => { + messages.forEach(m => { m[2] = m[2] === 'error' ? chalk.red(m[2]) : chalk.yellow(m[2]); - return m; }); + let outputTable = table(messages, { align: ['l', 'l', 'l'], stringLength(str) { return chalk.stripColor(str).length; }, }); + output += `${outputTable}\n\n`; }); diff --git a/packages/react-dev-utils/formatWebpackMessages.js b/packages/react-dev-utils/formatWebpackMessages.js index 6ed6bd090d3..f1262aad56b 100644 --- a/packages/react-dev-utils/formatWebpackMessages.js +++ b/packages/react-dev-utils/formatWebpackMessages.js @@ -16,6 +16,7 @@ // This is quite hacky and hopefully won't be needed when Webpack fixes this. // https://github.com/webpack/webpack/issues/2878 +var chalk = require('chalk'); var friendlySyntaxErrorLabel = 'Syntax error:'; function isLikelyASyntaxError(message) { @@ -86,15 +87,7 @@ function formatMessage(message, isError) { } // Prepend filename with an explanation. - lines[0] = - // Underline - String.fromCharCode(27) + - '[4m' + - // Filename - lines[0] + - // End underline - String.fromCharCode(27) + - '[24m' + + lines[0] = chalk.underline(lines[0]) + (isError ? ' contains errors.' : ' contains warnings.'); // Reassemble the message. diff --git a/packages/react-dev-utils/package.json b/packages/react-dev-utils/package.json index 1b11bfffabc..bd1bcd9b405 100644 --- a/packages/react-dev-utils/package.json +++ b/packages/react-dev-utils/package.json @@ -11,11 +11,11 @@ "node": ">=6" }, "files": [ - "formatter.js", "ansiHTML.js", "checkRequiredFiles.js", "clearConsole.js", "crashOverlay.js", + "eslintFormatter.js", "FileSizeReporter.js", "formatWebpackMessages.js", "getProcessForPort.js", diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js index 2f817de63ac..50ca67b1a47 100644 --- a/packages/react-scripts/config/webpack.config.dev.js +++ b/packages/react-scripts/config/webpack.config.dev.js @@ -17,9 +17,9 @@ const HtmlWebpackPlugin = require('html-webpack-plugin'); const CaseSensitivePathsPlugin = require('case-sensitive-paths-webpack-plugin'); const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin'); const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin'); +const eslintFormatter = require('react-dev-utils/eslintFormatter'); const getClientEnvironment = require('./env'); const paths = require('./paths'); -const formatter = require('react-dev-utils/formatter'); // Webpack uses `publicPath` to determine where the app is being served from. // In development, we always serve from the root. This makes config easier. @@ -121,17 +121,16 @@ module.exports = { enforce: 'pre', use: [ { - // @remove-on-eject-begin - // Point ESLint to our predefined config. options: { - formatter, + formatter: eslintFormatter, + // @remove-on-eject-begin baseConfig: { extends: ['react-app'], }, ignore: false, useEslintrc: false, + // @remove-on-eject-end }, - // @remove-on-eject-end loader: 'eslint-loader', }, ], diff --git a/packages/react-scripts/config/webpack.config.prod.js b/packages/react-scripts/config/webpack.config.prod.js index f51617005b9..fc8869de863 100644 --- a/packages/react-scripts/config/webpack.config.prod.js +++ b/packages/react-scripts/config/webpack.config.prod.js @@ -17,6 +17,7 @@ const HtmlWebpackPlugin = require('html-webpack-plugin'); const ExtractTextPlugin = require('extract-text-webpack-plugin'); const ManifestPlugin = require('webpack-manifest-plugin'); const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin'); +const eslintFormatter = require('react-dev-utils/eslintFormatter'); const paths = require('./paths'); const getClientEnvironment = require('./env'); @@ -117,17 +118,18 @@ module.exports = { enforce: 'pre', use: [ { - // @remove-on-eject-begin - // Point ESLint to our predefined config. options: { + formatter: eslintFormatter, + // @remove-on-eject-begin // TODO: consider separate config for production, // e.g. to enable no-console and no-debugger only in production. baseConfig: { extends: ['react-app'], }, + ignore: false, useEslintrc: false, + // @remove-on-eject-end }, - // @remove-on-eject-end loader: 'eslint-loader', }, ], diff --git a/packages/react-scripts/scripts/utils/createWebpackCompiler.js b/packages/react-scripts/scripts/utils/createWebpackCompiler.js index a333165b082..260b36c9756 100644 --- a/packages/react-scripts/scripts/utils/createWebpackCompiler.js +++ b/packages/react-scripts/scripts/utils/createWebpackCompiler.js @@ -100,17 +100,17 @@ module.exports = function createWebpackCompiler(config, onReadyCallback) { console.log(message); console.log(); }); + // Teach some ESLint tricks. - console.log('You may use special comments to disable some warnings.'); console.log( - 'Use ' + - chalk.yellow('// eslint-disable-next-line') + - ' to ignore the next line.' + 'Search the ' + + chalk.dim('keywords') + + ' from the right column to learn more.' ); console.log( - 'Use ' + - chalk.yellow('/* eslint-disable */') + - ' to ignore all warnings in a file.' + 'To ignore, add ' + + chalk.yellow('// eslint-disable-next-line') + + ' to the line before.' ); } }); From 39a0f26fd814795917ee74f623ced63614b373b2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 15 May 2017 00:25:38 +0100 Subject: [PATCH 5/5] Update README.md --- packages/react-dev-utils/README.md | 66 ++++++++++++++---------------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/packages/react-dev-utils/README.md b/packages/react-dev-utils/README.md index 976d8ae05b7..e44068c7fbd 100644 --- a/packages/react-dev-utils/README.md +++ b/packages/react-dev-utils/README.md @@ -110,6 +110,36 @@ clearConsole(); console.log('Just cleared the screen!'); ``` +#### `eslintFormatter(results: Object): string` + +This is our custom ESLint formatter that integrates well with Create React App console output. +You can use the default one instead if you prefer so. + +```js +const eslintFormatter = require('react-dev-utils/eslintFormatter'); + +// In your webpack config: +// ... +module: { + rules: [ + { + test: /\.(js|jsx)$/, + include: paths.appSrc, + enforce: 'pre', + use: [ + { + loader: 'eslint-loader', + options: { + // Pass the formatter: + formatter: eslintFormatter, + }, + }, + ], + } + ] +} +``` + #### `FileSizeReporter` ##### `measureFileSizesBeforeBuild(buildFolder: string): Promise` @@ -166,42 +196,6 @@ compiler.plugin('done', function(stats) { }); ``` -#### `formatter(results: Object): string` - -This is our custom ESLint formatter that integrates well with Create React App console output. You can remove it and use the default one instead - -```js -const formatter = require('react-dev-utils/formatter'); -const webpack = require('webpack'); - -// Eslint loader in your webpack config -module: { - strictExportPresence: true, - rules: [ - { parser: { requireEnsure: false } }, - { - test: /\.(js|jsx)$/, - enforce: 'pre', - use: [ - { - // Point ESLint to our predefined config. - options: { - formatter, - baseConfig: { - extends: ['react-app'], - }, - ignore: false, - useEslintrc: false, - }, - loader: 'eslint-loader', - }, - ], - include: paths.appSrc, - } - ] -} -``` - #### `getProcessForPort(port: number): string` Finds the currently running process on `port`.