From 2e59c5412c50ff22fc092cd21069c685ce486673 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Sun, 14 Jan 2018 13:54:01 -0500 Subject: [PATCH] Offer to set default browsers (#3792) * Offer to set browser defaults * Catch error on no * Add ending newlines * Ensure we re-check to prevent defaults from leaking * Reduce nesting * Add defaults message * More explicit --- packages/create-react-app/createReactApp.js | 10 +- .../react-dev-utils/WebpackDevServerUtils.js | 2 +- packages/react-dev-utils/browsersHelper.js | 111 ++++++++++++++---- packages/react-dev-utils/package.json | 1 + packages/react-scripts/package.json | 13 +- packages/react-scripts/scripts/build.js | 27 +++-- packages/react-scripts/scripts/eject.js | 3 +- packages/react-scripts/scripts/init.js | 11 +- packages/react-scripts/scripts/start.js | 19 ++- 9 files changed, 138 insertions(+), 59 deletions(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index a2d9fdbd86b..c289f2d7cad 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -48,6 +48,7 @@ const unpack = require('tar-pack').unpack; const url = require('url'); const hyperquest = require('hyperquest'); const envinfo = require('envinfo'); +const os = require('os'); const packageJson = require('./package.json'); @@ -173,7 +174,7 @@ function createApp(name, verbose, version, useNpm, template) { }; fs.writeFileSync( path.join(root, 'package.json'), - JSON.stringify(packageJson, null, 2) + JSON.stringify(packageJson, null, 2) + os.EOL ); const useYarn = useNpm ? false : shouldUseYarn(); @@ -481,7 +482,10 @@ function getPackageName(installPackage) { ); } else if (installPackage.match(/^file:/)) { const installPackagePath = installPackage.match(/^file:(.*)?$/)[1]; - const installPackageJson = require(path.join(installPackagePath, 'package.json')); + const installPackageJson = require(path.join( + installPackagePath, + 'package.json' + )); return Promise.resolve(installPackageJson.name); } return Promise.resolve(installPackage); @@ -600,7 +604,7 @@ function setCaretRangeForRuntimeDeps(packageName) { makeCaretRange(packageJson.dependencies, 'react'); makeCaretRange(packageJson.dependencies, 'react-dom'); - fs.writeFileSync(packagePath, JSON.stringify(packageJson, null, 2)); + fs.writeFileSync(packagePath, JSON.stringify(packageJson, null, 2) + os.EOL); } // If project only contains files generated by GH, it’s safe. diff --git a/packages/react-dev-utils/WebpackDevServerUtils.js b/packages/react-dev-utils/WebpackDevServerUtils.js index 4add9f9c1bc..1208d7fc776 100644 --- a/packages/react-dev-utils/WebpackDevServerUtils.js +++ b/packages/react-dev-utils/WebpackDevServerUtils.js @@ -318,7 +318,7 @@ function prepareProxy(proxy, appPublicFolder) { // However we also want to respect `proxy` for API calls. // So if `proxy` is specified as a string, we need to decide which fallback to use. // We use a heuristic: We want to proxy all the requests that are not meant - // for static assets and as all the requests for static assets will be using + // for static assets and as all the requests for static assets will be using // `GET` method, we can proxy all non-`GET` requests. // For `GET` requests, if request `accept`s text/html, we pick /index.html. // Modern browsers include text/html into `accept` header when navigating. diff --git a/packages/react-dev-utils/browsersHelper.js b/packages/react-dev-utils/browsersHelper.js index 8358f851870..02e46d3ecf5 100644 --- a/packages/react-dev-utils/browsersHelper.js +++ b/packages/react-dev-utils/browsersHelper.js @@ -9,36 +9,99 @@ const browserslist = require('browserslist'); const chalk = require('chalk'); const os = require('os'); +const inquirer = require('inquirer'); +const pkgUp = require('pkg-up'); +const fs = require('fs'); -function checkBrowsers(dir) { - const found = browserslist.findConfig(dir); +const defaultBrowsers = { + development: ['chrome', 'firefox', 'edge'].map( + browser => `last 2 ${browser} versions` + ), + production: ['>1%', 'last 4 versions', 'Firefox ESR', 'not ie < 11'], +}; - if (found == null) { - console.log( - chalk.red('As of react-scripts >=2 you must specify targeted browsers.') + - os.EOL + - `Please add a ${chalk.underline( - 'browserslist' - )} key to your ${chalk.bold('package.json')}.` +function checkBrowsers(dir, retry = true) { + const current = browserslist.findConfig(dir); + if (current != null) { + return Promise.resolve(current); + } + + if (!retry) { + return Promise.reject( + new Error( + chalk.red( + 'As of react-scripts >=2 you must specify targeted browsers.' + ) + + os.EOL + + `Please add a ${chalk.underline( + 'browserslist' + )} key to your ${chalk.bold('package.json')}.` + ) ); - return null; } - return found; + + const question = { + type: 'confirm', + name: 'shouldSetBrowsers', + message: + chalk.yellow("We're unable to detect target browsers.") + + `\n\nWould you like to add the defaults to your ${chalk.bold( + 'package.json' + )}?`, + default: true, + }; + return inquirer.prompt(question).then(answer => { + if (answer.shouldSetBrowsers) { + return ( + pkgUp(dir) + .then(filePath => { + if (filePath == null) { + return Promise.reject(); + } + const pkg = JSON.parse(fs.readFileSync(filePath)); + pkg['browserslist'] = defaultBrowsers; + fs.writeFileSync(filePath, JSON.stringify(pkg, null, 2) + os.EOL); + + browserslist.clearCaches(); + console.log(); + console.log(chalk.green('Set target browsers:')); + console.log(); + console.log( + `\t${chalk.bold('Production')}: ${chalk.cyan( + defaultBrowsers.production.join(', ') + )}` + ); + console.log( + `\t${chalk.bold('Development')}: ${chalk.cyan( + defaultBrowsers.development.join(', ') + )}` + ); + console.log(); + }) + // Swallow any error + .catch(() => {}) + .then(() => checkBrowsers(dir, false)) + ); + } else { + return checkBrowsers(dir, false); + } + }); } function printBrowsers(dir) { - let browsers = checkBrowsers(dir); - if (browsers == null) { - console.log('Built the bundle with default browser support.'); - return; - } - browsers = browsers[process.env.NODE_ENV] || browsers; - if (Array.isArray(browsers)) { - browsers = browsers.join(', '); - } - console.log( - `Built the bundle with browser support for ${chalk.cyan(browsers)}.` - ); + return checkBrowsers(dir).then(browsers => { + if (browsers == null) { + console.log('Built the bundle with default browser support.'); + return; + } + browsers = browsers[process.env.NODE_ENV] || browsers; + if (Array.isArray(browsers)) { + browsers = browsers.join(', '); + } + console.log( + `Built the bundle with browser support for ${chalk.cyan(browsers)}.` + ); + }); } -module.exports = { checkBrowsers, printBrowsers }; +module.exports = { defaultBrowsers, checkBrowsers, printBrowsers }; diff --git a/packages/react-dev-utils/package.json b/packages/react-dev-utils/package.json index c8f086ced87..1f9cc58884e 100644 --- a/packages/react-dev-utils/package.json +++ b/packages/react-dev-utils/package.json @@ -50,6 +50,7 @@ "inquirer": "5.0.0", "is-root": "1.0.0", "opn": "5.2.0", + "pkg-up": "2.0.0", "react-error-overlay": "^4.0.0", "recursive-readdir": "2.2.1", "shell-quote": "1.6.1", diff --git a/packages/react-scripts/package.json b/packages/react-scripts/package.json index a109bee5694..84d0faf615e 100644 --- a/packages/react-scripts/package.json +++ b/packages/react-scripts/package.json @@ -70,7 +70,16 @@ "fsevents": "1.1.2" }, "browserslist": { - "development": "last 2 chrome versions", - "production": [">1%", "last 4 versions", "Firefox ESR", "not ie < 11"] + "development": [ + "last 2 chrome versions", + "last 2 firefox versions", + "last 2 edge versions" + ], + "production": [ + ">1%", + "last 4 versions", + "Firefox ESR", + "not ie < 11" + ] } } diff --git a/packages/react-scripts/scripts/build.js b/packages/react-scripts/scripts/build.js index a4ba1cfcdb3..cca5ff28313 100644 --- a/packages/react-scripts/scripts/build.js +++ b/packages/react-scripts/scripts/build.js @@ -41,13 +41,6 @@ const printHostingInstructions = require('react-dev-utils/printHostingInstructio const FileSizeReporter = require('react-dev-utils/FileSizeReporter'); const printBuildError = require('react-dev-utils/printBuildError'); const { printBrowsers } = require('react-dev-utils/browsersHelper'); -// @remove-on-eject-begin -// Require browsers to be specified before you eject -const { checkBrowsers } = require('react-dev-utils/browsersHelper'); -if (!checkBrowsers(paths.appPath)) { - process.exit(1); -} -// @remove-on-eject-end const measureFileSizesBeforeBuild = FileSizeReporter.measureFileSizesBeforeBuild; @@ -63,9 +56,15 @@ if (!checkRequiredFiles([paths.appHtml, paths.appIndexJs])) { process.exit(1); } -// First, read the current file sizes in build directory. -// This lets us display how much they changed later. -measureFileSizesBeforeBuild(paths.appBuild) +// We require that you explictly set browsers and do not fall back to +// browserslist defaults. +const { checkBrowsers } = require('react-dev-utils/browsersHelper'); +checkBrowsers(paths.appPath) + .then(() => { + // First, read the current file sizes in build directory. + // This lets us display how much they changed later. + return measureFileSizesBeforeBuild(paths.appBuild); + }) .then(previousFileSizes => { // Remove all content but keep the directory so that // if you're in it, you don't end up in Trash @@ -122,7 +121,13 @@ measureFileSizesBeforeBuild(paths.appBuild) printBuildError(err); process.exit(1); } - ); + ) + .catch(err => { + if (err && err.message) { + console.log(err.message); + } + process.exit(1); + }); // Create the production build and print the deployment instructions. function build(previousFileSizes) { diff --git a/packages/react-scripts/scripts/eject.js b/packages/react-scripts/scripts/eject.js index c8438f5d156..558aa0b0b96 100644 --- a/packages/react-scripts/scripts/eject.js +++ b/packages/react-scripts/scripts/eject.js @@ -22,6 +22,7 @@ const paths = require('../config/paths'); const createJestConfig = require('./utils/createJestConfig'); const inquirer = require('react-dev-utils/inquirer'); const spawnSync = require('react-dev-utils/crossSpawn').sync; +const os = require('os'); const green = chalk.green; const cyan = chalk.cyan; @@ -218,7 +219,7 @@ inquirer fs.writeFileSync( path.join(appPath, 'package.json'), - JSON.stringify(appPackage, null, 2) + '\n' + JSON.stringify(appPackage, null, 2) + os.EOL ); console.log(); diff --git a/packages/react-scripts/scripts/init.js b/packages/react-scripts/scripts/init.js index 7854a803771..38deea60259 100644 --- a/packages/react-scripts/scripts/init.js +++ b/packages/react-scripts/scripts/init.js @@ -18,6 +18,8 @@ const fs = require('fs-extra'); const path = require('path'); const chalk = require('chalk'); const spawn = require('react-dev-utils/crossSpawn'); +const { defaultBrowsers } = require('react-dev-utils/browsersHelper'); +const os = require('os'); module.exports = function( appPath, @@ -43,16 +45,11 @@ module.exports = function( eject: 'react-scripts eject', }; - appPackage.browserslist = { - development: ['chrome', 'firefox', 'edge'].map( - browser => `last 2 ${browser} versions` - ), - production: ['>1%', 'last 4 versions', 'Firefox ESR', 'not ie < 11'], - }; + appPackage.browserslist = defaultBrowsers; fs.writeFileSync( path.join(appPath, 'package.json'), - JSON.stringify(appPackage, null, 2) + JSON.stringify(appPackage, null, 2) + os.EOL ); const readmeExists = fs.existsSync(path.join(appPath, 'README.md')); diff --git a/packages/react-scripts/scripts/start.js b/packages/react-scripts/scripts/start.js index 3694cce57cd..0e9e8b4e118 100644 --- a/packages/react-scripts/scripts/start.js +++ b/packages/react-scripts/scripts/start.js @@ -48,13 +48,6 @@ const createDevServerConfig = require('../config/webpackDevServer.config'); const useYarn = fs.existsSync(paths.yarnLockFile); const isInteractive = process.stdout.isTTY; -// @remove-on-eject-begin -// Require browsers to be specified before you eject -const { checkBrowsers } = require('react-dev-utils/browsersHelper'); -if (!checkBrowsers(paths.appPath)) { - process.exit(1); -} -// @remove-on-eject-end // Warn and crash if required files are missing if (!checkRequiredFiles([paths.appHtml, paths.appIndexJs])) { @@ -80,9 +73,15 @@ if (process.env.HOST) { console.log(); } -// We attempt to use the default port but if it is busy, we offer the user to -// run on a different port. `choosePort()` Promise resolves to the next free port. -choosePort(HOST, DEFAULT_PORT) +// We require that you explictly set browsers and do not fall back to +// browserslist defaults. +const { checkBrowsers } = require('react-dev-utils/browsersHelper'); +checkBrowsers(paths.appPath) + .then(() => { + // We attempt to use the default port but if it is busy, we offer the user to + // run on a different port. `choosePort()` Promise resolves to the next free port. + return choosePort(HOST, DEFAULT_PORT); + }) .then(port => { if (port == null) { // We have not found a port.