-
Notifications
You must be signed in to change notification settings - Fork 127
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
RFC: React Native as a monorepo #480
RFC: React Native as a monorepo #480
Conversation
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.
Thanks for sending this over @kelset
I've did a first pass and left some comments/nits
|
||
This proposal is not much as in introducing something completely new, but more to discuss a solution to an existing problem. To put simply, the current shape of the `react-native` codebase hosted in GitHub is lacking structure and consistency and this is leading to a number of issues (detailed below). | ||
|
||
This proposal is about re-shaping the existing codebase on GitHub into what is commonly expected of a monorepo in the JavaScript ecosystem (see, for example, [Babel.js](https://github.com/babel/babel) or [Next.js](https://github.com/vercel/next.js)): consistent versioning, naming and tooling. |
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.
I'd add Metro or React as well here
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.
I purposefully avoided adding Reactjs because it seems to suffer from some of the issues I list in the RFC - here's a quick example:
- https://github.com/facebook/react/blob/main/packages/react/package.json -> v18.1.0
- https://github.com/facebook/react/blob/main/packages/react-devtools-inline/package.json -> 4.26.6
- https://github.com/facebook/react/blob/main/packages/react-noop-renderer/package.json -> 16.0.0
- https://github.com/facebook/react/blob/main/packages/jest-mock-scheduler/package.json -> 0.1.0
- https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/package.json -> 4.5.0
For Metro, as far as I can see they seem to have addressed the naming and semver issues a the way fairly similar to the one we are proposing here (for example, see facebook/metro@fcff382) but they still falls under the category of "opaque repos" like RN (as far as I can see, it follows the same patterns as RN, like for when a PR needs to be merged: facebook/metro#819 ).
That said, I'll add a reference further down in the RFC to the fact that RN could probably learn from how Metro has been setup - only concern there is that it relies on Lerna (see https://github.com/facebook/metro/blob/94e79e0778c8b41224b54c24e48b01e9c53e426a/package.json#L33 ) and Lerna has been officially discontinued (nvm it looks like they are moving stewardship).
(EDIT: added)
Just as a supporting comment, my experience supporting the firebase community leads me to lean strongly to "locked versioning" for easy of user support / issue triage as it eliminates much of the versioning confusion - firebase ecosystem has transitioned fully to that style (if you'll allow that a gradle "bill of materials" fits the bill on android - since we use the BoM - though their components do have independent versioning underneath). It really helps a lot, so having "one true version" with automated release policy would be nice from my perspective. (https://invertase.io/blog/react-native-firebase-versioning) If some packages are higher velocity than others leading to faster versions and worries about release eng time, my counters are "more automation" and "versions are free". I recognize "more automation" isn't free, but within reason it should be possible, if faster revving packages are an issue. Not sure if there are other counterarguments but may well be, happy to learn if so, especially if they really are insurmountable and mean locked versioning is a non-starter in some cases |
One approach I was thinking about for this initial monorepo restructuring was a codemod script of some sort - so you don't have to keep rebasing a huge pull request keeping up with the main branch, you just need to maintain a script which restructures the repo/applies needed patches until it's decided the changeover is ready. There's a lot of internal stuff at Meta that would be effected by changing the structure of the RN repo too so this effort will need involvement from someone there. A codemod script approach would probably also be useful because once it's to the point where tests are passing on OSS with the new structure, someone at Meta could modify/add on to the script to change over things internally too. And hopefully it could be useful for helping people rebase their pull requests to the new structure when the change happens, too. |
That's a good point. I still think that the major challenge at this stage is letting this setup works with the internal infrastructure. Having one of your draft PR @empyrical imported internally as a first run could help us at least draft a list of the necessary pieces to make this happen. |
Following up on this RFC as I would like to see how we can actionate it.
If we manage to achieve the first two items here, we will already solve a lot of the versioning problems, plus we would not require manual publishing of first party packages anymore. Once we're done with it, we can look into:
This last item is also essentially blocked by facebook/metro#1 as we won't be able to run RN-Tester anymore otherwise. Do we feel the first two items are actionable in isolation? Or do we feel we can split this work further or differently? I just want to make sure this doesn't get stale as it looks too much of a monolithic work. |
I think we could start looking at #2 first, put it in place, then do #1. And yeah #3 can/should be tackled separately |
Yes the publishing (.2) could be done also first. The only gotchas there is make sure we don't re-publish older versions of packages which are already published (as they will fail to do so). Still is something we can split and do separately. Adding a couple of feedbacks/thoughts on the overall RFC:
|
If I may, I would like to suggest the same-ish set up as https://github.com/microsoft/rnx-kit. We've used many solutions in the past that we weren't happy with, but we have had zero complaints with Nx for monorepo management and Changesets for versioning so far.
How much storage do we get from GitHub? Could we store binary blobs here and download them on demand? |
GH Release would be a good solution. The problem is that we would like to have a solution that works also for nightly without having to publish a new Github Release every day as it will create a lot of noise in the repo. |
To the number of releases. I'm afraid we could end up in situation like this: Having hundreds of releases to crawl + generating yet another notification for users subscribed to the repo. Maybe I'm overestimating the impact of this and going with Pre-release for nightlies would also work. |
I agree with Nico here, I am not a fan of having too many GH releases - let's maybe consider an alternative, like publishing them as a new npm package separately? ex. |
The root issue here is sizing. The Hermes tarball is ~400Mb and can't stay in a NPM package. Splitting is 'doable' but would add a lot of custom infra to re-compose the artifacts so really suboptimal |
So basically it'd be better to have something like a "bucket", sourceforge style thing where to store them. Right. Need to think about this. |
What does that tarball include? |
Essentially. An S3 bucket would work but anything where we can store asset and access them via URL could work. Worth noting that CircleCI assets might already solve this issue. Artifacts have a default retention of ~30 days, which I believe is acceptable for nightlies: https://circleci.com/docs/artifacts#artifacts-and-self-hosted-runner or we can host them somewhere else.
Debug symbols are making the tarball explode both on Android & iOS. The iOS tarball here is 466Mb: https://github.com/facebook/react-native/releases/tag/v0.69.2 This is only for Hermes. All the RN libraries are shipped without Debug symbols due to size constrains, making for a far from optimal debugging experience IMHO. |
I am not familiar with RN's infrastructure, but I think you can take inspiration from the way Homebrew manages their pre-built binaries (Bottles) using Github's package registry. I am not an expert on it, but I remember reading some threads on the topic and that their solution has been advised and validates by Github They are using Github Packages to host them. Github Packages are not tied to releases or a repo per default (they belong to a user or org), so they wont pollute the Releases page, but they can optionally be linked to a repo. The trick is to upload the artifact as an OCI image, a container with only one layer. OCI images are a group of tgz archives and the content does not matter. They use Skopeo for it, like this:
From what I understand, the Once uploaded, the artifacts can be downloaded using a stable URL, and discovered using GH package API, without authentication. Github can cache them efficiently as all artifacts have a public and static URL once uploaded. This is probably a different discussion than this RFC, but I think this would elegantly solve the artefact distribution problem without having to manage external system or credentials. If needed, I can spent some time investigating this further and work with a React Native team member on the implementation. Some links:
|
thanks @renchap for the great insight! Worth looking into it more :) Just wanted to add a comment here to quote some things that @yungsters mentioned in his PR (facebook/react-native#34401) so that we won't lose track:
We should probably enhance the RFC with a section along the lines of "expectation on packages in the /packages folder" or something with some guidelines for when adding a new one or extracting one from react-native itself. This would also help with scenarios like the one recently mentioned in @necolas' RFC #496 (comment) of extracting Animated to its own package (that was previously attempted by @EvanBacon here facebook/react-native#28892) |
I haven't followed up on this yet, but I'd like to mention that this is a great solution and might actually be the silver bullet for our problems! I've reached out to @renchap and we'll try to come up with a solution around this. I'd say for the time being, let's trying to keep the conversation here focused on Monorepo (folder structure, versioning, releasing, etc.). We'll be sharing a separate discussion on packaging/artifacts in the future. |
Adding an extra note here, we will likely have to do some deep-dives across the react-native repo and figure out if there are rogue packages hidden within it. By complete chance I just found out about this extra package.json living deep within the ReactCommon folder... https://github.com/facebook/react-native/blob/main/ReactCommon/hermes/inspector/tools/msggen/package.json this type of scenario should not be present anymore when the goal of this RFC is completed (a hidden, non-published, package.json hidden deep within the source code) |
And it would likely help us avoid Animated becoming more coupled to React Native internals, as has recently happened with the introduction of |
Co-authored-by: Nicola Corti <corti.nico@gmail.com>
Co-authored-by: Nicola Corti <corti.nico@gmail.com>
Merged 🎉 Thanks everyone who helped pushed this proposal through the finish line - it's been a massive endeavour and I want to give a special thanks to @cortinico @hoxyq and @tido64 for all their work 👏 And thanks to everyone who commented, suggested, pointed things out! ...and now we couuuuld go back to @empyrical's RFC #49 🤣 😉 |
Summary: This PR adds [verdaccio](https://github.com/verdaccio/verdaccio) to release packages in the `packages` directory during the E2E test on CI. The rationale behind this is the following: - Firstly, we wanted to push the [monorepo RFC](react-native-community/discussions-and-proposals#480). We hit an issue when renaming the packages to follow the same convention caused by the e2e test using the template to fail. This is because the template installs packages from the live version of npm – and if you just rename a package in a given PR without releasing it, the package understandably can't be installed since it's not published, yet – as you can see [here](https://app.circleci.com/pipelines/github/facebook/react-native/15286/workflows/149df51f-f59b-4eb3-b92c-20c513111f04/jobs/282135?invite=true#step-108-283). - Secondly, the current e2e test on `main` does not actually test the latest code of the packages in the `packages` directory as it simply downloads the latest versions from npm. This creates a divide between what's tested and what users should expect when using nightlies or when a new minor is released. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [Internal] [Changed] - Use verdaccio for template e2e test Pull Request resolved: facebook#34577 Test Plan: `test_js` CI check should pass. Additionally, I have temporarily updated the [PR](facebook#34572) renaming `assets` to `assets-registry` to include the verdaccio changes – the `test_js` passes there, additionally proving merging this PR will unblock us with the rename PRs. Reviewed By: cipolleschi Differential Revision: D39723048 Pulled By: cortinico fbshipit-source-id: aeff3811967360740df3b3dbf1df50e506fb72d8
Summary: The [monorepo RFC](react-native-community/discussions-and-proposals#480) calls for renaming: * `react-native-community/eslint-config` -> `react-native/eslint-config` * `react-native-community/eslint-plugin` -> `react-native/eslint-plugin` It also calls for the versions to be aligned with the rest of main -- currently `0.72.0`. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [General][Changed] - Renamed `react-native-community/eslint-config` to `react-native/eslint-config` v0.72.0 to align with other packages [General][Changed] - Renamed `react-native-community/eslint-plugin` to `react-native/eslint-plugin` v0.72.0 to align with other packages Pull Request resolved: facebook#34581 Test Plan: First test is to run `yarn lint`, and verify that output matches before and after this change. Second test is to change the ESLint config to use the "old" packages (under `react-native-commnuity`) and run `yarn lint` to make sure that they work as expected. This is what customers will experience, until they manually move to the new ESLint packages. Reviewed By: cortinico Differential Revision: D41520500 Pulled By: hoxyq fbshipit-source-id: a61e5ae15d5aaf11f0143a3b0257a60a03b1550b
Summary: ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [General] [Changed] - Rename polyfills to js-polyfills as part of react-native-community/discussions-and-proposals#480 Pull Request resolved: facebook#34574 Reviewed By: cipolleschi Differential Revision: D39268818 Pulled By: hoxyq fbshipit-source-id: c87807460f27fc83667d18c350a4a847459f056e
…ebook#34571) Summary: ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://reactnative.dev/contributing/changelogs-in-pull-requests --> [General] [Changed] - Rename normalize-color to normalize-colors as part of react-native-community/discussions-and-proposals#480 Pull Request resolved: facebook#34571 Reviewed By: cortinico Differential Revision: D39235696 Pulled By: hoxyq fbshipit-source-id: b6d5fcae9fb5c953c2f7b48f73a95cd883ff8f63
This proposal is something that me and @tido64 (and @cortinico to some extent) have been brainstorming for a little while - after a recent problem with
react-native-codegen
and RN68.As per title, basically this proposal is not much as in introducing something completely new, but more to discuss a solution to an existing problem. To put simply, the current shape of the
react-native
codebase hosted in GitHub is lacking structure and consistency and this is leading to a number of issues.You can read more details in the text itself - here's the file view for easier reading experience.
To see proposal in graph form, expand this.
Feel free to add comments and questions! I'd love to see this turn into something actionable as soon as possible, since it addresses a number of system problems with the repo - it will require some coordination and collaboration, but I'm sure Meta and MSFT can collaborate to make it happen :)
This will also unlock/make easier to revisit other RFCs, such as @empyrical's #49
sidenote: this pr also adds a folder to keep images in,
assets/
, and adds to the .gitignore the pesky macos DS_Storelast updated on Sept 2022 to reflect the latest status.