Skip to content

Commit

Permalink
Offer to set default browsers (#3792)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Timer authored Jan 14, 2018
1 parent c012cc1 commit 727f487
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 59 deletions.
10 changes: 7 additions & 3 deletions packages/create-react-app/createReactApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dev-utils/WebpackDevServerUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
111 changes: 87 additions & 24 deletions packages/react-dev-utils/browsersHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
1 change: 1 addition & 0 deletions packages/react-dev-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "^3.0.0",
"recursive-readdir": "2.2.1",
"shell-quote": "1.6.1",
Expand Down
13 changes: 11 additions & 2 deletions packages/react-scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
}
27 changes: 16 additions & 11 deletions packages/react-scripts/scripts/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-scripts/scripts/eject.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
11 changes: 4 additions & 7 deletions packages/react-scripts/scripts/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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'));
Expand Down
19 changes: 9 additions & 10 deletions packages/react-scripts/scripts/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -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])) {
Expand All @@ -65,9 +58,15 @@ if (!checkRequiredFiles([paths.appHtml, paths.appIndexJs])) {
const DEFAULT_PORT = parseInt(process.env.PORT, 10) || 3000;
const HOST = process.env.HOST || '0.0.0.0';

// 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.
Expand Down

0 comments on commit 727f487

Please sign in to comment.