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.

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

Merged
merged 29 commits into from
Feb 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
bf2d814
Support for multiple source paths via package.json srcPaths entry.
Nov 9, 2017
d9f5cf7
Clean up, use file matching (similar to original) in webpack configs …
bradfordlemley Jan 10, 2018
12e76ad
Remove package lock files.
bradfordlemley Jan 11, 2018
bd1124d
Fix for eject.
bradfordlemley Jan 11, 2018
eae24e9
Filter tests to run only tests in monorepo components included by the…
bradfordlemley Jan 12, 2018
f4b3a0d
Fix conditions for when to use template.
bradfordlemley Jan 12, 2018
739b59b
Fix eject.
bradfordlemley Jan 12, 2018
7227e5d
Remove code that is not needed w/ Jest 22.
bradfordlemley Jan 13, 2018
a93573d
Include all cra-comp tests in monorepo instead of trying to include o…
bradfordlemley Jan 13, 2018
8102907
Pin find-pkg version.
bradfordlemley Jan 14, 2018
9a1b92c
Hopefully fix jest test file matching on windows by removing first sl…
bradfordlemley Jan 14, 2018
f4f2882
E2E tests for monorepo.
bradfordlemley Jan 17, 2018
d8e0319
Run monorepo tests in CI.
bradfordlemley Jan 18, 2018
f96d04c
Fix and test post-eject build.
bradfordlemley Jan 18, 2018
565c1d7
Fix e2e test.
bradfordlemley Jan 18, 2018
75ae0c5
Fix test suite names in appveyor.
bradfordlemley Jan 18, 2018
fbc6bde
Include individual package dirs as srcPaths instead of top-level mono…
bradfordlemley Jan 18, 2018
21f0b00
Fix running tests after eject.
bradfordlemley Jan 18, 2018
9feda8b
Clean up test workspace, add some verifcations.
bradfordlemley Jan 19, 2018
8ab33c7
Cleanup.
bradfordlemley Jan 19, 2018
d52e904
Try to fix hang when running test on appveyor.
bradfordlemley Jan 20, 2018
79ac815
Don't write babel or lint config to package.json when ejecting.
bradfordlemley Jan 21, 2018
3e81144
Incorporate review comments.
bradfordlemley Jan 22, 2018
ec6f5a8
Fixes for windows.
bradfordlemley Jan 22, 2018
978674f
Fix for kitchensink mocha tests compiling.
bradfordlemley Jan 22, 2018
fc9b890
Add lerna monorepo test.
bradfordlemley Jan 23, 2018
4d0bc68
Fix lerna bootstrap on windows.
bradfordlemley Jan 23, 2018
9e7490c
Incorporate more review comments:
bradfordlemley Jan 25, 2018
50b4666
Add monorepo info to user guide.
bradfordlemley Jan 30, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ script:
- 'if [ $TEST_SUITE = "installs" ]; then tasks/e2e-installs.sh; fi'
- 'if [ $TEST_SUITE = "kitchensink" ]; then tasks/e2e-kitchensink.sh; fi'
- 'if [ $TEST_SUITE = "old-node" ]; then tasks/e2e-old-node.sh; fi'
- 'if [ $TEST_SUITE = "monorepos" ]; then tasks/e2e-monorepos.sh; fi'
env:
matrix:
- TEST_SUITE=simple
- TEST_SUITE=installs
- TEST_SUITE=kitchensink
- TEST_SUITE=monorepos
matrix:
include:
- node_js: 0.10
Expand Down
5 changes: 4 additions & 1 deletion appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@ environment:
test_suite: "installs"
- nodejs_version: 8
test_suite: "kitchensink"
- nodejs_version: 8
test_suite: "monorepos"
- nodejs_version: 6
test_suite: "simple"
- nodejs_version: 6
test_suite: "installs"
- nodejs_version: 6
test_suite: "kitchensink"

- nodejs_version: 6
test_suite: "monorepos"
cache:
- node_modules -> appveyor.cleanup-cache.txt
- packages\react-scripts\node_modules -> appveyor.cleanup-cache.txt
Expand Down
5 changes: 4 additions & 1 deletion packages/react-scripts/config/jest/babelTransform.js
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,
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

// @remove-on-eject-end
});
59 changes: 48 additions & 11 deletions packages/react-scripts/config/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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('.'),
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we merge useTemplate and checkForMonoRepo together

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the usetemplate assignment to the checkForMonoRepo
and use checkForMonoRepo for that.

Copy link
Contributor

Choose a reason for hiding this comment

The 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());
}
12 changes: 7 additions & 5 deletions packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -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[/\\\\]/],
Copy link
Contributor

Choose a reason for hiding this comment

The 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 node_modules and we want to skip those?

Copy link
Contributor Author

@bradfordlemley bradfordlemley Jan 21, 2018

Choose a reason for hiding this comment

The 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.)

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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,
},
},
Expand Down
12 changes: 7 additions & 5 deletions packages/react-scripts/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,21 @@ module.exports = {
options: {
formatter: eslintFormatter,
eslintPath: require.resolve('eslint'),
// @remove-on-eject-begin
// TODO: consider separate config for production,
// e.g. to enable no-console and no-debugger only in production.
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[/\\\\]/],
},
{
// "oneOf" will traverse all following loaders until one will
Expand All @@ -186,7 +187,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
Expand All @@ -196,8 +198,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')],
compact: true,
highlightCode: true,
},
Expand Down Expand Up @@ -317,8 +319,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,
},
},
Expand Down
3 changes: 3 additions & 0 deletions packages/react-scripts/fixtures/kitchensink/.babelrc
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"
}
}
11 changes: 11 additions & 0 deletions packages/react-scripts/fixtures/monorepos/packages/comp2/index.js
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"
]
}
}
Binary file not shown.
Loading