-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
eject removes linked react-scripts copy instead of symlink #1732
Comments
btw. Also hardcode at check if react-scripts is linked at paths.js#L116 |
Suggested implementation: 1. Read name from
|
Why do you not like the second option? "Don't destroy what isn't yours" seems like a generally useful maxim to contain damage in case of other potential bugs in the future. |
It is more clear for me because - in this way
Sounds very reasonable but If you vote for 2nd way, then I suggest little update: wrap eject.js:160 with such if (ownPath.indexOf(appPath) === 0) {
// remove react-scripts and react-scripts binaries from app node_modules
} and read name from const { name } = require('../package.json');
const reactScriptsPath = path.resolve(`node_modules/${name}`);
const reactScriptsLinked = fs.existsSync(reactScriptsPath) && fs.lstatSync(reactScriptsPath).isSymbolicLink(); |
Yep, let's do it like this. |
@tuchk4 Do you want to send a PR for this? |
@gaearon yeap. |
Fixed in #1736. |
Starts here #1356 (comment)
ownPath
- path toreact-scripts
root.If
react-scripts
is linked - app node_modules looks like this1st and 2nd cases are the same expect
eject
- we remove (eject.js:160)react-scripts
from node_modules after eject.In 2nd case local CRA packages/react-scripts will be removed instead of app node_modules/react-scripts.
I had unpleasant situation when run eject at app with linked react-scripts and my local CRA packages/react-scripts were removed with uncommitted changes :(
I have another implementation without name hardcode. I will test it and make new PR
The text was updated successfully, but these errors were encountered: