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
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Clean up, use file matching (similar to original) in webpack configs …
…instead of matching function.
bradfordlemley committed Jan 22, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit d9f5cf77e8e3bc312d19057fda90d9de3067be50
46 changes: 25 additions & 21 deletions packages/react-scripts/config/paths.js
Original file line number Diff line number Diff line change
@@ -92,7 +92,6 @@ module.exports = {
if (
fs.existsSync(path.join(appDirectory, 'packages', 'react-scripts', 'config'))
) {
const appPackageJson = require(resolveOwn('package.json'));
module.exports = {
dotenv: resolveOwn('template/.env'),
appPath: resolveApp('.'),
@@ -114,25 +113,6 @@ if (
}
// @remove-on-eject-end

const isInPath = (fpath, searchPaths) => {
for (let i = 0; i < searchPaths.length; i++) {
if (fpath.startsWith(searchPaths[i] + path.sep)) {
return true;
}
}
return false;
};

const hasNodeModules = /[/\\\\]node_modules[/\\\\]/;

// treat as source if realpath is in a srcPath, but not in a node_modules dir
const isSrcFile = srcFile => {
const realSrc = fs.realpathSync(srcFile);
return (
!realSrc.match(hasNodeModules) && isInPath(realSrc, module.exports.srcPaths)
);
};

module.exports.srcPaths = [module.exports.appSrc];

// if app is in a monorepo (lerna or yarn workspace), allow any module inside
@@ -153,5 +133,29 @@ if (monoPkgPath) {
}
}

module.exports.shouldLint = modPath => isSrcFile(modPath);
//
// BELOW IS TO SUPPORT JEST TRANSPILING SOURCE FOR MONOREPOS
// * JEST DOESN'T MATCH ON REALPATHS, BUT DOES WITH JEST >= 22.0.?, SO
// THIS CAN BE REMOVED WITH JEST > 22.0.?
//

const isInPath = (fpath, searchPaths) => {
for (let i = 0; i < searchPaths.length; i++) {
if (fpath.startsWith(searchPaths[i] + path.sep)) {
return true;
}
}
return false;
};

const hasNodeModules = /[/\\\\]node_modules[/\\\\]/;

// realpath is in a srcPaths, but not in a node_modules dir
const isSrcFile = srcFile => {
const realSrc = fs.realpathSync(srcFile);
return (
!realSrc.match(hasNodeModules) && isInPath(realSrc, module.exports.srcPaths)
);
};

module.exports.shouldTranspile = modPath => isSrcFile(modPath);
6 changes: 4 additions & 2 deletions packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
@@ -156,7 +156,8 @@ module.exports = {
loader: require.resolve('eslint-loader'),
},
],
include: paths.shouldLint,
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
@@ -178,7 +179,8 @@ module.exports = {
// The preset includes JSX, Flow, and some ESnext features.
{
test: /\.(js|jsx|mjs)$/,
include: paths.shouldTranspile,
include: paths.srcPaths,
exclude: [/[/\\\\]node_modules[/\\\\]/],
use: [
// This loader parallelizes code compilation, it is optional but
// improves compile time on larger projects
6 changes: 4 additions & 2 deletions packages/react-scripts/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
@@ -165,7 +165,8 @@ module.exports = {
loader: require.resolve('eslint-loader'),
},
],
include: paths.shouldLint,
include: paths.srcPaths,
exclude: [/[/\\\\]node_modules[/\\\\]/],
},
{
// "oneOf" will traverse all following loaders until one will
@@ -186,7 +187,8 @@ module.exports = {
// The preset includes JSX, Flow, and some ESnext features.
{
test: /\.(js|jsx|mjs)$/,
include: paths.shouldTranspile,
include: paths.srcPaths,
exclude: [/[/\\\\]node_modules[/\\\\]/],
use: [
// This loader parallelizes code compilation, it is optional but
// improves compile time on larger projects
7 changes: 3 additions & 4 deletions packages/react-scripts/scripts/utils/createJestConfig.js
Original file line number Diff line number Diff line change
@@ -47,10 +47,9 @@ module.exports = (resolve, rootDir, srcRoots, isEjecting) => {
'config/jest/fileTransform.js'
),
},
// pattern matching doesn't work for workspaces (symlinks from node_modules)
// because jest doesn't match against realpaths
// ...so decision to tranpile is inside babelTransform which can use realpaths
// for matching
// jest doesn't match against realpaths, so pattern matching doesn't work
// for monorepos with symlinks from node_modules to module source
// moved pattern matching into babelTransform for now
// jest 22.0.x does match patterns against realpath so this can probably
// change with jest 22.0.x+
transformIgnorePatterns: [