Skip to content

Beaufity output of eject.js script #769

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.

Sign up for GitHub

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

Merged
gaearon merged 18 commits into from
Sep 30, 2016
Merged

Beaufity output of eject.js script #769

Changes from 3 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
350b2bc
Beaufity output of eject.js script
azakordonets Sep 26, 2016
822ad8c
change formatting of the eject.js output and move colors to cyan
azakordonets Sep 27, 2016
bfe4850
change message about file copy
azakordonets Sep 27, 2016
1c5f505
add missing three dots to some statements in eject.js script
azakordonets Sep 27, 2016
173e87c
change color of "copying files" line and do not repeat copy path anym…
azakordonets Sep 28, 2016
41345b7
fix merge conflict
azakordonets Sep 28, 2016
259b6f5
Remove yellow color from "Removing dependency" line
azakordonets Sep 28, 2016
52b7123
changing color to "Adding dependency" line
azakordonets Sep 28, 2016
917404e
Add line that outputs which react script is getting replaced by simil…
azakordonets Sep 28, 2016
1a8ff9e
remove not used anymore colors
azakordonets Sep 28, 2016
4b9fa8a
add console line about updating Jest configs
azakordonets Sep 28, 2016
f6f635d
fix typo
azakordonets Sep 28, 2016
79a4c2b
change formatting of replacing script output in eject.js
azakordonets Sep 30, 2016
c440b27
fix merge conflicts
azakordonets Sep 30, 2016
52c432f
remove "Writing package.json" file console output
azakordonets Sep 30, 2016
308a21e
make quotes cyan in "Replacing script" console output
azakordonets Sep 30, 2016
2e208c9
update console log output for Jest, Babel, ESLint update and group th…
azakordonets Sep 30, 2016
40fa310
Style nits
gaearon Sep 30, 2016
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
36 changes: 21 additions & 15 deletions packages/react-scripts/scripts/eject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,21 @@ var path = require('path');
var prompt = require('react-dev-utils/prompt');
var rimrafSync = require('rimraf').sync;
var spawnSync = require('cross-spawn').sync;
var chalk = require('chalk');
var green = chalk.green;
var cyan = chalk.cyan;
var red = chalk.red;

prompt(
'Are you sure you want to eject? This action is permanent.',
false
).then(shouldEject => {
if (!shouldEject) {
console.log('Close one! Eject aborted.');
console.log(cyan('Close one! Eject aborted.'));
process.exit(1);
}

console.log('Ejecting...');
console.log();

var ownPath = path.join(__dirname, '..');
var appPath = path.join(ownPath, '..', '..');
Expand Down Expand Up @@ -61,8 +64,10 @@ prompt(
fs.mkdirSync(path.join(appPath, 'config', 'jest'));
fs.mkdirSync(path.join(appPath, 'scripts'));

console.log();
console.log(cyan('Copying files'));

@gaearon gaearon Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s add ... at the end of every group title

files.forEach(function(file) {
console.log('Copying ' + file + ' to ' + appPath);
console.log(cyan(' Copying ') + file+ ' to ' + appPath);

@gaearon gaearon Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try making file and appPath cyan instead of Copying?

@vjeux vjeux Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the output, it prints the same giant appPath on every single line.

Suggestion: could we put this path in the header instead?

Copying files to /Users/.../tempApp ...
  Copying .babelrc
  Copying .eslintrc

var content = fs
.readFileSync(path.join(ownPath, file), 'utf8')
// Remove dead code from .js files on eject
Expand All @@ -76,21 +81,21 @@ prompt(

var ownPackage = require(path.join(ownPath, 'package.json'));
var appPackage = require(path.join(appPath, 'package.json'));

console.log(cyan('Managing dependencies'));

@gaearon gaearon Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s call this Updating dependencies...

var ownPackageName = ownPackage.name;
console.log('Removing dependency: ' + ownPackageName);
console.log(red(' Removing dependency: ') + ownPackageName);

@gaearon gaearon Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s use yellow here so it doesn’t feel like an error

delete appPackage.devDependencies[ownPackageName];

Object.keys(ownPackage.dependencies).forEach(function (key) {
// For some reason optionalDependencies end up in dependencies after install
if (ownPackage.optionalDependencies[key]) {
return;
}
console.log('Adding dependency: ' + key);
console.log(cyan(' Adding dependency: ') + key);

@gaearon gaearon Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar, let’s make dependency name cyan (or yellow) instead of the label.

appPackage.devDependencies[key] = ownPackage.dependencies[key];
});

console.log('Updating scripts');
console.log();
console.log(cyan('Updating scripts'));

@gaearon gaearon Sep 27, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, let’s add ...

delete appPackage.scripts['eject'];
Object.keys(appPackage.scripts).forEach(function (key) {
appPackage.scripts[key] = appPackage.scripts[key]

@gaearon gaearon Sep 27, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s print each step here?
For example:

  Replacing "react-scripts start" with "node scripts/start.js"

Expand All @@ -107,20 +112,21 @@ prompt(
true
);

console.log('Writing package.json');
console.log();
console.log(cyan('Writing ') + 'package.json');

@gaearon gaearon Sep 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here

fs.writeFileSync(
path.join(appPath, 'package.json'),
JSON.stringify(appPackage, null, 2)
);
console.log();

console.log('Running npm install...');
console.log(cyan('Running npm install...'));
rimrafSync(ownPath);
spawnSync('npm', ['install'], {stdio: 'inherit'});
console.log('Ejected successfully!');
console.log(green('Ejected successfully!'));
console.log();

console.log('Please consider sharing why you ejected in this survey:');
console.log(' http://goo.gl/forms/Bi6CZjk1EqsdelXk1');
console.log();
});
console.log(green('Please consider sharing why you ejected in this survey:'));
console.log(green(' http://goo.gl/forms/Bi6CZjk1EqsdelXk1'));
console.log()
})