undici icon indicating copy to clipboard operation
undici copied to clipboard

Move to monorepo

Open ronag opened this issue 4 months ago • 34 comments

This would solve...

Currently the undici package is quite heavy with a lot of stuff that you might not need, e.g. web api. Would be good to separate undici "core" into its own package.

The implementation should look like...

Use pnpm

ronag avatar Feb 11 '24 12:02 ronag

I think @Ethan-Arrowood was managing this idea for a while, after the split of undici-types

metcoder95 avatar Feb 12 '24 10:02 metcoder95

There's some old threads about monorepo ideas. Happy to revisit

Ethan-Arrowood avatar Feb 12 '24 13:02 Ethan-Arrowood

I think it's inevitable that we adopt a monorepo at this point, this project has grown too big, and there are separate subsystems that could be used separately.

mcollina avatar Feb 12 '24 14:02 mcollina

Use pnpm

This is also my preference. Does anyone feel extra-strongly about using npm?

pnpm is now included in corepack so it should be very easy for everyone to use it without any additional install or set up (minus a corepack enable call).

Ethan-Arrowood avatar Feb 12 '24 19:02 Ethan-Arrowood

I will also start by experimenting with npm workspaces to start. They've supposedly come along way so maybe its worth trying again.

Ethan-Arrowood avatar Feb 12 '24 20:02 Ethan-Arrowood

I've not been able to make npm workspace work in any reasonable way for OSS development

I think the most useful part would be if somebody could figure out how to set it up with changesets so that we could ship independent versions of the packages. Alternatively, they will all have the same version.

mcollina avatar Feb 12 '24 22:02 mcollina

I have experience with changesets. Do you want me to try npm+changesets or go straight to pnpm+changesets?

Ethan-Arrowood avatar Feb 12 '24 22:02 Ethan-Arrowood

Rough plan:

  1. Create PR that moves everything into packages/undici and documentation
    • No code will split up just yet
    • CI should run as normal
    • Docs site should work as normal
    • Other tooling should continue to work (build, commit hooks, etc.), and run from root as normal
    • This should only introduce npm workspaces. Changesets should not be required.
    • Even though the code will exist in packages/undici directory, the package should still be named and published as undici (for now).
    • (In nodejs/node) Update Node.js -> Undici vendor process

Step 1 is simply the first step in migrating to a monorepo. This will reduce impact on contributors and should not disrupt development.

  1. Code split (incremental)
    • Create new packages packages/core, packages/fetch, and packages/types
    • Start publishing packages as @undici/core, @undici/fetch, and @undici/types
    • Retain packages/undici as undici and make it roll-up and export all of the other packages
    • Introduce changesets for version and publishing management.
    • (Optional) Move linting and formatting to root

Step 2 serves as an incremental step to a true monorepo organization. By retaining undici package we don't have to affect Node.js or users. Everything will appear as normal.

  1. Migrate Node.js from undici to @undici/fetch

    • Make necessary change in nodejs/node to vendor @undici/fetch instead of undici
  2. More code splitting!

    • Intentionally not-well-defined at this time, consider other parts of Undici that we can split out into its own separate packages.

Ethan-Arrowood avatar Feb 13 '24 15:02 Ethan-Arrowood

I'm +1

mcollina avatar Feb 13 '24 18:02 mcollina

LGTM 🚀

metcoder95 avatar Feb 13 '24 19:02 metcoder95

I'll have a PR up for step 1 very soon. I'd like to land #2766 first as that will help simplify things.

Ethan-Arrowood avatar Feb 19 '24 17:02 Ethan-Arrowood

My experience with npm workspaces is that they are not fit for purpose. Specifically, they are missing the workspace protocol, https://pnpm.io/workspaces#referencing-workspace-packages-through-aliases, which makes it impossible to maintain an OSS library without it. (Or I couldn’t figure out how to handle versioning).


@KhafraDev wdyt?

mcollina avatar Feb 20 '24 07:02 mcollina

I've found more success where versions are updated by a tool such as changesets. That said, switching to pnpm has other great benefits -- and everyone can do it since it's supported in corepack. Ill make sure to present options for us to consider

Ethan-Arrowood avatar Feb 20 '24 14:02 Ethan-Arrowood

I dont like monorepos. They make managing code complicated. I would rather have one package released on npm with everything than having to manage many packages where each package has different versions.

Uzlopak avatar Feb 20 '24 15:02 Uzlopak

Without hearing the benefits of having a monorepo, I can't make an argument against it, or agree to having one.

A monorepo won't make undici easier to maintain, it won't make undici easier to publish, and it won't make it easier to install. Is the only benefit bundle size? If so, undici has constantly grown and now has 7.2 million downloads a week, and we have not received a single complaint about bundle size. People who use @types/node aren't complaining about the added size of undici-types, even though most of the types aren't being used.

