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

chore: Move to monorepo ❗ ❗ ❗ #7342

Merged
merged 17 commits into from
Sep 21, 2023
Merged

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Sep 20, 2023

  • Reorganized almost all files in the project:
    • packages/rxjs: The new home for the main rxjs package.
    • apps/rxjs.dev: The new home for the docs app
    • packages/*: will be where other future packages live (i.e. @rxjs/observable)
    • We may move the decision tree project out from under the docs app to the apps/* directory at some point.
  • "Fixed" the docs app tests: We've had an issue where the many of the tests were broken following an update to the docs up. I fixed this by simply deleting the broken tests (I know) after attempting to debug them and realizing it was a huge time suck. The principle here is that it's better to run some tests for the docs app in CI than run no tests in CI for the docs app.
  • Updated lodash (and @types/lodash): This was done to fix problems with the TS@latest tests we run in CI. This is because the newer version of TypeScript fixed the types for WeakMap, and the older version of @types/lodash had them redefined differently. This is a small change.
  • Updated a LOT of locations in a lot of build-related files. The changes are obviously to just point from ./ to ./packages/rxjs/ in some cases.
  • Added a script to ensure the license file and code of conduct are duplicated to be under the rxjs package when we publish.
  • moved to yarn!!: This is a big one. I did not want to do this, but I had no choice. Basically, npm install for workspaces will hoist all packages to the top level. When that's done, both @types/mocha (from rxjs) and @types/jasmine (from the docs app) are hoisted to the root node_modules folder. When that happens, TypeScript chokes on build because of duplicate type definitions for things like describe, xit, etc. Yarn features a nice nohoist setting, and we'll have to stick with Yarn until npm decides how they're going to fix it

Reorganized everything. Removed broken tests from docs app so we can run tests in CI for that again. Some tests are better than no tests. Updates the GitHub actions, some of the README stuff, and build scripts
@benlesh
Copy link
Member Author

benlesh commented Sep 20, 2023

Note that the older actions "Node 16 build" and "ts@latest (18)" can be ignored for this PR.

image

@benlesh
Copy link
Member Author

benlesh commented Sep 20, 2023

Note to @jakovljevic-mladen and @niklas-wortmann: To build or run the docs app now you'll want to use yarn workspaces: yarn workspace rxjs.dev start for example. Pretty straightforward, but I wanted to call it out.

After you initially pull this down, you'll want to delete your node_modules, then run yarn install, after that you're good to go.

Copy link
Member

@jakovljevic-mladen jakovljevic-mladen left a comment

Choose a reason for hiding this comment

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

Couple of issues found during code review.

I will have to test this locally which I will do later this week (hopefully).

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
apps/rxjs.dev/tools/transforms/config.js Show resolved Hide resolved
packages/rxjs/README.md Show resolved Hide resolved
packages/rxjs/README.md Show resolved Hide resolved
packages/rxjs/README.md Outdated Show resolved Hide resolved
benlesh and others added 9 commits September 21, 2023 09:24
Co-authored-by: Mladen Jakovljević <[email protected]>
Co-authored-by: Mladen Jakovljević <[email protected]>
Co-authored-by: Mladen Jakovljević <[email protected]>
Co-authored-by: Mladen Jakovljević <[email protected]>
Co-authored-by: Mladen Jakovljević <[email protected]>
Co-authored-by: Mladen Jakovljević <[email protected]>
@benlesh
Copy link
Member Author

benlesh commented Sep 21, 2023

@jakovljevic-mladen I had to revert both of your suggestions for resolving directory/file locations that I committed above. They broke the docs app build. If you think those are incorrect, I suggest we fix it in a follow up.

Otherwise, I think this is good to go.

@benlesh
Copy link
Member Author

benlesh commented Sep 21, 2023

I'm going to go ahead and merge this so we can move on. @jakovljevic-mladen you can double-check the docs app to make sure everything is fine, but I ran it locally and ran through a few user flows just to see if it was okay, and everything checked out fine.

@benlesh benlesh merged commit 0bd47ea into ReactiveX:master Sep 21, 2023
3 checks passed
@jakovljevic-mladen
Copy link
Member

@benlesh, I guess I was wrong about PROJECT_ROOT. It was really meant to point to the project root, while all other configurations should've been derived from that point. Tested it locally and works like a charm 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants