Add support for yarn and lerna monorepos.

[bradfordlemley]

This adds support for lerna and yarn workspace monorepos, #1333.

There are some open questions as to how monorepos should work, so this is just a starting point.

Testing was performed with example and instructions at: https://github.com/bradfordlemley/cra-monorepo-examples

(Note: this PR pulls in changes from #3435 which is for supporting running create-react-app in a monorepo space.)

[gaearon]

Sorry, had to cancel the CI build because I'm working on fixing some test flakiness and need very quick feedback loop. Please remind me to restart the CI tomorrow (or push another commit in a few hours).

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[bradfordlemley]

Just realized that Issue 3031/PR 3435 isn't actually a prerequisite for this PR, so I've removed PR 3435 changes here which cleans up this PR considerably to focus it on Issue 1333.

Notes:
Issue 3031 deals with running create-react-app inside lerna/yarn workspace.
Issue 1333 deals with supporting shared source inside lerna/yarn workspace.

[gaearon]

Sorry, had to cancel CI build again, please try in an hour or so

[gaearon]

Please resubmit this against the next branch and try to remove Jest-specific things that have been resolved in 22

[gaearon]

I'd prefer to remove this (and rely on Jest 22 behavior). After ejecting, the project is already complex enough. We should try to reduce the area for complexity that the user will have to maintain directly.

[gaearon]

Please verify this?

[gaearon]

Same

[gaearon]

Please pin the version.

[gaearon]

Can you explain more about why this block is necessary, what it does, and what happens on eject?

[bradfordlemley]

toRelRootDir itself converts a path to be relative to the jest string-literal <rootDir> thing, producing strings like <rootDir>/src and <rootDir>/../../cra-comps. These are what end up in the ejected jest config and allow that static ejected jest config to work.

Much of the code below that is to include only tests from cra-comps in the monorepo that the app actually references via dependencies, rather than including tests from all cra-comps in monorepo. That was mostly a proof-of-concept -- my latest thinking is that is not the desired functionality, so that code should be removed. I'll start some more discussion on the desired functionality in the main comments area of this PR. I'll just remove the functionality/code for now and can re-add later if it's decided that functionality is desired, and I suggest we re-visit the code at that time.

[gaearon]

Sounds great!

[gaearon]

Does this need updating now?

outdated

[gaearon]

We’re going to need integration tests around this. I don’t know what’s the best place to put them yet. Maybe worth setting up a separate end to end suite similar to the ones we already have.

[bradfordlemley]

Was just about to ask where would be a good place to add tests for this -- a new e2e suite sounds like a good idea -- I'll start there.

Also, I think it'd be helpful to have some info on source sharing in the user guide, so planning on adding something there.

[bradfordlemley]

Open questions on desired functionality. Was hoping to generate some discussion in #1333 (comment), but hasn't gotten action so far, so I'll re-post those questions here, along with my preferences:

1. Which (if any) tests from cra-comps should run when running app tests?
   • Preference: Default to include tests from ALL cra-comps in the monorepo and let projects apply filters as-desired via jest command line, e.g. "yarn test -- cra-app1 comp1" in package.json scripts. This seems to allow the most flexibility and I think it will be really useful to many devs to be able to easily run ALL tests.
   • Other option 1: Only include tests from cra-comps that the app actually included as dependencies. CRA implementation would probably have to make assumptions about how the monorepo handles components (e.g. symlink from node_modules). Also, to be correct, should probably recurse (include dependencies of dependencies). So, harder and fragiler.
2. Should files from cra-comps be linted when building app?
   • Preference: yes, lint
3. Should a directory structure be enforced for cra-comps? (e.g. component source files must be under /src)
   • Preference: no enforcement
4. How to flag components that should be treated as cra-comps?
   • Preference: By convention, anything under cra-comps (name??) subdir of monorepo root should be treated like cra source. This seems like the most conservative approach. Additional flags/options to mark cra source could be added later, with more discussion.
   • Other option 1: Allow project to configure/specify which paths should be treated as cra-comps. Easily doable (??), but requires config.
   • Other option 2: Flag each component somehow, e.g. in its package.json. Probably would need significant discussion. Implementation for file matching would (I think) become functional instead of pattern-based, which (I think) is supported by webpack, but (I think) would need support added in jest and it might have a significant impact on eject functionality.

Would be really good to get input on these questions and generally have more feedback on how this should work. It's obvious many folks are wanting to share source in their cra apps ... maybe the initial implementation just needs to get out there, and then the feedback will come ?? :)

[gaearon]

How about we start with just supporting packages as the folder name (possibly throwing if we encounter other settings), and always treating those as needing test/lint/compilation? Once we get that out we can seek feedback about other how other workflows fit into this.

[bradfordlemley]

How about we start with just supporting packages as the folder name (possibly throwing if we encounter other settings), and always treating those as needing test/lint/compilation?

The initial feature probably should account for monorepos that use some non-CRA JS. Transpiling stuff that doesn't really need to be transpiled might not be a huge problem, but linting and testing could be issues: linting files that don't meet CRA lint, and picking up test files that match CRA file patterns, but aren't jest tests.

Also, I'm not a fan of using 'packages' top-level -- to me it's a meaningless extra path I have to drill into to get to what I really want to see. So would prefer not to require that, but not something I'll quibble over.

[gaearon]

Also, I'm not a fan of using 'packages' top-level -- to me it's a meaningless extra path I have to drill into to get to what I really want to see. So would prefer not to require that, but not something I'll quibble over.

We can still start with that and later relax the requirement. It seems like there's many unknowns and it's easier to start stricter to learn about them.

The initial feature probably should account for monorepos that use some non-CRA JS. Transpiling stuff that doesn't really need to be transpiled might not be a huge problem, but linting and testing could be issues: linting files that don't meet CRA lint, and picking up test files that match CRA file patterns, but aren't jest tests.

Would a single monorepo always fall on one side or the other, or is it likely we'll have a mix? What are some common use cases for wanting to share lint/test/build pipeline, or not share it?

[gaearon]

If some code doesn't match CRA specs, can it be outside of the CRA monorepo? e.g. if we support CRA being in a monorepo.

[bradfordlemley]

I have a monorepo that has both backend node services and frontend CRA apps, mainly because the backend and apps share some JS, but also just for managing the project. There's other stuff in the repo.

"But, sir, I don't want to eject, fork, or go on the cart!"

[gaearon]

Fair enough. I think you have enough context to do good decisions here. How do you feel about writing up a more complete proposal where you make some opinionated decisions about how it should work, taking the existing project philosophy into consideration?

[bradfordlemley]

I think the picture below describes the complete proposal.
Hope that's sufficient, let me know if you had some other format/mechanism in mind.

monorepo
  |--packages.json: workspaces: ["apps/*", "backend/*", "comps/*", "cra-comps/*"] <-- (config yarn workspace)
  |--apps  <-- nothing special about this dir
    |--cra-app1 <-- basic cra-app, doesn't use any monorepo comps
      |--package.json:
        devDependencies: [react-scripts]
        scripts.test: "react-scripts test --env=jsdom -- apps/cra-app1"  <-- apply filter to only run tests in this dir
    |--cra-app2 <-- cra-app uses some monorepo comps
      |--package.json:
        dependencies: [comp1, comp2], devDependencies: [react-scripts]
        scripts.test: "react-scripts test --env=jsdom"  <-- standard test cmd, will include all tests in cra-comps, but can be filtered with awesome jest user interface
      |--src
        |--file.js: import comp1 from 'comp1'  <-- yay, linted and transpiled comp1!
  |--cra-comps  <-- !!special!!, any cra build from anywhere in the monorepo will automatically transpile/lint/test anything in here
    |--comp1
      |--package.json: dependencies: [lodash]
      |--comp1.js
      |--comp1.test.js  <-- included in tests
    |--comp2 <-- cra comp with dependency on other cra-comp
      |--package.json: dependencies: [comp1]
      |--comp2.js: import comp1 from 'comp1'
      |--comp2.test.js  <-- included in tests
  |--backend  <-- nothing special about this dir
    |--node_svc1  <-- maybe has its own way to build comp1?
      |--package.json: dependencies: [comp1, comp3]
  |--comps  <-- nothing special about this dir
    |--comp3
      |--index.js  <-- not transpiled
      |--index.test.js <-- not picked up by cra tests
    |--comp4
      |--package.json: main: build/index.js
      |--build
        |--index.js <-- not transpiled
      |--index.js  <-- not transpiled
      |--index.test.js <-- not picked up by cra tests

[gaearon]

In your proposal, which package.json files explicitly define a dependency on react-scripts?

[bradfordlemley]

In your proposal, which package.json files explicitly define a dependency on react-scripts?

Only the cra-apps.

[marcinczenko]

From the documentation:

However, when you use react-scripts for an app in a monorepo, all packages in the monorepo are treated as app sources -- they are watched, linted, transpiled, and tested in the same way as if they were part of the app itself.

This all works pretty well, except for one thing. Transpiling what already is transpiled is bad idea - the linter breaks because it is not supposed to lint transpiled packages.

[bradfordlemley]

Transpiling what already is transpiled is bad idea - the linter breaks because it is not supposed to lint transpiled packages.

The #4092 proposal should solve this problem.

Related issue: #4249.

[marcinczenko]

Thanks for pointing this out. Has potential. I will also play a bit with this on the forked repo.

I was thinking that the first very small step would be to just make sure that only private packages are added to path.srcPaths. But I agree that sourceDependencies would add more control.

I also I like your comment from parcel/pull#1101:

1. default to use the consumer's build if the source package doesn't have .babelrc,
2. in addition support the same sourceDependencies.

I still hesitate though for point (1): is it better to look for .babelrc or to check if pkg.private === true or even to check if either pkg.module or pkg.main can be resolved and if so, do not transpile. Then you do not have to deal with cases when .babelrc is present, but additionally the package is included in consumer app's sourceDependencies - checking pkg.module or pkg.main feels more natural.

Actually our path to realise there is a problem was yet slightly different. We were trying to run all the jest tests in the mono repo from the root. So most of our packages are valid packages that are meant to be published at some point so they do have properly resolving pkg.module or pkg.main and of course it has its own .babelrc. Next to that we have the "apps" that will never be published as npm package. First problem was with hoisting. We first used the nohoist yarn workspace setting for the "apps" to get them going, but that was not ideal. Then we found react-scripts@2.0 and that worked way better. The fact that I can enter the workspace with the consumer app and run tests not only for the app itself but also for other packages did not feel that bad but it would be better if we can call react-scripts test --env=jsdom from the top level rather than from the workspace containing the consumer app. Unfortunately yarn start breaks everything and to have it working we needed to point pkg.module to the actual source files (so not transpiled === wrong). Using pkg.module and then transpile feels like breaking the law.

Lastly, having something like pkg.source proposed in parcel/pull#1101 is very intriguing, but I am afraid that it all quickly becomes very complex.

In my current state of mind, I would like the most if we could run the tests from the root using just jest and our own .babelrc configuration in every package and consumer app. Only when you enter the consumer app workspace and use react-scripts test --env=jsdom directly, then it should behave as a single repo create-react-app application (so ignoring .babelrc even if present) and all dependencies should be treated as regular npm dependencies (so no extra intelligence/logic). To get this it seem to be enough to get rid of the following from create-react-app/packages/react-scripts/config/paths.js:

if (mono.isAppIncluded) {
  Array.prototype.push.apply(module.exports.srcPaths, mono.pkgs);
}

You can check it here: https://github.com/marcinczenko/create-react-app/commits/mono
and you can try it by add "@react-frontend-developer/react-scripts": "2.0.0-next.b2fd8db8.11" to devDependencies.

In this approach I am still not sure how to get the packages automatically rebuilding when their sources change - maybe this logic should belong to yarn workspaces.

Sorry if something turns out to be inconsistent - I am still searching myself what would be the most reasonable approach.

[marcinczenko]

After playing around with react-scripts@2 in monorepo, I am more and more convinced that we should be very careful with the things like pkg.sourceDependencies, and pkg.source. In any case, they should be treated as optional and by default create-react-app should not add any extra semantics to what is the monorepo. It is simply not its job. react-scripts should play nice with monorepo (so it should not break it and one should not be forced to use nohoist), but beyond that it should not, by default, try to interfere with any other workspaces in any way.

We use now the fork I mentioned in my previous comment (https://github.com/marcinczenko/create-react-app/commits/mono) and we are really happy with it. We can run all the tests in the monorepo from the root (including the tests of the apps created with create-react-app - we just need to add our own .babalrc), but we can also enter the workspace where the app created with create-react-app is and do yarn test there and have the standard create-react-app semantics limited to that and only that workspace.

[bradfordlemley]

@marcinczenko I think the problem that you're experiencing with the current 2.0 implementation is that there is no way to opt monorepo packages out -- if your CRA app includes a package from the monorepo, it will be transpiled and linted. I think #4092 should solve that issue because it requires you to opt packages in by listing them in your app package's sourceDependencies. I think your 2.0 fork just disables CRA monorepo functionality, right? ... the effect should be the same when #4092 is implemented, as long as your app doesn't list any sourceDependencies. But, if there will still be a problem after #4092 is implemented, can you point it out so it can be addressed?

Additionally...can you explain a bit more about how your solution is working to help others understand other ways to handle a monorepo? Eg, when you're working on your app and other packages in the monorepo at the same time, how do you rebuild other packages in the monorepo? Do you have a top-level script that kicks off watchers/builders for each packages?

[marcinczenko]

@bradfordlemley You're right. If the "to be managed by react-scripts" workspaces are only those that are explicitly white-listed in sourceDependencies then indeed by default (i.e. when I do not use sourceDependencies) I will get the desired behaviour that I described in my previous comment.

Just for the matter of completeness, currently, to my best knowledge, as soon as you have a single workspace including react-scripts@2 and you run yarn test or yarn start from it, than all other workspaces are treated as sourceDependencies - and it does not matter if the "react-scripts" workspace includes any of them. I understand that this will not be case after sourceDependencies proposal is incorporated but I needed to have it off, otherwise we could not do anything (this is the reason for my fork).

For the automatic rebuilding of the dependent workspaces: we did not make our minds yet, but we are thinking about. Currently, we just manually rebuild using lerna with scopes, and because all our packages are scoped, then this is just one command for all of them. I was thinking this: yarn knows which workspaces are there. Let's say that, by default, yarn does not automatically rebuild anything, but a given workspace could have in its package.json something like this:

"workspaces": {
  "watch": ["sources"],
  "watchScript": "yarn build"
}

Or, maybe it calls build script by default, or maybe even we can enable watching of the given workspace interactively: yarn [workspace] (un)watch.

Even if we have this functionality in yarn, then react-scripts should still be able to watch changes in all the packages that are in the monorepo and it depends on, but not necessarily transpile and lint them all. The user should still have control.

Let's take an example. Imagine you have a the following source-tree:

workspaces/
  workspace1: create-react-app-based
    dependencies:
      - package1
      - package2
      - package3
    sourceDependencies:
      - package1
      - package2
  package1:
      .babelrc
      package.json
        - module: es/index.js
        - sources: src/index.js
  package2:
      .babelrc
      package.json
        - module: es/index.js
  package3:
      .babelrc
      package.json
        - module: es/index.js

So, above, package1 and package2 are sourceDependency. For both of them I would expect that react-scripts would update the dev-server if it is running when they change. For package1, the change means that any of its source files changed. For package2, the change means that it got rebuilt by yarn or by any external script (e.g. there is the change in the folder where the entry point pointed by pkg.module is). Then for package1, I would expect that @react-scripts will also lint and transpile every time the sources of package1 change. For this to work I need something like pkg.source so that react-scripts know what to watch and so that we can prevent transpiling and linting already transpiled sources (pointed to by pkg.module). package2 is also a sourceDependency so I still want that react-scripts is watching it but because its package.json does not have pkg.source I do not want react-scripts to lint and transpile.
Finally, package3 is not a sourceDependency thus, react-scripts should leave it alone :).

To summarize, I would expect the following semantics:

if package dependency belongs to sourceDependencies
    if `pkg.source` resolves in the package.json of the dependency then
        lint, transpile, etc
    else
        just watch it so that when it gets rebuild by yarn or external script, 
             the dev-server or jest will refresh
else
    do not touch that package in any way
end

Does #4092 already distinguish between these three cases? I am not sure about the syntax, but what I like about this semantics is that it gives you flexibility and control.

[bradfordlemley]

For the package2 case (has its own watch/build), it seems like it should "just work" (automatically update devserver) without having to specify it as source. I haven't tried that case, but I'm assuming it doesn't "just work" right now. I think it should be pretty straight-forward to make it work with a tweak to the devserver config. Do you see any problems with that proposal of making it "just work" without having to specify it as source?

[marcinczenko]

To my best knowledge, dev server will not be updated. If you rebuild your workspaces, then you have to manually restart developer server. I think there were many discussions whether react-scripts should observe node_modules or not. I would not mind if in case of monorepos, symlinked packages from the same monorepo that are listed as dependencies in the create-react-based consumer app, are observed by react-scripts ("just work"). Hmm...I think you are right. I double checked and it seems like it gets updated.

So it would also mean that we do not distinguish between package2 and package3. I think this is acceptable (if you do not want devserver to be update just do not build the package - or make yarn workspaces not watching it).

[alex-pex]

Currently CRA reads package.json "main" field as the entrypoint.

{
  "name": "my-package",
  "main": "src/index.js"
}

I'd prefer something like

{
  "name": "my-package",
  "source": "src/index.js",
  "main": "lib/index.js",
  "module": "lib/index.esm.js",
  "browser": "lib/index.umd.js"
}

It let me work on a package without using a babel watcher, but still compile the package before publishing.
Or am I missing something? How can I build and publish independant package using CRA?

[trompx]

@alex-pex are you able to use an already transpiled package in an CRA app?

I developp in a monorepo, some packages are really small so I import them in CRA app with: main: src/index.js

Now for some big packages, I wish to import them in my app already transpiled. But when I set in the imported package package.json file: main: dist/js/bundle.js I get Module not found: Can't resolve '@repo/mypackage'

Most of the time, I want to import packages not transpiled to benefit from tree shaking. But sometimes I wish to export it already transpiled so having an option to indicate which files to pull from package.json (either main or source as alex suggest woule be great).

[alex-pex]

@trompx I'm currently using lerna + nwb. I transpile every package using a babel watcher, and use them in my app like a regular package. I need this setup because the package are also used by a different team (working on a separate app), and I publish versions for them, on a private repository.

I didn't try but with CRA v1, but it should already work, except there is no automatic compilation.

The problem here is how the source entrypoint is declared: using "main" conflicts with regular package (main should point to a compiled es5 version). I propose "source", as microbundle does https://github.com/developit/microbundle#specifying-builds-in-packagejson