Skip to content

Add global deprecate warning on create-react-app package #7452

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

Closed
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
5 changes: 5 additions & 0 deletions packages/create-react-app/createReactApp.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const dns = require('dns');
const envinfo = require('envinfo');
const execSync = require('child_process').execSync;
const fs = require('fs-extra');
const isInstalledGlobally = require('is-installed-globally');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a simple way to achieve this without an extra dependency?

Copy link
Author

Choose a reason for hiding this comment

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

We need to get a current global path for npm or yarn and check it against the current executing path for npm or yarn.

Not sure if it possible internally without dependency in create-react-app script.

const hyperquest = require('hyperquest');
const inquirer = require('inquirer');
const os = require('os');
Expand Down Expand Up @@ -124,6 +125,10 @@ const program = new commander.Command(packageJson.name)
})
.parse(process.argv);

if (isInstalledGlobally) {
console.log(`global installation for create-react-app is deprecated.`);
Copy link

Choose a reason for hiding this comment

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

I think is good if you use console.warn with chalk.yellow to give a better warning message:

console.warn(`${chalk.yellow('global installation for create-react-app is deprecated.')}`); 

Copy link
Contributor

@mrmckeb mrmckeb Aug 5, 2019

Choose a reason for hiding this comment

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

I don't agree @Taym95, as this is Node, not the browser.

But I do agree that we should use colouring! :)

A few other thoughts:

  • Perhaps this should be the first thing emitted from the script? Above program?
  • Could the copy be more descriptive? Something like:

Global installations of create-react-app are now deprecated.

To remove the globally installed package, run npm uninstall -g create-react-app.

Updated installation instructions can be found here: https://facebook.github.io/create-react-app/docs/getting-started#quick-start

Edit: Actually @Taym95, I've noted that we do use console.warn in other places in our Node scripts - so that's a great suggestion ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

We may need a short URL for that - @ianschmitz, do you know if we have one?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not to my knowledge @mrmckeb.

}

if (program.info) {
console.log(chalk.bold('\nEnvironment Info:'));
return envinfo
Expand Down
1 change: 1 addition & 0 deletions packages/create-react-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"fs-extra": "7.0.1",
"hyperquest": "2.1.3",
"inquirer": "6.2.2",
"is-installed-globally": "0.2.0",
"semver": "6.0.0",
"tar-pack": "3.4.1",
"tmp": "0.0.33",
Expand Down