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
facebook · Jan 15, 2018 · 2e59c54 · 2e59c54
1 parent 0ff2349
commit 2e59c54
Show file tree

Hide file tree

Showing 9 changed files with 138 additions and 59 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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.