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.

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

Jump to bottom

Ejecting when scripts folder already exists causes eject to fail permanently #939

Closed
heldinz opened this issue Oct 21, 2016 · 2 comments
Closed

Ejecting when scripts folder already exists causes eject to fail permanently #939

heldinz opened this issue Oct 21, 2016 · 2 comments
Labels
issue: bug

Comments

@heldinz
Copy link
Contributor

heldinz commented Oct 21, 2016

We ran npm run eject and answered y at the prompt. It logged Ejecting... and then stopped with no further output. No config or scripts were copied over, just the new folders config/jest.

We realised that the problem might be that we already had a folder called scripts. We renamed this folder and re-ran the eject command, but got the same result.

I'm fairly certain that the error occurs when the eject script tries to create the scripts folder but it could just as easily have occurred when creating the config or config/jest folders, had we already had existing folders with those names in our project. Although the script checks for the presence of files existing at the path it will copy them to, it does not check for the presence of the folders before creating those.

The eject script should either fail early with the error that the folder already exists (as it does with the files), or it should check for the presence of the folder before attempting to create it. Instead, it fails silently.
@gaearon
Copy link
Contributor

gaearon commented Oct 21, 2016

It should definitely fail early in such cases. I think this used to work but we might have broken it at some point. Would you mind looking into fixing this?

@heldinz
Copy link
Contributor Author

heldinz commented Oct 21, 2016

Sure!

heldinz added a commit to heldinz/create-react-app that referenced this issue Oct 24, 2016
@heldinz
Check for presence of folders before continuing eject. Closes faceboo…
996fd33 
…k#939.
@heldinz heldinz mentioned this issue Oct 24, 2016
Merged
gaearon pushed a commit that referenced this issue Oct 28, 2016
@heldinz @gaearon
Check for presence of folders before continuing eject. Closes #939. (#…
7ce4b6e 
…951)
kitze added a commit to kitze/custom-react-scripts that referenced this issue Nov 4, 2016
@kitze
Merge branch 'master' of https://github.com/facebookincubator/create-…
d444609 
…react-app

# By Ville Immonen (11) and others
# Via Dan Abramov
* 'master' of https://github.com/facebookincubator/create-react-app: (39 commits)
  Remove redundant `function` from export statement (facebook#996)
  Add Gatsby to alternatives (facebook#995)
  Allow webpack 2 as peerDependency in react-dev-utils (facebook#963)
  Remove custom babel-loader cache dir config (facebook#983)
  Check for presence of folders before continuing eject. Closes facebook#939. (facebook#951)
  Fixes facebook#952 (facebook#953)
  Always build before deploying to gh-pages (facebook#959)
  Add collectCoverageFrom option to collect coverage on files without any tests. (facebook#961)
  Catch and noop call to open web browser. (facebook#964)
  Gently nudge users towards https by default (facebook#974)
  Enable compression on webpack-dev-server (facebook#966) (facebook#968)
  Add next.js to Alternatives
  Point people to npm Windows instructions
  Encourage people to try recent npm
  Fix an attribution link in 0.7.0 changelog
  Update CLI version in changelog
  Publish
  Update eslint-config-react-app version in the guide
  Update changelog for 0.7.0
  Revert "Temporarily remove 0.7.0 changelog as it's not out yet"
  ...

Conflicts:
	packages/babel-preset-react-app/package.json
	packages/create-react-app/package.json
	packages/eslint-config-react-app/package.json
	packages/react-dev-utils/package.json
	packages/react-scripts/config/webpack.config.dev.js
	packages/react-scripts/config/webpack.config.prod.js
	packages/react-scripts/package.json
eXtreme added a commit to eXtreme/create-react-app that referenced this issue Nov 18, 2016
@eXtreme
Merge pull request #2
7747830 
* pull2:
  Support Yarn (facebook#898)
  Fix chrome tab reuse (facebook#1035)
  Remove unnecessary transform plugins for object spread to work (facebook#1052)
  Clears the usage of react-jsx-source & react-jsx-self (facebook#992)
  Update babel-present-env and use node: 'current' as target (facebook#1051)
  Remove redundant `function` from export statement (facebook#996)
  Add Gatsby to alternatives (facebook#995)
  Allow webpack 2 as peerDependency in react-dev-utils (facebook#963)
  Remove custom babel-loader cache dir config (facebook#983)
  Check for presence of folders before continuing eject. Closes facebook#939. (facebook#951)
  Fixes facebook#952 (facebook#953)
  Always build before deploying to gh-pages (facebook#959)
  Add collectCoverageFrom option to collect coverage on files without any tests. (facebook#961)
  Catch and noop call to open web browser. (facebook#964)
  Gently nudge users towards https by default (facebook#974)
  Enable compression on webpack-dev-server (facebook#966) (facebook#968)
  Add next.js to Alternatives
  Point people to npm Windows instructions
  Encourage people to try recent npm

# Conflicts:
#	packages/react-scripts/config/webpack.config.dev.js
#	packages/react-scripts/package.json
#	packages/react-scripts/utils/createJestConfig.js
jarlef pushed a commit to jarlef/create-react-app that referenced this issue Nov 28, 2016
@heldinz @jarlef
Check for presence of folders before continuing eject. Closes faceboo…
2572c2a 
…k#939. (facebook#951)
alexdriaguine pushed a commit to alexdriaguine/create-react-app that referenced this issue Jan 23, 2017
@heldinz @alexdriaguine
Check for presence of folders before continuing eject. Closes faceboo…
0c0d144 
…k#939. (facebook#951)
randycoulman pushed a commit to CodingZeal/create-react-app that referenced this issue May 8, 2017
@heldinz @randycoulman
Check for presence of folders before continuing eject. Closes faceboo…
72928de 
…k#939. (facebook#951)
maltestenzel pushed a commit to maltestenzel/custom-react-scripts that referenced this issue Mar 7, 2018
@kitze
Merge branch 'master' of https://github.com/facebookincubator/create-…
9e4a35f 
…react-app

# By Ville Immonen (11) and others
# Via Dan Abramov
* 'master' of https://github.com/facebookincubator/create-react-app: (39 commits)
  Remove redundant `function` from export statement (facebook#996)
  Add Gatsby to alternatives (facebook#995)
  Allow webpack 2 as peerDependency in react-dev-utils (facebook#963)
  Remove custom babel-loader cache dir config (facebook#983)
  Check for presence of folders before continuing eject. Closes facebook#939. (facebook#951)
  Fixes facebook#952 (facebook#953)
  Always build before deploying to gh-pages (facebook#959)
  Add collectCoverageFrom option to collect coverage on files without any tests. (facebook#961)
  Catch and noop call to open web browser. (facebook#964)
  Gently nudge users towards https by default (facebook#974)
  Enable compression on webpack-dev-server (facebook#966) (facebook#968)
  Add next.js to Alternatives
  Point people to npm Windows instructions
  Encourage people to try recent npm
  Fix an attribution link in 0.7.0 changelog
  Update CLI version in changelog
  Publish
  Update eslint-config-react-app version in the guide
  Update changelog for 0.7.0
  Revert "Temporarily remove 0.7.0 changelog as it's not out yet"
  ...

Conflicts:
	packages/babel-preset-react-app/package.json
	packages/create-react-app/package.json
	packages/eslint-config-react-app/package.json
	packages/react-dev-utils/package.json
	packages/react-scripts/config/webpack.config.dev.js
	packages/react-scripts/config/webpack.config.prod.js
	packages/react-scripts/package.json
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
issue: bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gaearon @heldinz