-
-
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
Add support for yarn and lerna monorepos. #3741
Changes from all commits
bf2d814
d9f5cf7
12e76ad
bd1124d
eae24e9
f4b3a0d
739b59b
7227e5d
a93573d
8102907
9a1b92c
f4f2882
d8e0319
f96d04c
565c1d7
75ae0c5
fbc6bde
21f0b00
9feda8b
8ab33c7
d52e904
79ac815
3e81144
ec6f5a8
978674f
fc9b890
4d0bc68
9e7490c
50b4666
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,18 @@ | ||
// @remove-file-on-eject | ||
// @remove-on-eject-begin | ||
/** | ||
* Copyright (c) 2014-present, Facebook, Inc. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
// @remove-on-eject-end | ||
'use strict'; | ||
|
||
const babelJest = require('babel-jest'); | ||
|
||
module.exports = babelJest.createTransformer({ | ||
presets: [require.resolve('babel-preset-react-app')], | ||
// @remove-on-eject-begin | ||
babelrc: false, | ||
// @remove-on-eject-end | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,8 @@ | |
const path = require('path'); | ||
const fs = require('fs'); | ||
const url = require('url'); | ||
const findPkg = require('find-pkg'); | ||
const globby = require('globby'); | ||
|
||
// Make sure any symlinks in the project folder are resolved: | ||
// https://github.com/facebookincubator/create-react-app/issues/637 | ||
|
@@ -63,6 +65,8 @@ module.exports = { | |
servedPath: getServedPath(resolveApp('package.json')), | ||
}; | ||
|
||
let checkForMonorepo = true; | ||
|
||
// @remove-on-eject-begin | ||
const resolveOwn = relativePath => path.resolve(__dirname, '..', relativePath); | ||
|
||
|
@@ -86,17 +90,13 @@ module.exports = { | |
ownNodeModules: resolveOwn('node_modules'), // This is empty on npm 3 | ||
}; | ||
|
||
const ownPackageJson = require('../package.json'); | ||
const reactScriptsPath = resolveApp(`node_modules/${ownPackageJson.name}`); | ||
const reactScriptsLinked = | ||
fs.existsSync(reactScriptsPath) && | ||
fs.lstatSync(reactScriptsPath).isSymbolicLink(); | ||
|
||
// config before publish: we're in ./packages/react-scripts/config/ | ||
if ( | ||
!reactScriptsLinked && | ||
__dirname.indexOf(path.join('packages', 'react-scripts', 'config')) !== -1 | ||
) { | ||
// detect if template should be used, ie. when cwd is react-scripts itself | ||
const useTemplate = | ||
appDirectory === fs.realpathSync(path.join(__dirname, '..')); | ||
|
||
checkForMonorepo = !useTemplate; | ||
|
||
if (useTemplate) { | ||
module.exports = { | ||
dotenv: resolveOwn('template/.env'), | ||
appPath: resolveApp('.'), | ||
|
@@ -117,3 +117,40 @@ if ( | |
}; | ||
} | ||
// @remove-on-eject-end | ||
|
||
module.exports.srcPaths = [module.exports.appSrc]; | ||
|
||
const findPkgs = (rootPath, globPatterns) => { | ||
const globOpts = { | ||
cwd: rootPath, | ||
strict: true, | ||
absolute: true, | ||
}; | ||
return globPatterns | ||
.reduce( | ||
(pkgs, pattern) => | ||
pkgs.concat(globby.sync(path.join(pattern, 'package.json'), globOpts)), | ||
[] | ||
) | ||
.map(f => path.dirname(path.normalize(f))); | ||
}; | ||
|
||
const getMonorepoPkgPaths = () => { | ||
const monoPkgPath = findPkg.sync(path.resolve(appDirectory, '..')); | ||
if (monoPkgPath) { | ||
// get monorepo config from yarn workspace | ||
const pkgPatterns = require(monoPkgPath).workspaces; | ||
const pkgPaths = findPkgs(path.dirname(monoPkgPath), pkgPatterns); | ||
// only include monorepo pkgs if app itself is included in monorepo | ||
if (pkgPaths.indexOf(appDirectory) !== -1) { | ||
return pkgPaths.filter(f => fs.realpathSync(f) !== appDirectory); | ||
} | ||
} | ||
return []; | ||
}; | ||
|
||
if (checkForMonorepo) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The useTemplate code is removed when ejecting. I'm sure there are other ways to do this logic, but not sure there is a clearly cleaner way ... if you have a way you think is clearly cleaner, send it in, I'll take it :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. change the variable name as needed. |
||
// if app is in a monorepo (lerna or yarn workspace), treat other packages in | ||
// the monorepo as if they are app source | ||
Array.prototype.push.apply(module.exports.srcPaths, getMonorepoPkgPaths()); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -145,18 +145,19 @@ module.exports = { | |
options: { | ||
formatter: eslintFormatter, | ||
eslintPath: require.resolve('eslint'), | ||
// @remove-on-eject-begin | ||
baseConfig: { | ||
extends: [require.resolve('eslint-config-react-app')], | ||
}, | ||
// @remove-on-eject-begin | ||
ignore: false, | ||
useEslintrc: false, | ||
// @remove-on-eject-end | ||
}, | ||
loader: require.resolve('eslint-loader'), | ||
}, | ||
], | ||
include: paths.appSrc, | ||
include: paths.srcPaths, | ||
exclude: [/[/\\\\]node_modules[/\\\\]/], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please explain why this has become necessary? Is this because non-CRA packages have their source code at the same level of nesting as their own There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, since srcPaths includes comp1 root (or or top-level monorepo root, depending on decision above), both comp1/file.js and comp1/node_modules/somedep/somefile.js get included in the include filter, so we need to filter out node_modules. (Originally, only app/src was included, so there was no need to filter out app/node_modules.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Am not sure this is getting exclufed. I can still find node_modules inside the comp1 that is inside app3. Doesnt it completely exclude nodemodules? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These webpack excludes just exclude them from getting transpiled and linted. |
||
}, | ||
{ | ||
// "oneOf" will traverse all following loaders until one will | ||
|
@@ -178,7 +179,8 @@ module.exports = { | |
// The preset includes JSX, Flow, and some ESnext features. | ||
{ | ||
test: /\.(js|jsx|mjs)$/, | ||
include: paths.appSrc, | ||
include: paths.srcPaths, | ||
exclude: [/[/\\\\]node_modules[/\\\\]/], | ||
use: [ | ||
// This loader parallelizes code compilation, it is optional but | ||
// improves compile time on larger projects | ||
|
@@ -188,8 +190,8 @@ module.exports = { | |
options: { | ||
// @remove-on-eject-begin | ||
babelrc: false, | ||
presets: [require.resolve('babel-preset-react-app')], | ||
// @remove-on-eject-end | ||
presets: [require.resolve('babel-preset-react-app')], | ||
// This is a feature of `babel-loader` for webpack (not Babel itself). | ||
// It enables caching results in ./node_modules/.cache/babel-loader/ | ||
// directory for faster rebuilds. | ||
|
@@ -275,8 +277,8 @@ module.exports = { | |
options: { | ||
// @remove-on-eject-begin | ||
babelrc: false, | ||
presets: [require.resolve('babel-preset-react-app')], | ||
// @remove-on-eject-end | ||
presets: [require.resolve('babel-preset-react-app')], | ||
cacheDirectory: true, | ||
}, | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
{ | ||
"presets": ["react-app"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import React from 'react'; | ||
|
||
const Comp1 = () => <div>Comp1</div>; | ||
|
||
export default Comp1; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import React from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
import Comp1 from '.'; | ||
|
||
it('renders Comp1 without crashing', () => { | ||
const div = document.createElement('div'); | ||
ReactDOM.render(<Comp1 />, div); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"name": "comp1", | ||
"version": "1.0.0", | ||
"main": "index.js", | ||
"license": "MIT", | ||
"devDependencies": { | ||
"react": "^16.2.0", | ||
"react-dom": "^16.2.0" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import React from 'react'; | ||
|
||
import Comp1 from 'comp1'; | ||
|
||
const Comp2 = () => ( | ||
<div> | ||
Comp2, nested Comp1: <Comp1 /> | ||
</div> | ||
); | ||
|
||
export default Comp2; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import React from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
import Comp2 from '.'; | ||
|
||
it('renders Comp2 without crashing', () => { | ||
const div = document.createElement('div'); | ||
ReactDOM.render(<Comp2 />, div); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
{ | ||
"name": "comp2", | ||
"dependencies": { | ||
"comp1": "^1.0.0" | ||
}, | ||
"devDependencies": { | ||
"react": "^16.2.0", | ||
"react-dom": "^16.2.0" | ||
}, | ||
"version": "1.0.0", | ||
"main": "index.js", | ||
"license": "MIT" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# See https://help.github.com/ignore-files/ for more about ignoring files. | ||
|
||
# dependencies | ||
/node_modules | ||
|
||
# testing | ||
/coverage | ||
|
||
# production | ||
/build | ||
|
||
# misc | ||
.DS_Store | ||
.env.local | ||
.env.development.local | ||
.env.test.local | ||
.env.production.local | ||
|
||
npm-debug.log* | ||
yarn-debug.log* | ||
yarn-error.log* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
{ | ||
"name": "cra-app1", | ||
"version": "0.1.0", | ||
"private": true, | ||
"dependencies": { | ||
"comp2": "^1.0.0", | ||
"react": "^16.2.0", | ||
"react-dom": "^16.2.0" | ||
}, | ||
"devDependencies": { | ||
"react-scripts": "latest" | ||
}, | ||
"scripts": { | ||
"start": "react-scripts start", | ||
"build": "react-scripts build", | ||
"test": "react-scripts test --env=jsdom", | ||
"eject": "react-scripts eject" | ||
}, | ||
"browserslist": { | ||
"development": [ | ||
"last 2 chrome versions", | ||
"last 2 firefox versions", | ||
"last 2 edge versions" | ||
], | ||
"production": [ | ||
">1%", | ||
"last 4 versions", | ||
"Firefox ESR", | ||
"not ie < 11" | ||
] | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still create babelrc on eject? If we do I think it might be confusing that removing
babel-preset-react-app
from it won't actually remove it (because it's also inlined here and in webpack config).Maybe we don't need to create babelrc on eject then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, babel config still gets written to package.json on eject. Yes, it could be confusing, and probably should not be there. Will remove it. Same thing for eslint config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FTR, removing babel config from package.json impacted mocha tests that run in e2e-kitchensink. Babelrc was already being created in the e2e script itself, but only for tests before eject. I opted to fix this by adding .babelrc to the fixture and removing the part of e2e script that was creating it.