Appending CRA gitignore to existing .gitignore fails due to fs-extra v8

[mjsarfatti]

Describe the bug

Creating a new app in a folder with an existing .gitignore fails with error Error: dest already exists.. This is using the master branch (I discovered it while forking it to customize CRA), but it will happen on tagged releases as well.

The reason is react-scripts/scripts/init.js:175 checks for:

  if (err.code === 'EEXIST') {

in order to decide whether to append CRA's gitignore to the existing file. But fs-extra/moveSync v8 instead of throwing an error with code EEXIST seems to be throwing an error with message dest already exists. and undefined code.

Before v8: jprichardson/node-fs-extra@0bc36ff#diff-ed59bd97e02b23238a4db2f82738f5f3L29

After v8: jprichardson/node-fs-extra@0bc36ff#diff-04b1b5a0bc03a081b62ca97774ac1443R27

Did you try recovering your dependencies?

Not applicable.

Which terms did you search for in User Guide?

"EEXIST", "dest already exists", "gitignore".

Environment

Environment Info:

  System:
    OS: macOS 10.15
    CPU: (8) x64 Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
  Binaries:
    Node: 10.14.2 - /usr/local/bin/node
    Yarn: 1.16.0 - ~/.yarn/bin/yarn
    npm: 6.10.3 - ~/npm/bin/npm
  Browsers:
    Chrome: 77.0.3865.120
    Firefox: 70.0
    Safari: 13.0.2
  npmPackages:
    react: Not Found
    react-dom: Not Found
    react-scripts: Not Found
  npmGlobalPackages:
    create-react-app: Not Found

Steps to reproduce

1. Create a folder and a sample .gitignore file
2. cd into the folder and run npx create-react-app .

Expected behavior

App is initialized and CRA gitignore is appended to existing .gitignore

Actual behavior

Installation fails. Here is the last lines of the console log:

...
├─ [email protected]
├─ [email protected]
├─ [email protected]
├─ [email protected]
└─ [email protected]
✨  Done in 20.68s.
/Users/manus/Lavori/Web/AGENCY-APPS/2019/2019n13_Bip_SimPLE/simple-app/node_modules/react-scripts-laravel/scripts/init.js:182
      throw err;
      ^

Error: dest already exists.
    at doRename (/Users/manus/Lavori/Web/AGENCY-APPS/2019/2019n13_Bip_SimPLE/simple-app/node_modules/fs-extra/lib/move-sync/move-sync.js:25:34)
    at Object.moveSync (/Users/manus/Lavori/Web/AGENCY-APPS/2019/2019n13_Bip_SimPLE/simple-app/node_modules/fs-extra/lib/move-sync/move-sync.js:17:10)
    at module.exports (/Users/manus/Lavori/Web/AGENCY-APPS/2019/2019n13_Bip_SimPLE/simple-app/node_modules/react-scripts-laravel/scripts/init.js:168:8)
    at [eval]:3:14
    at Script.runInThisContext (vm.js:96:20)
    at Object.runInThisContext (vm.js:303:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at evalScript (internal/bootstrap/node.js:586:27)
    at startup (internal/bootstrap/node.js:264:9)

Aborting installation.
  node  has failed.

Deleting generated file... node_modules
Deleting generated file... package.json
Deleting generated file... yarn.lock
Done.

Reproducible demo

Not applicable.

[mjsarfatti]

Ps: I can submit a PR, but it might be a lot faster for a core team member to fix that line in init.js... But please do let me know!

[ianschmitz]

It looks like you're installing from a fork of CRA (react-scripts-laravel). Can you reproduce using our package?

[mjsarfatti]

Disclaimer: this issue is on the master branch, which I understand being unstable, but fixing it might prevent an issue once 3.3.0 is released.

It's weird though, I can't reproduce using your package, even though the only substantial thing I modified in mine are a few paths in packages/react-scripts/config/paths.js .

It shouldn't work though, because:
a. packages/react-scripts/package.json requires "fs-extra": "^8.1.0" (bumped in #7814)
b. fs-extra/moveSync v8 throws "dest already exists." instead of "EEXIST"
c. packages/react-scripts/scripts/init.js checks only for "EEXIST"

But maybe there are some internals I'm not fully grasping?

[miraage]

It is very strange to have the code property being anything other than existing codes which are used by the fs module.

I can't find any notes about these renamings in the fs-extra changelog.

[mjsarfatti]

I know, it's weird... Here is the culprit line: https://github.com/jprichardson/node-fs-extra/blob/8.1.0/lib/move-sync/move-sync.js#L25 (as you can see an error is thrown with no code attached)

And here is the diff v7/v8: jprichardson/node-fs-extra@0bc36ff#diff-ed59bd97e02b23238a4db2f82738f5f3

[bmuenzenmeyer]

also seeing this in a fork which is using [email protected] - and was about to customize init.js with more defensive code, only to realize that defensive code already exists but it's not being tripped, and hence this issue

I've isolated this snippet of code into two REPLs

✔️ [email protected] moveSync behavior with error.code
❌ [email protected] moveSync behavior without error.code

Not sure how to proceed. I I think the appropriate next step would be to ask fs-extra to throw a specific code?

as @mjsarfatti mentions, this will be a 3.3.0 bug if released. We opened it as a story because a common generation flow is to init a repo in GitHub first, with README and .gitignore files, and then CRA into that repo

EDIT: opened an issue on the fs-extra repo

EDIT 2: received word from the fs-extra team that this is now by design. If so, then, should this code change to look directly for the message dest already exists, or better yet, use fs.existsSync prior to the move and act accordingly

[bmuenzenmeyer]

I've confirmed in the PR that fixes this issue that this is now an active bug in [email protected]