Use yarn when running inside yarn workspace. #3997

bradfordlemley · 2018-02-08T22:18:38Z

Fixes this CI failure:

TypeError: Cannot read property 'loose' of undefined (While processing preset: "/tmp/tmp.XQoCgXQtTQ/yarn-ws/node_modules/babel-preset-react-app/index.js")

Timer · 2018-02-09T03:41:02Z

Don't we already do this?

create-react-app/packages/react-scripts/scripts/eject.js

Lines 227 to 256 in 47d2d94

    
           if (fs.existsSync(paths.yarnLockFile)) { 
        
             const windowsCmdFilePath = path.join( 
        
               appPath, 
        
               'node_modules', 
        
               '.bin', 
        
               'react-scripts.cmd' 
        
             ); 
        
             let windowsCmdFileContent; 
        
             if (process.platform === 'win32') { 
        
               // https://github.com/facebook/create-react-app/pull/3806#issuecomment-357781035 
        
               // Yarn is diligent about cleaning up after itself, but this causes the react-scripts.cmd file 
        
               // to be deleted while it is running. This trips Windows up after the eject completes. 
        
               // We'll read the batch file and later "write it back" to match npm behavior. 
        
               try { 
        
                 windowsCmdFileContent = fs.readFileSync(windowsCmdFilePath); 
        
               } catch (err) { 
        
                 // If this fails we're not worse off than if we didn't try to fix it. 
        
               } 
        
             } 
        
             console.log(cyan('Running yarn...')); 
        
             spawnSync('yarnpkg', ['--cwd', process.cwd()], { stdio: 'inherit' }); 
        
             if (windowsCmdFileContent && !fs.existsSync(windowsCmdFilePath)) { 
        
               try { 
        
                 fs.writeFileSync(windowsCmdFilePath, windowsCmdFileContent); 
        
               } catch (err) { 
        
                 // If this fails we're not worse off than if we didn't try to fix it. 
        
               } 
        
             }

gaearon · 2018-02-09T11:16:35Z

packages/react-scripts/scripts/eject.js

@@ -224,7 +233,7 @@ inquirer
      }
    }