The entirety of undici is 1.24 MB. According to npm, fetch is 277 kB, cache is 25.6, and WebSocket is 57.5. If we were to split them into separate packages, undici core will still be ~880 kB. That is a menial difference when the docs being bundled are 183 kB, types are ~80 kB, and llhttp is 283 kB. I believe Matteo prefers that docs are included for them to be easy to access (same reason we don't use esbuild on undici and put a minified file on npm), but claiming that bundle size is too big while shipping a substantial amount of markdown files doesn't make sense to me.

I am willing to change my mind on this, if there is a noticeable improvement to any area of development. Let me know 😄!

KhafraDev avatar Feb 20 '24 15:02 KhafraDev

Well, next undici version should be about 150 kb smaller. We can still remove some artifacts in llhttp and save about 8 kb and maybe the 1kb in for the MIT license in the fetch folder by giving credits for fetch to @Ethan-Arrowood in the main license file.

Maybe 74 kb if we drop the non-simd-version of llhttp.

Uzlopak avatar Feb 20 '24 15:02 Uzlopak

The biggest motivation for me is simplification. We have so much going on in this library now, splitting into at least core and fetch would be a developer experience improvement imo.

I think the benchmark and documentation split outs are still valuable even if we don't reorganize code.

Ethan-Arrowood avatar Feb 20 '24 16:02 Ethan-Arrowood

fetch is contained to lib/fetch, the only difference would be that it would be moved to packages/fetch/lib and I couldn't directly require/import the files since they'd have to use workspace aliases. The code is very organized and easy to work on right now.

KhafraDev avatar Feb 20 '24 16:02 KhafraDev

Also cache, eventsource, and websocket that are all related to web platform APIs too, right?

I won't open a monorepo PR just yet. I'd like to split out other pieces first like benchmarks, docs, and examples. Maybe @ronag or @mcollina have other inputs regarding monorepo organization.

Ethan-Arrowood avatar Feb 20 '24 17:02 Ethan-Arrowood

yes, those are all self-contained (and small relative to package size)

KhafraDev avatar Feb 20 '24 17:02 KhafraDev

I would prefer to have all web api stuff in own thing.

ronag avatar Feb 20 '24 17:02 ronag

The biggest motivation for me is simplification. We have so much going on in this library now, splitting into at least core and fetch would be a developer experience improvement IMO.

+1

metcoder95 avatar Feb 20 '24 19:02 metcoder95

how would it improve developer experience?

If developer = maintainer, the act of maintaining the tooling offsets any simplification (I don't see how adding more tooling could simplify the development process). Being able to write pure javascript that works without tooling is a huge benefit here. Even 'simple' tooling like esbuild has caused us issues - imagine more complex tools! Personally, moving to a monorepo would break my development process.

If developer = user, having to install more packages and deal with a breaking change for a benefit no one here has expanded on doesn't seem like a good idea.

I'm not looking for an essay or a research study, just anecdotal evidence would be fine.

KhafraDev avatar Feb 20 '24 19:02 KhafraDev

For me it would be easier not to deal with all the web api both in terms of browsing code and testing.

ronag avatar Feb 20 '24 20:02 ronag

regarding testing - what is the issue with that? too slow?

Uzlopak avatar Feb 20 '24 20:02 Uzlopak

web tests will still run in a monorepo though

KhafraDev avatar Feb 21 '24 01:02 KhafraDev

Then I would suggest to split it into two different repos. undici core and undici web.

ronag avatar Feb 21 '24 05:02 ronag

We can leverage a tool like turborepo to make this as efficient as possible, but since web would depend on core, any change to core would result in running web too. But like if maybe pieces like the redirect handler or mocks lived outside of core, then changing those pieces would not require running core or web. Just think of it like a directed graph. It will depend on how we organize our modules.


I think the issue some of us have is the information density of the project. Sure the code lives in separate folders, but yet we still have a lot of it. It is difficult to know where to make changes, how to add new apis, etc. Anecdotally, I was confused for a bit what the cache directory contained, and why it wasn't a http cache solution that all of undici could use (like the idea shared in #2256).

Beyond that I have other related issues such as code quality, documentation, and testing organization that come to mind. I can empathize with @KhafraDev and @Uzlopak , and agree that just switching to a monorepo layout is not going to fix these issues directly. However, in my opinion, fixing things like that can be easier when there is less code density.

Having a minimal core package will enable us to slim down and simplify the core api to a point where it will also be simpler to document, test, type, and more. Then other pieces can build off of that and adopt the same simplicity in docs, tests, etc.

Ethan-Arrowood avatar Feb 21 '24 06:02 Ethan-Arrowood

I haven't thought too much about what a minimal @undici/core would contain. I find the current Dispatcher implementation to be confusing. The indirection in particular is the worst part (such as the makeDispatcher piece in index.js). This obviously was done for good reason (such as extending/implementing the interface in an easy way across the repo). But if for example these pieces lived in their own module, it could force us to come up with a better API for other pieces like the Mock interfaces or custom Handlers.

This is just an off-the-cuff idea, but I hope it helps provide a little more anecdotal evidence behind my support for a monorepo organization

Ethan-Arrowood avatar Feb 21 '24 07:02 Ethan-Arrowood

I think the information density is an excellent description. Something been bothering me but I couldn't describe it. I think especially for third-party library implementers (such as my own nxt-undici and others such as elastic and possibly axios, got etc...) who only actual need or care about undici core. We/They have to navigate through a huge project just for the few things we need.

ronag avatar Feb 21 '24 07:02 ronag