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: Use Nx [proposed] #135

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

chore: Use Nx [proposed] #135

wants to merge 6 commits into from

Conversation

Peeja
Copy link
Member

@Peeja Peeja commented Jan 28, 2025

What's going on here

As I encountered friction during the monorepo import, I started exploring a few options. Nx quickly stood out as a way to untangle a lot of issues, mainly dealing with task dependencies across multiple packages. So I decided to run with it for a bit and see if it kept panning out. It did, and here's the result.

We don't have to keep Nx—a fair bit of the work here was just clarifying and standardizing the npm scripts, which is important with or without Nx—but I'm going to make the case for it in this PR.

What's in this PR

This PR adds Nx to the existing monorepo. It doesn't include the w3ui import, which will be in a separate PR to keep things clearer. But I've used that import to drive some of the decisions in this PR.

This PR does not yet include the ability to release. That should be added before merging, but I want to get this in front of eyeballs ASAP.

What to look for

Here are some things to try:

  • Pull the branch, pnpm install, and try pnpm nx run-many -t test, which will run the test task in every project in the repo. It should be passing.
  • Try running it again. It should use the cache! Try changing a file somewhere and see how it breaks the cache (minimally).
  • Have a look at the GH Actions workflow and output. I've replaced the individual package-specific workflows with one which runs everything. Note that in this PR it's running things over all projects, but in a feature PR it would only run tasks for the affected projects. (This PR, being the one that adds Nx, affects everything.)
  • Also notice that a lot of the tasks in that workflow are cached, because they've run in earlier GHA runs and the code hasn't changed since.

Why Nx?

  • Builds on pnpm: Nx builds on top of existing monorepo tools. It doesn't replace pnpm—it uses pnpm's notion of the workspace with packages and scripts.

  • Task dependencies: A lot of tasks depend on each other. Before attw looks over the (built) types for problems, a package has to build. Before one package can build, all the packages it depends on have to build, too. Nx manages the dependency graph across the entire monorepo and runs everything in order, as needed.

  • A great task runner: Instead of just dumping all the output into a terminal together like pnpm mostly does, Nx has output that handles lots of projects and tasks better. It also provides HTML output that can make it even easier to look through the results.

    Run results in terminal

  • Caching: Each task can be cached (if appropriate) so it doesn't have to re-run if its input files haven't changed, which makes running large DAGs of tasks over and over again during development a lot faster.

    YouTube

  • Only test affected projects: The affected command can run a task (such as test) for all of and only the projects that changes in the current branch have touched.

  • Flaky test detection and handling: I haven't tried this myself yet, but it sounds pretty great.

  • File-based release versioning: Just like Changesets.

Nx Cloud

Nx also offers a product, called Nx Cloud. A bunch of it is completely free, and the paid portions include a pretty generous free tier.

  • Hosted run results: As far as I can tell, this is just entirely free. At the completion of a run, whether local or in CI, you get a URL to a page that displays the results of your tasks. You get to use this without even signing up for anything.

  • Remote caching (Nx Replay): Cached task results are stored in Nx Cloud. That means that even if you haven't run and cached a task yet, if someone else on the team has, you can simply use their cached result. For instance, when you check out a new branch and build it, that build may already have been cached by the person who was working on it, making your initial build super fast.

    YouTube

  • Distributed task execution (Nx Agents): Rather than run CI tasks on GH Actions itself, we can send the tasks off to Nx Agents. These can efficiently divide the individual tasks to run in parallel. That's more effective than trying to manually split the tasks and projects across separate GH Actions workflows in advance.

    YouTube

And what's great is that if we try the free tier and run out of free credits, it simply turns off the Nx Cloud features and reverts to entirely local runs. These are just enhancements layered on top of Nx. And, if it does fit our project and the value proposition is correct, the cost of the Nx Agents should be less than the cost it saves us in GH Actions minutes, not to mention developer time. (A minute on an Nx Cloud Linux Medium costs $0.00055/cr × 10cr = $0.0055; a minute on a GitHub Linux 2-Core costs $0.008.)

Bonus

If we really like it, we might even consider bringing our Go code in here too! But that's a discussion for another time.

nx-go

@Peeja Peeja force-pushed the nx branch 3 times, most recently from 3480ba2 to d35844c Compare January 28, 2025 18:06
@Peeja Peeja changed the title Nx! chore: Use Nx [proposed] Jan 29, 2025
Copy link
Member

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

My first response is:

This all looks great, with the caveat that I'm not in the weeds these days on the latest monorepo tools. But Nx seems to have good sentiment around web.

I'm not sure I'd go all in the Nx Cloud thing yet. It doesn't look like Nx Cloud is included for now -- am I reading that correctly? I'm somewhat loathe to add more services into our builds beyond Github, though I can't tell if maybe there are replacements for Github there? And then I also could be convinced if we could drop seed.run when we merge upload-service-infra.

But beyond that this all seems reasonable. Questions are just that.

Oh one other bit: once w3ui is merged, is it safe to then start having other devs bring in other repos? Or can we jump into that as soon as this is merged?

"@docusaurus/core": "^3.0.0",
"@docusaurus/preset-classic": "^3.0.0",
"@nx/js": "^20.3.2",
Copy link
Member

Choose a reason for hiding this comment

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

just curious what's the difference between these two NX packages?

Copy link
Member Author

Choose a reason for hiding this comment

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

nx is Nx itself; @nx/js is the plugin for JavaScript/TypeScript projects. I believe it's @nx/js that infers Nx tasks from npm scripts, for instance.

@@ -184,7 +184,7 @@ export const createService = (context) => ({
...createAdminService(context),
// @ts-expect-error `uploadTable` items now have a `cause` field. This does
// not matter since `admin/store/inspect` handler does not use this table.
store: createLegacyAdminService(context).store
store: createLegacyAdminService(context).store,
Copy link
Member

Choose a reason for hiding this comment

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

??? I thought we were set to no trailing commas?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we use "trailingComma": "es5", which is trailing commas anywhere that ES5 won't complain about them (whereas modern ES is more permissive).

@@ -32,22 +33,5 @@
"skipLibCheck": true,
"stripInternal": true,
"resolveJsonModule": true
},
"typedocOptions": {
Copy link
Member

Choose a reason for hiding this comment

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

currious why this isn't neccesary?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not necessary because we're not using TypeDoc anymore. But I'm not fully ripping that out right now, and I don't remember now why I felt it was helpful to get rid of this. Something about the way the TS configs inherit from each other? I probably could add it back, but we're just going to rip it out again shortly when we completely remove TypeDoc and Docusaurus as unused.

@hannahhoward
Copy link
Member

hannahhoward commented Jan 30, 2025

If we really like it, we might even consider bringing our Go code in here too! But that's a discussion for another time.

Why bring in code from a language that already has built in support for monorepo patterns from the jump? :P

#unneccesarySnark

@Peeja
Copy link
Member Author

Peeja commented Jan 30, 2025

A discussion for another time, Hannah. 😛

Seriously, though, if anything, it would be so we can have dependencies run across the two languages. Golang's native monorepo support is roughly equivalent to pnpm's workspaces, and Nx would run on top of both. I'm not prepared to assert that it's worth actually using it on our Go code right now, though, just a thought for the future.

@Peeja
Copy link
Member Author

Peeja commented Jan 30, 2025

It doesn't look like Nx Cloud is included for now -- am I reading that correctly?

Yeah, that's right. I have added a key for it, but that just means that at the end of a local run you get a URL with prettier output. It's not actually even associated with the project right now, they just have a free service that does that.

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