-    if (fs.existsSync(paths.yarnLockFile)) {
+    if (fs.existsSync(paths.yarnLockFile) || isYarnAvailable()) {


If the problem is due to npm corrupting a Yarn-produced tree, this can be solved by deleting node_modules first before re-running npm.

i don't think it's corrupting, per-se -- it's just that if this is happening within a yarn workspace, npm can't handle it properly.

But I’d like to understand what’s going on here better. Why do we run Yarn in this test even if the user created an app with npm?

You mean echo yes | npm run eject, right? We can change that to yarn. In that case, maybe you want to detect that the eject script was run with yarn and automatically use yarn for the install? That would help enforce some consistency with yarn/npm usage, but not sure what your philosophy is on that. Let me know what you want to do in this PR, not sure this PR is the appropriate place for that auto-detecting change.

Hmm no, I just mean that if an app was created with npm, we should run npm on ejecting, and if it was created with Yarn, we should use Yarn.

We use the lockfile to tell if the app was created with Yarn. Is this not sufficient in this case? Why?

Let's extract that into react-dev-utils maybe and use in both places?
Or cache this in config/paths.js.

There are a bunch of places where a decision to use yarn or npm is made, probably all should be updated:

create-react-app/packages/create-react-app/createReactApp.js

Line 188 in c342677

const useYarn = useNpm ? false : shouldUseYarn();

create-react-app/packages/react-scripts/scripts/init.js

Line 88 in c342677

const useYarn = fs.existsSync(path.join(appPath, 'yarn.lock'));

create-react-app/packages/react-scripts/scripts/eject.js

Line 227 in c342677

if (fs.existsSync(paths.yarnLockFile)) {

create-react-app/packages/react-scripts/scripts/build.js

Line 48 in c342677

const useYarn = fs.existsSync(paths.yarnLockFile);

The first one intentionally checks for existence of Yarn because we haven't created a project yet.

All the other ones are checking for presence of yarn.lock. If it's committed, then the project was created with Yarn (maybe by another member of your team) so you should really install it (even if you don't have it).

I don't mind unifying all except the first one.

For createReactApp.js: if a user is creating an app in a yarn workspace, they really want to be using yarn, using npm will cause issues.

This sounds good.

gaearon · 2018-02-09T17:37:08Z

packages/create-react-app/createReactApp.js

@@ -49,7 +49,7 @@ const url = require('url');
 const hyperquest = require('hyperquest');
 const envinfo = require('envinfo');
 const os = require('os');
-
+const monorepo = require('react-dev-utils/monorepo');


Let's use a name likeWorkspaceUtils so it's clearer it's a bunch of functions.

gaearon · 2018-02-09T18:44:20Z

packages/create-react-app/createReactApp.js

@@ -49,7 +49,7 @@ const url = require('url');
 const hyperquest = require('hyperquest');
 const envinfo = require('envinfo');
 const os = require('os');
-const monorepo = require('react-dev-utils/monorepo');
+const getMonorepo = require('react-dev-utils/workspaceUtils').getMonorepo;


How about findMonorepo? Sorry for bikeshedding on this.

Users who don't use monorepos might be confused after ejecting because how would you "get" a monorepo that doesn't exist.

gaearon · 2018-02-10T13:07:22Z

Thank you!

Use yarn when running inside yarn workspace. (facebook#3997)

* Run yarn after ejecting. * On eject, choose to run yarn instead of npm if yarn is available. * Move monorepo to react-dev-utils. * Fix lint. * Rename monorepo to workspaceUtils. * Add react-dev-utils dep for create-react-app. * getMonorepo -> findMonorepo

This reverts commit 2c34d5b.

* Run yarn after ejecting. * On eject, choose to run yarn instead of npm if yarn is available. * Move monorepo to react-dev-utils. * Fix lint. * Rename monorepo to workspaceUtils. * Add react-dev-utils dep for create-react-app. * getMonorepo -> findMonorepo

This reverts commit 2c34d5b.

Run yarn after ejecting.

f1018cf

bradfordlemley changed the title ~~Run yarn after ejecting in e2e-monorepo.~~ Fix for e2e-monorepos failure. Feb 8, 2018

facebook-github-bot added the CLA Signed label Feb 8, 2018

bradfordlemley mentioned this pull request Feb 8, 2018

Upgrade dotenv-expand to 4.2.0 (#3961) #3992

Merged

On eject, choose to run yarn instead of npm if yarn is available.

869413f

gaearon reviewed Feb 9, 2018

View reviewed changes

Move monorepo to react-dev-utils.

c792a98

bradfordlemley changed the title ~~Fix for e2e-monorepos failure.~~ Use yarn when running inside yarn workspace. Feb 9, 2018

Fix lint.

a34a610

gaearon reviewed Feb 9, 2018

View reviewed changes

bradfordlemley added 2 commits February 9, 2018 10:59

Rename monorepo to workspaceUtils.

4817651

Add react-dev-utils dep for create-react-app.

c664c11

gaearon reviewed Feb 9, 2018

View reviewed changes

bradfordlemley mentioned this pull request Feb 10, 2018

Add support for new yarn workspaces config format #4001

Merged

getMonorepo -> findMonorepo

41fb5b0

gaearon approved these changes Feb 10, 2018

View reviewed changes

gaearon added the tag: bug fix label Feb 10, 2018

gaearon added this to the 2.0.0 milestone Feb 10, 2018

gaearon merged commit 2c34d5b into facebook:next Feb 10, 2018

tvalentius added a commit to tvalentius/create-react-app that referenced this pull request Feb 10, 2018

Merge pull request #6 from facebook/next

b7f2d96

Use yarn when running inside yarn workspace. (facebook#3997)

Timer mentioned this pull request Sep 18, 2018

Completion roadmap for react-scripts@2.0 #5024

Closed

Timer added a commit to Timer/create-react-app that referenced this pull request Sep 18, 2018

Revert "Use yarn when running inside yarn workspace. (facebook#3997)"

cc328d6

This reverts commit 2c34d5b.

zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Sep 30, 2018

Revert "Use yarn when running inside yarn workspace. (facebook#3997)"

0c508a8

This reverts commit 2c34d5b.

bradfordlemley added a commit to bradfordlemley/create-react-app that referenced this pull request Oct 11, 2018

Use yarn when running in monorepo, facebook#3997

ae9ffa4

bradfordlemley added a commit to bradfordlemley/create-react-app that referenced this pull request Nov 1, 2018

Use yarn when running in monorepo, facebook#3997

2322d0e

lock bot locked and limited conversation to collaborators Jan 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use yarn when running inside yarn workspace. #3997

Use yarn when running inside yarn workspace. #3997

bradfordlemley commented Feb 8, 2018 •

edited

Loading

Timer commented Feb 9, 2018

gaearon Feb 9, 2018

bradfordlemley Feb 9, 2018 •

edited

Loading

gaearon Feb 9, 2018

bradfordlemley Feb 9, 2018

gaearon Feb 9, 2018

gaearon Feb 9, 2018

bradfordlemley Feb 9, 2018

gaearon Feb 9, 2018 •

edited

Loading

bradfordlemley Feb 9, 2018

gaearon Feb 9, 2018

gaearon Feb 9, 2018

gaearon Feb 9, 2018

gaearon commented Feb 10, 2018

Use yarn when running inside yarn workspace. #3997

Use yarn when running inside yarn workspace. #3997

Conversation

bradfordlemley commented Feb 8, 2018 • edited Loading

Timer commented Feb 9, 2018

Choose a reason for hiding this comment

bradfordlemley Feb 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaearon Feb 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaearon commented Feb 10, 2018

bradfordlemley commented Feb 8, 2018 •

edited

Loading

bradfordlemley Feb 9, 2018 •

edited

Loading

gaearon Feb 9, 2018 •

edited

Loading