Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put react-scripts in dependencies, not devDependencies #2657

Merged
merged 3 commits into from
Jun 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 5 additions & 16 deletions packages/create-react-app/createReactApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,7 @@ function run(
})
.then(packageName => {
checkNodeVersion(packageName);

// Since react-scripts has been installed with --save
// we need to move it into devDependencies and rewrite package.json
// also ensure react dependencies have caret version range
fixDependencies(packageName);
setCaretRangeForRuntimeDeps(packageName);

const scriptsPath = path.resolve(
process.cwd(),
Expand Down Expand Up @@ -497,16 +493,14 @@ function checkAppName(appName) {
}

// TODO: there should be a single place that holds the dependencies
const dependencies = ['react', 'react-dom'];
const devDependencies = ['react-scripts'];
const allDependencies = dependencies.concat(devDependencies).sort();
if (allDependencies.indexOf(appName) >= 0) {
const dependencies = ['react', 'react-dom', 'react-scripts'].sort();
if (dependencies.indexOf(appName) >= 0) {
console.error(
chalk.red(
`We cannot create a project called ${chalk.green(appName)} because a dependency with the same name exists.\n` +
`Due to the way npm works, the following names are not allowed:\n\n`
) +
chalk.cyan(allDependencies.map(depName => ` ${depName}`).join('\n')) +
chalk.cyan(dependencies.map(depName => ` ${depName}`).join('\n')) +
chalk.red('\n\nPlease choose a different project name.')
);
process.exit(1);
Expand All @@ -533,7 +527,7 @@ function makeCaretRange(dependencies, name) {
dependencies[name] = patchedVersion;
}

function fixDependencies(packageName) {
function setCaretRangeForRuntimeDeps(packageName) {
const packagePath = path.join(process.cwd(), 'package.json');
const packageJson = require(packagePath);

Expand All @@ -543,16 +537,11 @@ function fixDependencies(packageName) {
}

const packageVersion = packageJson.dependencies[packageName];

if (typeof packageVersion === 'undefined') {
console.error(chalk.red(`Unable to find ${packageName} in package.json`));
process.exit(1);
}

packageJson.devDependencies = packageJson.devDependencies || {};
packageJson.devDependencies[packageName] = packageVersion;
delete packageJson.dependencies[packageName];

makeCaretRange(packageJson.dependencies, 'react');
makeCaretRange(packageJson.dependencies, 'react-dom');

Expand Down
22 changes: 16 additions & 6 deletions packages/react-scripts/scripts/eject.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,24 +146,34 @@ inquirer

console.log(cyan('Updating the dependencies'));
const ownPackageName = ownPackage.name;
if (appPackage.devDependencies[ownPackageName]) {
console.log(` Removing ${cyan(ownPackageName)} from devDependencies`);
delete appPackage.devDependencies[ownPackageName];
if (appPackage.devDependencies) {
// We used to put react-scripts in devDependencies
if (appPackage.devDependencies[ownPackageName]) {
console.log(` Removing ${cyan(ownPackageName)} from devDependencies`);
delete appPackage.devDependencies[ownPackageName];
}
}
appPackage.dependencies = appPackage.dependencies || {};
if (appPackage.dependencies[ownPackageName]) {
console.log(` Removing ${cyan(ownPackageName)} from dependencies`);
delete appPackage.dependencies[ownPackageName];
}

Object.keys(ownPackage.dependencies).forEach(key => {
// For some reason optionalDependencies end up in dependencies after install
if (ownPackage.optionalDependencies[key]) {
return;
}
console.log(` Adding ${cyan(key)} to devDependencies`);
appPackage.devDependencies[key] = ownPackage.dependencies[key];
console.log(` Adding ${cyan(key)} to dependencies`);
appPackage.dependencies[key] = ownPackage.dependencies[key];
});
// Sort the deps
const unsortedDependencies = appPackage.dependencies;
appPackage.dependencies = {};
Object.keys(unsortedDependencies).sort().forEach(key => {
appPackage.dependencies[key] = unsortedDependencies[key];
});
console.log();

console.log(cyan('Updating the scripts'));
delete appPackage.scripts['eject'];
Object.keys(appPackage.scripts).forEach(key => {
Expand Down
1 change: 0 additions & 1 deletion packages/react-scripts/scripts/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ module.exports = function(

// Copy over some of the devDependencies
appPackage.dependencies = appPackage.dependencies || {};
appPackage.devDependencies = appPackage.devDependencies || {};

// Setup the script rules
appPackage.scripts = {
Expand Down
9 changes: 0 additions & 9 deletions tasks/e2e-installs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,6 @@ function checkDependencies {
echo "There are extraneous dependencies in package.json"
exit 1
fi


if ! awk '/"devDependencies": {/{y=1;next}/},/{y=0; next}y' package.json | \
grep -v -q -E '^\s*"react(-dom|-scripts)?"'; then
echo "Dev Dependencies are correct"
else
echo "There are extraneous devDependencies in package.json"
exit 1
fi
}

function create_react_app {
Expand Down