From 7b1a32be6ec9f99a6c9a3c66813f3ac09c4736b9 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 30 Sep 2018 17:44:49 -0400 Subject: [PATCH] Polish webpack message output (#5174) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Only install react-scripts in CI mode * Link locally * Re-enable all output tests * 💄 Polish webpack output * Test sass support message * Add more tests, but disabled * Format missing default export error * Format aliased import * Why was node-sass required? Odd * Format webpack rejection error * Re-enable unknown package test * Format file not found error and catch module scope plugin error * Re-disable case sensitive paths * Intercept and format case sensitive path errors * Test out of scope message formatting * Run behavior on macOS * Run behavior on Node 8 and 10, only Node 8 for macOS * Add some debugging * Update matcher * Only check stderr * Remove old snapshot * More debug * Remove debug * Add new debug * Disable test on linux * Add comment for future --- .travis.yml | 3 + .../__snapshots__/index.test.js.snap | 103 ++++++++-- .../webpack-message-formatting/index.test.js | 85 +++++++- .../webpack-message-formatting/package.json | 4 +- .../src/AppAliasUnknownExport.js | 13 ++ .../src/AppIncorrectCase.js | 10 + .../src/AppNoDefault.js | 10 + .../src/AppOutOfScopeImport.js | 10 + .../webpack-message-formatting/src/AppSass.js | 10 + .../src/AppSass.scss | 3 + .../src/AppUnknownFile.js | 10 + .../webpack-message-formatting/src/Export5.js | 1 + .../src/ExportNoDefault.js | 1 + fixtures/smoke/boostrap-sass/package.json | 3 +- .../package.json | 3 +- fixtures/smoke/graphql-with-mjs/package.json | 3 +- fixtures/smoke/relative-paths/package.json | 4 +- fixtures/utils.js | 58 ++++++ .../react-dev-utils/ModuleNotFoundPlugin.js | 146 ++++++++++++++ packages/react-dev-utils/ModuleScopePlugin.js | 37 ++-- packages/react-dev-utils/eslintFormatter.js | 3 + .../react-dev-utils/formatWebpackMessages.js | 186 ++++++------------ packages/react-dev-utils/package.json | 2 + .../config/webpack.config.dev.js | 4 + .../config/webpack.config.prod.js | 4 + packages/react-scripts/scripts/build.js | 16 +- tasks/e2e-behavior.sh | 4 +- 27 files changed, 558 insertions(+), 178 deletions(-) create mode 100644 fixtures/output/webpack-message-formatting/src/AppAliasUnknownExport.js create mode 100644 fixtures/output/webpack-message-formatting/src/AppIncorrectCase.js create mode 100644 fixtures/output/webpack-message-formatting/src/AppNoDefault.js create mode 100644 fixtures/output/webpack-message-formatting/src/AppOutOfScopeImport.js create mode 100644 fixtures/output/webpack-message-formatting/src/AppSass.js create mode 100644 fixtures/output/webpack-message-formatting/src/AppSass.scss create mode 100644 fixtures/output/webpack-message-formatting/src/AppUnknownFile.js create mode 100644 fixtures/output/webpack-message-formatting/src/Export5.js create mode 100644 fixtures/output/webpack-message-formatting/src/ExportNoDefault.js create mode 100644 packages/react-dev-utils/ModuleNotFoundPlugin.js diff --git a/.travis.yml b/.travis.yml index fd83ef7393e..e104f22e1a9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,5 +28,8 @@ env: - TEST_SUITE=behavior matrix: include: + - os: osx + node_js: 8 + env: TEST_SUITE=behavior - node_js: 4 env: TEST_SUITE=old-node diff --git a/fixtures/output/webpack-message-formatting/__snapshots__/index.test.js.snap b/fixtures/output/webpack-message-formatting/__snapshots__/index.test.js.snap index 923ae734dee..6a510ef70b9 100644 --- a/fixtures/output/webpack-message-formatting/__snapshots__/index.test.js.snap +++ b/fixtures/output/webpack-message-formatting/__snapshots__/index.test.js.snap @@ -1,12 +1,26 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`webpack message formatting formats aliased unknown export 1`] = ` +Object { + "stderr": "Creating an optimized production build... +Failed to compile. + +./src/App.js +Attempted import error: 'bar' is not exported from './AppUnknownExport' (imported as 'bar2'). + + +", + "stdout": "", +} +`; + exports[`webpack message formatting formats babel syntax error 1`] = ` Object { "stderr": "Creating an optimized production build... Failed to compile. ./src/App.js -Syntax error: Unterminated JSX contents (8:12) +Syntax error: Unterminated JSX contents (8:13) 6 |
7 | @@ -25,16 +39,16 @@ Syntax error: Unterminated JSX contents (8:12) exports[`webpack message formatting formats css syntax error 1`] = ` Object { "stderr": "Creating an optimized production build... -Failed to compile. - -./src/AppCss.css -Syntax Error: (3:2) Unexpected } +Failed to compile. -  1 | .App { -  2 |  color: red; -> 3 | }} -  |  ^ -  4 |  +./src/AppCss.css +Syntax error: Unexpected } (3:2) + + 1 | .App { + 2 | color: red; +> 3 | }} + | ^ + 4 | ", @@ -74,12 +88,58 @@ To ignore, add // eslint-disable-next-line to the line before. } `; +exports[`webpack message formatting formats file not found error 1`] = ` +Object { + "stderr": "Creating an optimized production build... +Failed to compile. + +./src/App.js +Cannot find file './ThisFileSouldNotExist' in './src'. + + +", + "stdout": "", +} +`; + exports[`webpack message formatting formats missing package 1`] = ` Object { "stderr": "Creating an optimized production build... -Failed to compile. - -Module not found: Error: Can't resolve 'unknown-package' in '/private/var/folders/c3/vytj6_h56b77f_g72smntm3m0000gn/T/bf26e1d3700ad14275f6eefb5e4417c1/src' +Failed to compile. + +./src/App.js +Cannot find module: 'unknown-package'. Make sure this package is installed. + +You can install this package by running: yarn add unknown-package. + + +", + "stdout": "", +} +`; + +exports[`webpack message formatting formats no default export 1`] = ` +Object { + "stderr": "Creating an optimized production build... +Failed to compile. + +./src/App.js +Attempted import error: './ExportNoDefault' does not contain a default export (imported as 'myImport'). + + +", + "stdout": "", +} +`; + +exports[`webpack message formatting formats out of scope error 1`] = ` +Object { + "stderr": "Creating an optimized production build... +Failed to compile. + +./src/App.js +You attempted to import ../OutOfScopeImport which falls outside of the project src/ directory. Relative imports outside of src/ are not supported. +You can either move it inside src/, or add a symlink to it from project's node_modules/. ", @@ -93,7 +153,22 @@ Object { Failed to compile. ./src/App.js -1:1677-1680 './AppUnknownExport' does not contain an export named 'bar'. +Attempted import error: 'bar' is not exported from './AppUnknownExport'. + + +", + "stdout": "", +} +`; + +exports[`webpack message formatting helps when users tries to use sass 1`] = ` +Object { + "stderr": "Creating an optimized production build... +Failed to compile. + +./src/AppSass.scss +To import Sass files, you first need to install node-sass. +Run \`npm install node-sass\` or \`yarn add node-sass\` inside your workspace. ", diff --git a/fixtures/output/webpack-message-formatting/index.test.js b/fixtures/output/webpack-message-formatting/index.test.js index 985caaffde6..475fb108536 100644 --- a/fixtures/output/webpack-message-formatting/index.test.js +++ b/fixtures/output/webpack-message-formatting/index.test.js @@ -1,4 +1,8 @@ -const { bootstrap, getOutputProduction } = require('../../utils'); +const { + bootstrap, + getOutputDevelopment, + getOutputProduction, +} = require('../../utils'); const fs = require('fs-extra'); const path = require('path'); const Semaphore = require('async-sema'); @@ -19,7 +23,7 @@ describe('webpack message formatting', () => { semaphore.release(); }); - xit('formats babel syntax error', async () => { + it('formats babel syntax error', async () => { fs.copySync( path.join(__dirname, 'src', 'AppBabel.js'), path.join(testDirectory, 'src', 'App.js') @@ -29,8 +33,7 @@ describe('webpack message formatting', () => { expect(response).toMatchSnapshot(); }); - xit('formats css syntax error', async () => { - // TODO: fix me! + it('formats css syntax error', async () => { fs.copySync( path.join(__dirname, 'src', 'AppCss.js'), path.join(testDirectory, 'src', 'App.js') @@ -40,8 +43,7 @@ describe('webpack message formatting', () => { expect(response).toMatchSnapshot(); }); - xit('formats unknown export', async () => { - // TODO: fix me! + it('formats unknown export', async () => { fs.copySync( path.join(__dirname, 'src', 'AppUnknownExport.js'), path.join(testDirectory, 'src', 'App.js') @@ -51,8 +53,27 @@ describe('webpack message formatting', () => { expect(response).toMatchSnapshot(); }); - xit('formats missing package', async () => { - // TODO: fix me! + it('formats aliased unknown export', async () => { + fs.copySync( + path.join(__dirname, 'src', 'AppAliasUnknownExport.js'), + path.join(testDirectory, 'src', 'App.js') + ); + + const response = await getOutputProduction({ directory: testDirectory }); + expect(response).toMatchSnapshot(); + }); + + it('formats no default export', async () => { + fs.copySync( + path.join(__dirname, 'src', 'AppNoDefault.js'), + path.join(testDirectory, 'src', 'App.js') + ); + + const response = await getOutputProduction({ directory: testDirectory }); + expect(response).toMatchSnapshot(); + }); + + it('formats missing package', async () => { fs.copySync( path.join(__dirname, 'src', 'AppMissingPackage.js'), path.join(testDirectory, 'src', 'App.js') @@ -85,4 +106,52 @@ describe('webpack message formatting', () => { const response = await getOutputProduction({ directory: testDirectory }); expect(response).toMatchSnapshot(); }); + + it('helps when users tries to use sass', async () => { + fs.copySync( + path.join(__dirname, 'src', 'AppSass.js'), + path.join(testDirectory, 'src', 'App.js') + ); + + const response = await getOutputProduction({ directory: testDirectory }); + expect(response).toMatchSnapshot(); + }); + + it('formats file not found error', async () => { + fs.copySync( + path.join(__dirname, 'src', 'AppUnknownFile.js'), + path.join(testDirectory, 'src', 'App.js') + ); + + const response = await getOutputProduction({ directory: testDirectory }); + expect(response).toMatchSnapshot(); + }); + + it('formats case sensitive path error', async () => { + fs.copySync( + path.join(__dirname, 'src', 'AppIncorrectCase.js'), + path.join(testDirectory, 'src', 'App.js') + ); + + const response = await getOutputDevelopment({ directory: testDirectory }); + if (process.platform === 'darwin') { + expect(response.stderr).toMatch( + `Cannot find file: 'export5.js' does not match the corresponding name on disk: './src/Export5.js'.` + ); + } else { + expect(response.stderr).not.toEqual(''); // TODO: figure out how we can test this on Linux/Windows + // I believe getting this working requires we tap into enhanced-resolve + // pipeline, which is debt we don't want to take on right now. + } + }); + + it('formats out of scope error', async () => { + fs.copySync( + path.join(__dirname, 'src', 'AppOutOfScopeImport.js'), + path.join(testDirectory, 'src', 'App.js') + ); + + const response = await getOutputProduction({ directory: testDirectory }); + expect(response).toMatchSnapshot(); + }); }); diff --git a/fixtures/output/webpack-message-formatting/package.json b/fixtures/output/webpack-message-formatting/package.json index 5f6f0ee96a2..4fda3598df5 100644 --- a/fixtures/output/webpack-message-formatting/package.json +++ b/fixtures/output/webpack-message-formatting/package.json @@ -1,9 +1,7 @@ { "dependencies": { - "node-sass": "4.x", "react": "latest", - "react-dom": "latest", - "react-scripts": "latest" + "react-dom": "latest" }, "browserslist": [ ">0.2%" diff --git a/fixtures/output/webpack-message-formatting/src/AppAliasUnknownExport.js b/fixtures/output/webpack-message-formatting/src/AppAliasUnknownExport.js new file mode 100644 index 00000000000..df2716efc78 --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/AppAliasUnknownExport.js @@ -0,0 +1,13 @@ +import React, { Component } from 'react'; +import { bar as bar2 } from './AppUnknownExport'; + +class App extends Component { + componentDidMount() { + bar2(); + } + render() { + return
; + } +} + +export default App; diff --git a/fixtures/output/webpack-message-formatting/src/AppIncorrectCase.js b/fixtures/output/webpack-message-formatting/src/AppIncorrectCase.js new file mode 100644 index 00000000000..40f69d43938 --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/AppIncorrectCase.js @@ -0,0 +1,10 @@ +import React, { Component } from 'react'; +import five from './export5'; + +class App extends Component { + render() { + return
{five}
; + } +} + +export default App; diff --git a/fixtures/output/webpack-message-formatting/src/AppNoDefault.js b/fixtures/output/webpack-message-formatting/src/AppNoDefault.js new file mode 100644 index 00000000000..9087eed49d0 --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/AppNoDefault.js @@ -0,0 +1,10 @@ +import React, { Component } from 'react'; +import myImport from './ExportNoDefault'; + +class App extends Component { + render() { + return
{myImport}
; + } +} + +export default App; diff --git a/fixtures/output/webpack-message-formatting/src/AppOutOfScopeImport.js b/fixtures/output/webpack-message-formatting/src/AppOutOfScopeImport.js new file mode 100644 index 00000000000..a8717e23082 --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/AppOutOfScopeImport.js @@ -0,0 +1,10 @@ +import React, { Component } from 'react'; +import myImport from '../OutOfScopeImport'; + +class App extends Component { + render() { + return
{myImport}
; + } +} + +export default App; diff --git a/fixtures/output/webpack-message-formatting/src/AppSass.js b/fixtures/output/webpack-message-formatting/src/AppSass.js new file mode 100644 index 00000000000..20ac0e211a5 --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/AppSass.js @@ -0,0 +1,10 @@ +import React, { Component } from 'react'; +import './AppSass.scss'; + +class App extends Component { + render() { + return
; + } +} + +export default App; diff --git a/fixtures/output/webpack-message-formatting/src/AppSass.scss b/fixtures/output/webpack-message-formatting/src/AppSass.scss new file mode 100644 index 00000000000..724638c3a7b --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/AppSass.scss @@ -0,0 +1,3 @@ +.App { + color: red; +} diff --git a/fixtures/output/webpack-message-formatting/src/AppUnknownFile.js b/fixtures/output/webpack-message-formatting/src/AppUnknownFile.js new file mode 100644 index 00000000000..d3b2ce26730 --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/AppUnknownFile.js @@ -0,0 +1,10 @@ +import React, { Component } from 'react'; +import DefaultExport from './ThisFileSouldNotExist'; + +class App extends Component { + render() { + return
; + } +} + +export default App; diff --git a/fixtures/output/webpack-message-formatting/src/Export5.js b/fixtures/output/webpack-message-formatting/src/Export5.js new file mode 100644 index 00000000000..ba35c21b821 --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/Export5.js @@ -0,0 +1 @@ +export default 5; diff --git a/fixtures/output/webpack-message-formatting/src/ExportNoDefault.js b/fixtures/output/webpack-message-formatting/src/ExportNoDefault.js new file mode 100644 index 00000000000..1608f67ee7a --- /dev/null +++ b/fixtures/output/webpack-message-formatting/src/ExportNoDefault.js @@ -0,0 +1 @@ +export const six = 6; diff --git a/fixtures/smoke/boostrap-sass/package.json b/fixtures/smoke/boostrap-sass/package.json index ba85a20cecf..adb19270b6b 100644 --- a/fixtures/smoke/boostrap-sass/package.json +++ b/fixtures/smoke/boostrap-sass/package.json @@ -3,7 +3,6 @@ "bootstrap": "4.x", "node-sass": "4.x", "react": "latest", - "react-dom": "latest", - "react-scripts": "latest" + "react-dom": "latest" } } diff --git a/fixtures/smoke/builds-with-multiple-runtimes/package.json b/fixtures/smoke/builds-with-multiple-runtimes/package.json index 7829ebdeabd..b2792516c6e 100644 --- a/fixtures/smoke/builds-with-multiple-runtimes/package.json +++ b/fixtures/smoke/builds-with-multiple-runtimes/package.json @@ -3,7 +3,6 @@ "dva": "2.4.0", "ky": "0.3.0", "react": "latest", - "react-dom": "latest", - "react-scripts": "latest" + "react-dom": "latest" } } diff --git a/fixtures/smoke/graphql-with-mjs/package.json b/fixtures/smoke/graphql-with-mjs/package.json index f7658f9b7a3..c97e6b8d013 100644 --- a/fixtures/smoke/graphql-with-mjs/package.json +++ b/fixtures/smoke/graphql-with-mjs/package.json @@ -4,7 +4,6 @@ "graphql": "14.0.2", "react-apollo": "2.2.1", "react": "latest", - "react-dom": "latest", - "react-scripts": "latest" + "react-dom": "latest" } } diff --git a/fixtures/smoke/relative-paths/package.json b/fixtures/smoke/relative-paths/package.json index 3ed0e3e4eea..64d2bb6dd27 100644 --- a/fixtures/smoke/relative-paths/package.json +++ b/fixtures/smoke/relative-paths/package.json @@ -1,6 +1,4 @@ { - "dependencies": { - "react-scripts": "latest" - }, + "dependencies": {}, "homepage": "." } diff --git a/fixtures/utils.js b/fixtures/utils.js index e40ffba0938..2bcab03c4a2 100644 --- a/fixtures/utils.js +++ b/fixtures/utils.js @@ -6,12 +6,36 @@ const os = require('os'); const stripAnsi = require('strip-ansi'); async function bootstrap({ directory, template }) { + const shouldInstallScripts = process.env.CI && process.env.CI !== 'false'; await Promise.all( ['public/', 'src/', 'package.json'].map(async file => fs.copy(path.join(template, file), path.join(directory, file)) ) ); + if (shouldInstallScripts) { + const packageJson = fs.readJsonSync(path.join(directory, 'package.json')); + packageJson.dependencies = Object.assign(packageJson.dependencies, { + 'react-scripts': 'latest', + }); + fs.writeJsonSync(path.join(directory, 'package.json'), packageJson); + } await execa('yarnpkg', ['install', '--mutex', 'network'], { cwd: directory }); + if (!shouldInstallScripts) { + fs.ensureSymlinkSync( + path.resolve( + path.join( + __dirname, + '..', + 'packages', + 'react-scripts', + 'bin', + 'react-scripts.js' + ) + ), + path.join(directory, 'node_modules', '.bin', 'react-scripts') + ); + await execa('yarnpkg', ['link', 'react-scripts'], { cwd: directory }); + } } async function isSuccessfulDevelopment({ directory }) { @@ -43,6 +67,39 @@ async function isSuccessfulProduction({ directory }) { } } +async function getOutputDevelopment({ directory, env = {} }) { + try { + const { stdout, stderr } = await execa( + './node_modules/.bin/react-scripts', + ['start', '--smoke-test'], + { + cwd: directory, + env: Object.assign( + {}, + { + BROWSER: 'none', + PORT: await getPort(), + CI: 'false', + FORCE_COLOR: '0', + }, + env + ), + } + ); + return { stdout: stripAnsi(stdout), stderr: stripAnsi(stderr) }; + } catch (err) { + return { + stdout: '', + stderr: stripAnsi( + err.message + .split(os.EOL) + .slice(2) + .join(os.EOL) + ), + }; + } +} + async function getOutputProduction({ directory, env = {} }) { try { const { stdout, stderr } = await execa( @@ -71,5 +128,6 @@ module.exports = { bootstrap, isSuccessfulDevelopment, isSuccessfulProduction, + getOutputDevelopment, getOutputProduction, }; diff --git a/packages/react-dev-utils/ModuleNotFoundPlugin.js b/packages/react-dev-utils/ModuleNotFoundPlugin.js new file mode 100644 index 00000000000..55705fd3ea9 --- /dev/null +++ b/packages/react-dev-utils/ModuleNotFoundPlugin.js @@ -0,0 +1,146 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use strict'; + +const chalk = require('chalk'); +const findUp = require('find-up'); +const path = require('path'); + +class ModuleNotFoundPlugin { + constructor(appPath, yarnLockFile) { + this.appPath = appPath; + this.yarnLockFile = yarnLockFile; + + this.useYarnCommand = this.useYarnCommand.bind(this); + this.getRelativePath = this.getRelativePath.bind(this); + this.prettierError = this.prettierError.bind(this); + } + + useYarnCommand() { + try { + return findUp.sync('yarn.lock', { cwd: this.appPath }) != null; + } catch (_) { + return false; + } + } + + getRelativePath(_file) { + let file = path.relative(this.appPath, _file); + if (file.startsWith('..')) { + file = _file; + } else if (!file.startsWith('.')) { + file = '.' + path.sep + file; + } + return file; + } + + prettierError(err) { + let { details: _details = '', origin } = err; + + if (origin == null) { + const caseSensitivity = + err.message && + /\[CaseSensitivePathsPlugin\] `(.*?)` .* `(.*?)`/.exec(err.message); + if (caseSensitivity) { + const [, incorrectPath, actualName] = caseSensitivity; + const actualFile = this.getRelativePath( + path.join(path.dirname(incorrectPath), actualName) + ); + const incorrectName = path.basename(incorrectPath); + err.message = `Cannot find file: '${incorrectName}' does not match the corresponding name on disk: '${actualFile}'.`; + } + return err; + } + + const file = this.getRelativePath(origin.resource); + let details = _details.split('\n'); + + const request = /resolve '(.*?)' in '(.*?)'/.exec(details); + if (request) { + const isModule = details[1] && details[1].includes('module'); + const isFile = details[1] && details[1].includes('file'); + + let [, target, context] = request; + context = this.getRelativePath(context); + if (isModule) { + const isYarn = this.useYarnCommand(); + details = [ + `Cannot find module: '${target}'. Make sure this package is installed.`, + '', + 'You can install this package by running: ' + + (isYarn + ? chalk.bold(`yarn add ${target}`) + : chalk.bold(`npm install ${target}`)) + + '.', + ]; + } else if (isFile) { + details = [`Cannot find file '${target}' in '${context}'.`]; + } else { + details = [err.message]; + } + } else { + details = [err.message]; + } + err.message = [file, ...details].join('\n').replace('Error: ', ''); + + const isModuleScopePluginError = + err.error && err.error.__module_scope_plugin; + if (isModuleScopePluginError) { + err.message = err.message.replace('Module not found: ', ''); + } + return err; + } + + apply(compiler) { + const { prettierError } = this; + compiler.hooks.make.intercept({ + register(tap) { + if ( + !(tap.name === 'MultiEntryPlugin' || tap.name === 'SingleEntryPlugin') + ) { + return tap; + } + return Object.assign({}, tap, { + fn: (compilation, callback) => { + tap.fn(compilation, (err, ...args) => { + if (err && err.name === 'ModuleNotFoundError') { + err = prettierError(err); + } + callback(err, ...args); + }); + }, + }); + }, + }); + compiler.hooks.normalModuleFactory.tap('ModuleNotFoundPlugin', nmf => { + nmf.hooks.afterResolve.intercept({ + register(tap) { + if (tap.name !== 'CaseSensitivePathsPlugin') { + return tap; + } + return Object.assign({}, tap, { + fn: (compilation, callback) => { + tap.fn(compilation, (err, ...args) => { + if ( + err && + err.message && + err.message.includes('CaseSensitivePathsPlugin') + ) { + err = prettierError(err); + } + callback(err, ...args); + }); + }, + }); + }, + }); + }); + } +} + +module.exports = ModuleNotFoundPlugin; diff --git a/packages/react-dev-utils/ModuleScopePlugin.js b/packages/react-dev-utils/ModuleScopePlugin.js index 9ce24fd11a0..e84d2b38aab 100644 --- a/packages/react-dev-utils/ModuleScopePlugin.js +++ b/packages/react-dev-utils/ModuleScopePlugin.js @@ -9,6 +9,7 @@ const chalk = require('chalk'); const path = require('path'); +const os = require('os'); class ModuleScopePlugin { constructor(appSrc, allowedFiles = []) { @@ -63,24 +64,28 @@ class ModuleScopePlugin { ); }) ) { - callback( - new Error( - `You attempted to import ${chalk.cyan( - request.__innerRequest_request - )} which falls outside of the project ${chalk.cyan( + const scopeError = new Error( + `You attempted to import ${chalk.cyan( + request.__innerRequest_request + )} which falls outside of the project ${chalk.cyan( + 'src/' + )} directory. ` + + `Relative imports outside of ${chalk.cyan( 'src/' - )} directory. ` + - `Relative imports outside of ${chalk.cyan( - 'src/' - )} are not supported. ` + - `You can either move it inside ${chalk.cyan( - 'src/' - )}, or add a symlink to it from project's ${chalk.cyan( - 'node_modules/' - )}.` - ), - request + )} are not supported.` + + os.EOL + + `You can either move it inside ${chalk.cyan( + 'src/' + )}, or add a symlink to it from project's ${chalk.cyan( + 'node_modules/' + )}.` ); + Object.defineProperty(scopeError, '__module_scope_plugin', { + value: true, + writable: false, + enumerable: false, + }); + callback(scopeError, request); } else { callback(); } diff --git a/packages/react-dev-utils/eslintFormatter.js b/packages/react-dev-utils/eslintFormatter.js index f18c7307081..cdef9c70b43 100644 --- a/packages/react-dev-utils/eslintFormatter.js +++ b/packages/react-dev-utils/eslintFormatter.js @@ -42,6 +42,9 @@ function formatter(results) { } let line = message.line || 0; + if (message.column) { + line += ':' + message.column; + } let position = chalk.bold('Line ' + line + ':'); return [ '', diff --git a/packages/react-dev-utils/formatWebpackMessages.js b/packages/react-dev-utils/formatWebpackMessages.js index 2c4edc07eb0..4b0f44acb74 100644 --- a/packages/react-dev-utils/formatWebpackMessages.js +++ b/packages/react-dev-utils/formatWebpackMessages.js @@ -7,15 +7,8 @@ 'use strict'; -// WARNING: this code is untranspiled and is used in browser too. -// Please make sure any changes are in ES5 or contribute a Babel compile step. - -// Some custom utilities to prettify Webpack output. -// 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:'; +const chalk = require('chalk'); +const friendlySyntaxErrorLabel = 'Syntax error:'; function isLikelyASyntaxError(message) { return message.indexOf(friendlySyntaxErrorLabel) !== -1; @@ -24,159 +17,108 @@ function isLikelyASyntaxError(message) { // Cleans up webpack error messages. // eslint-disable-next-line no-unused-vars function formatMessage(message, isError) { - var lines = message.split('\n'); + let lines = message.split('\n'); - // Strip `WorkerError` header off message before parsing - // https://github.com/webpack-contrib/thread-loader/blob/6fb5daff313c4839196cf533bdcdf14815a386d2/src/WorkerError.js - lines = lines.filter(function(message) { - return message.indexOf('Thread Loader (Worker') === -1; - }); - - // Add empty line for errors from third-party webpack plugins - if (lines.length < 2) { - lines[1] = ''; - } + // Strip Webpack-added headers off errors/warnings + // https://github.com/webpack/webpack/blob/master/lib/ModuleError.js + lines = lines.filter(line => !/Module [A-z ]+\(from/.test(line)); - // Strip `ModuleWarning` head off message before parsing (because of ESLint) - // https://github.com/webpack/webpack/blob/c77030573de96b8293c69dd396492f8e2d46561e/lib/ModuleWarning.js - var moduleWarningPrefix = 'Module Warning: '; - if (lines[1].indexOf(moduleWarningPrefix) === 0) { - lines[1] = lines[1].slice(moduleWarningPrefix.length); - } else if (lines[1].match(/Module Warning \(from.*?\):/)) { - lines.splice(1, 1); - } - - // Strip `ModuleError` header off message before parsing - // https://github.com/webpack/webpack/blob/c77030573de96b8293c69dd396492f8e2d46561e/lib/ModuleError.js - var moduleErrorPrefix = 'Module Error: '; - if (lines[1].indexOf(moduleErrorPrefix) === 0) { - lines[1] = lines[1].slice(moduleErrorPrefix.length); - } else if (lines[1].match(/Module Error \(from.*?\):/)) { - lines.splice(1, 1); - } + // Transform parsing error into syntax error + // TODO: move this to our ESLint formatter? + lines = lines.map(line => { + const parsingError = /Line (\d+):(?:(\d+):)?\s*Parsing error: (.+)$/.exec( + line + ); + if (!parsingError) { + return line; + } + const [, errorLine, errorColumn, errorMessage] = parsingError; + return `${friendlySyntaxErrorLabel} ${errorMessage} (${errorLine}:${errorColumn})`; + }); - // Simplify `ModuleBuildError` before parsing (these may be nested so we use a while loop) - // https://github.com/webpack/webpack/blob/c77030573de96b8293c69dd396492f8e2d46561e/lib/ModuleBuildError.js - while ( - lines.length > 2 && - lines[1].match(/Module build failed \(from.*?\):/) - ) { - lines.splice(1, 1); - lines[1] = 'Module build failed: ' + lines[1]; - } + message = lines.join('\n'); + // Smoosh syntax errors (commonly found in CSS) + message = message.replace( + /SyntaxError\s+\((\d+):(\d+)\)\s*(.+?)\n/g, + `${friendlySyntaxErrorLabel} $3 ($1:$2)\n` + ); + // Remove columns from ESLint formatter output (we added these for more + // accurate syntax errors) + message = message.replace(/Line (\d+):\d+:/g, 'Line $1:'); + // Clean up export errors + message = message.replace( + /^.*export '(.+?)' was not found in '(.+?)'.*$/gm, + `Attempted import error: '$1' is not exported from '$2'.` + ); + message = message.replace( + /^.*export 'default' \(imported as '(.+?)'\) was not found in '(.+?)'.*$/gm, + `Attempted import error: '$2' does not contain a default export (imported as '$1').` + ); + message = message.replace( + /^.*export '(.+?)' \(imported as '(.+?)'\) was not found in '(.+?)'.*$/gm, + `Attempted import error: '$1' is not exported from '$3' (imported as '$2').` + ); + lines = message.split('\n'); - if (lines.length > 2 && lines[1] === '') { - // Remove extra newline. + // Remove leading newline + if (lines.length > 2 && lines[1].trim() === '') { lines.splice(1, 1); } - - // Remove webpack-specific loader notation from filename. - // Before: - // ./~/css-loader!./~/postcss-loader!./src/App.css - // After: - // ./src/App.css - if (lines[0].lastIndexOf('!') !== -1) { - lines[0] = lines[0].substr(lines[0].lastIndexOf('!') + 1); - } - - lines = lines.filter(function(line) { - // Webpack adds a list of entry points to warning messages: - // @ ./src/index.js - // @ multi react-scripts/~/react-dev-utils/webpackHotDevClient.js ... - // It is misleading (and unrelated to the warnings) so we clean it up. - // It is only useful for syntax errors but we have beautiful frames for them. - return line.indexOf(' @ ') !== 0; - }); - - // line #0 is filename - // line #1 is the main error message - if (!lines[0] || !lines[1]) { - return lines.join('\n'); - } + // Clean up file name + lines[0] = lines[0].replace(/^(.*) \d+:\d+-\d+$/, '$1'); // Cleans up verbose "module not found" messages for files and packages. - if (lines[1].indexOf('Module not found: ') === 0) { + if (lines[1] && lines[1].indexOf('Module not found: ') === 0) { lines = [ lines[0], - // Clean up message because "Module not found: " is descriptive enough. lines[1] - .replace("Cannot resolve 'file' or 'directory' ", '') - .replace('Cannot resolve module ', '') .replace('Error: ', '') - .replace('[CaseSensitivePathsPlugin] ', ''), + .replace('Module not found: Cannot find file:', 'Cannot find file:'), ]; } - if (lines[1].match(/Cannot find module.+node-sass/)) { - lines[1] = - 'To import Sass files in this project, you need to install node-sass.\n'; + // Add helpful message for users trying to use Sass for the first time + if (lines[1] && lines[1].match(/Cannot find module.+node-sass/)) { + lines[1] = 'To import Sass files, you first need to install node-sass.\n'; lines[1] += - 'Please run `npm i node-sass --save` or `yarn add node-sass` inside your workspace.'; - } - - // Cleans up syntax error messages. - if (lines[1].indexOf('Module build failed: ') === 0) { - lines[1] = lines[1].replace( - 'Module build failed: Syntax Error ', - friendlySyntaxErrorLabel - ); - lines[1] = lines[1].replace( - /Module build failed: .*?: /, - friendlySyntaxErrorLabel + ' ' - ); - lines[1] = lines[1].trim(); - - if (lines[1] === friendlySyntaxErrorLabel && lines[2] === '') { - lines.splice(2, 1); - if (lines.length > 2) { - lines[1] += ' ' + lines[2]; - lines.splice(2, 1); - } - } - } - - // Clean up export errors. - // TODO: we should really send a PR to Webpack for this. - var exportError = /\s*(.*?)\s*(?:")?export '(.+?)' was not found in '(.+?)'/; - if (lines[1].match(exportError)) { - lines[1] = lines[1].replace( - exportError, - "$1 '$3' does not contain an export named '$2'." - ); + 'Run `npm install node-sass` or `yarn add node-sass` inside your workspace.'; } lines[0] = chalk.inverse(lines[0]); - // Reassemble the message. message = lines.join('\n'); // Internal stacks are generally useless so we strip them... with the // exception of stacks containing `webpack:` because they're normally - // from user code generated by WebPack. For more information see + // from user code generated by Webpack. For more information see // https://github.com/facebook/create-react-app/pull/1050 message = message.replace( /^\s*at\s((?!webpack:).)*:\d+:\d+[\s)]*(\n|$)/gm, '' ); // at ... ...:x:y message = message.replace(/^\s*at\s(\n|$)/gm, ''); // at + lines = message.split('\n'); + // Remove duplicated newlines + lines = lines.filter( + (line, index, arr) => + index === 0 || line.trim() !== '' || line.trim() !== arr[index - 1].trim() + ); + + // Reassemble the message + message = lines.join('\n'); return message.trim(); } function formatWebpackMessages(json) { - var formattedErrors = json.errors.map(function(message) { + const formattedErrors = json.errors.map(function(message) { return formatMessage(message, true); }); - var formattedWarnings = json.warnings.map(function(message) { + const formattedWarnings = json.warnings.map(function(message) { return formatMessage(message, false); }); - var result = { - errors: formattedErrors, - warnings: formattedWarnings, - }; + const result = { errors: formattedErrors, warnings: formattedWarnings }; if (result.errors.some(isLikelyASyntaxError)) { // If there are any syntax errors, show just them. - // This prevents a confusing ESLint parsing error - // preceding a much more useful Babel syntax error. result.errors = result.errors.filter(isLikelyASyntaxError); } return result; diff --git a/packages/react-dev-utils/package.json b/packages/react-dev-utils/package.json index c51955bc533..87508e89faa 100644 --- a/packages/react-dev-utils/package.json +++ b/packages/react-dev-utils/package.json @@ -29,6 +29,7 @@ "InterpolateHtmlPlugin.js", "launchEditor.js", "launchEditorEndpoint.js", + "ModuleNotFoundPlugin.js", "ModuleScopePlugin.js", "noopServiceWorkerMiddleware.js", "openBrowser.js", @@ -48,6 +49,7 @@ "detect-port-alt": "1.1.6", "escape-string-regexp": "1.0.5", "filesize": "3.6.1", + "find-up": "3.0.0", "global-modules": "1.0.0", "gzip-size": "5.0.0", "inquirer": "6.2.0", diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js index 517a19fe08f..819ca0e9562 100644 --- a/packages/react-scripts/config/webpack.config.dev.js +++ b/packages/react-scripts/config/webpack.config.dev.js @@ -20,6 +20,7 @@ const getClientEnvironment = require('./env'); const paths = require('./paths'); const ManifestPlugin = require('webpack-manifest-plugin'); const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier'); +const ModuleNotFoundPlugin = require('react-dev-utils/ModuleNotFoundPlugin'); // Webpack uses `publicPath` to determine where the app is being served from. // In development, we always serve from the root. This makes config easier. @@ -372,6 +373,9 @@ module.exports = { // // In development, this will be an empty string. new InterpolateHtmlPlugin(HtmlWebpackPlugin, env.raw), + // This gives some necessary context to module not found errors, such as + // the requesting resource. + new ModuleNotFoundPlugin(paths.appPath), // Makes some environment variables available to the JS code, for example: // if (process.env.NODE_ENV === 'development') { ... }. See `./env.js`. new webpack.DefinePlugin(env.stringified), diff --git a/packages/react-scripts/config/webpack.config.prod.js b/packages/react-scripts/config/webpack.config.prod.js index 8207bf4fe90..e04382a561e 100644 --- a/packages/react-scripts/config/webpack.config.prod.js +++ b/packages/react-scripts/config/webpack.config.prod.js @@ -24,6 +24,7 @@ const getCSSModuleLocalIdent = require('react-dev-utils/getCSSModuleLocalIdent') const paths = require('./paths'); const getClientEnvironment = require('./env'); const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier'); +const ModuleNotFoundPlugin = require('react-dev-utils/ModuleNotFoundPlugin'); // Webpack uses `publicPath` to determine where the app is being served from. // It requires a trailing slash, or the file assets will get an incorrect path. @@ -459,6 +460,9 @@ module.exports = { // In production, it will be an empty string unless you specify "homepage" // in `package.json`, in which case it will be the pathname of that URL. new InterpolateHtmlPlugin(HtmlWebpackPlugin, env.raw), + // This gives some necessary context to module not found errors, such as + // the requesting resource. + new ModuleNotFoundPlugin(paths.appPath), // Makes some environment variables available to the JS code, for example: // if (process.env.NODE_ENV === 'production') { ... }. See `./env.js`. // It is absolutely essential that NODE_ENV was set to production here. diff --git a/packages/react-scripts/scripts/build.js b/packages/react-scripts/scripts/build.js index 4ebf9f609a1..a9f6b87e2d0 100644 --- a/packages/react-scripts/scripts/build.js +++ b/packages/react-scripts/scripts/build.js @@ -141,12 +141,20 @@ function build(previousFileSizes) { let compiler = webpack(config); return new Promise((resolve, reject) => { compiler.run((err, stats) => { + let messages; if (err) { - return reject(err); + if (!err.message) { + return reject(err); + } + messages = formatWebpackMessages({ + errors: [err.message], + warnings: [], + }); + } else { + messages = formatWebpackMessages( + stats.toJson({ all: false, warnings: true, errors: true }) + ); } - const messages = formatWebpackMessages( - stats.toJson({ all: false, warnings: true, errors: true }) - ); if (messages.errors.length) { // Only keep the first error. Others are often indicative // of the same problem, but confuse the reader with noise. diff --git a/tasks/e2e-behavior.sh b/tasks/e2e-behavior.sh index 9ee3c8555d8..4a48b423b35 100755 --- a/tasks/e2e-behavior.sh +++ b/tasks/e2e-behavior.sh @@ -96,10 +96,10 @@ git clean -df # ****************************************************************************** # Smoke tests -./node_modules/.bin/jest --config fixtures/smoke/jest.config.js +CI=true ./node_modules/.bin/jest --config fixtures/smoke/jest.config.js # Output tests -./node_modules/.bin/jest --config fixtures/output/jest.config.js +CI=true ./node_modules/.bin/jest --config fixtures/output/jest.config.js # Cleanup cleanup