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

Local build DX improvements #490

Merged
merged 6 commits into from
Oct 22, 2023
Merged

Conversation

alpharder
Copy link
Contributor

@alpharder alpharder commented Oct 20, 2023

Why

My main goal was to be able to install and build monorepo packages locally.
My environment is MacOS and Node v20.8.0 and I was following instructions from DEVELOPMENT.md.
I haven't recorded all of dozens build errors I've encountered, but I was effectively unable to get packages built.

I want to contribute to this project and these problems were a great obstacle preventing me from doing that.
I hope that these changes will help other potential contributors avoid encountering same problems as I did.

Summary of changes

Local package linking

  • Local package linking is now done via NPM's native feature: workspaces. Yarn and PNPM support it too. npm install command now handles what lerna bootstrap and lerna link handled previously.
  • Lerna was updated to the latest version. They also recommend switching to NPM workspaces: https://lerna.js.org/docs/legacy-package-management#background
  • Important change: Workspace package dependencies are now hoisted to a single, top-most node_modules directory, meaning packages/*/node_modules will only contain dependencies that are conflicting with other packages' dependencies. For, example, packages/foo/package.json and packages/bar/package.json require [email protected] and packages/baz/package.json requires [email protected]/node_modules would contain [email protected] and packages/baz/node_modules would contain [email protected].
  • Important change: individual packages don't contain their own package-lock.json file, the one from the project root is used.

Other changes

  • npm install now automatically installs compilers, allowing to avoid calling npm run install-compiler;
  • Angular dependency versions are now synchronized to match exactly same versions across GUI apps
  • The following packages are excluded from the workspaces, preventing lerna build from building them. They seem to be non-buildable in their current state:
    "!packages/benchmark",
    "!packages/fs",
    "!packages/example-app",
    "!packages/orm-browser-example",
    "!packages/framework-examples"
  • Packages' tsconfig.json files were updated to have compilerOptions.types correctly set, preventing redundant TS compilation from checking types of every node_modules/@types/* package (even not used in a particular monorepo package), which would occur every time a monorepo package is being built. This is because of having a single common node_modules shared across monorepo packages.

Relinquishment of Rights

Please mark following checkbox to confirm that you relinquish all rights of your changes:

  • I waive and relinquish all rights regarding this changes (including code, text, and images) to Deepkit UG (limited), Germany. This changes (including code, text, and images) are under MIT license without name attribution, copyright notice, and permission notice requirement.

@alpharder alpharder marked this pull request as ready for review October 20, 2023 21:53
@alpharder alpharder marked this pull request as draft October 20, 2023 23:04
@alpharder
Copy link
Contributor Author

I have to adapt Github CI workflow file prior for marking it as ready for review.

@alpharder
Copy link
Contributor Author

I don't think that integrating Nx can bring any value to the project, it's just another layer on top NPM workspaces.

It's bloated and adds another layer of abstraction (= more moving parts).

NPM workspaces has everything a monorepo need. Moreover, I believe Lerna could also be ditched because it doesn't bring any value too.

@alpharder
Copy link
Contributor Author

I could also make some adjustments in order to make it buildable under Windows.

@marcus-sa
Copy link
Contributor

marcus-sa commented Oct 21, 2023

I don't think that integrating Nx can bring any value to the project, it's just another layer on top NPM workspaces.

It's bloated and adds another layer of abstraction (= more moving parts).

NPM workspaces has everything a monorepo need. Moreover, I believe Lerna could also be ditched because it doesn't bring any value too.

This is completely wrong. Nx isn't just a layer on top of NPM workspaces, in fact it has nothing to do with NPM workspaces. They're completely different tools.

Nx brings a lot of value, especially to a mono repository of this size.

Have you even used Nx before?

@marcus-sa
Copy link
Contributor

You can literally only do two things with NPM workspaces

  • Manage dependencies across packages
  • Run scripts in packages

@marcus-sa
Copy link
Contributor

marcus-sa commented Oct 21, 2023

Also, this won't and can't solve the most annoying thing about the current setup which is package linking.

@alpharder
Copy link
Contributor Author

Have you even used Nx before?

Yes, when NPM workspaces haven't existed yet and Nx provided what they now call as integrated monorepos. It was useful back then.
Now, since package managers do provide their own workspaces, they've transformed their business model into:

a) Providing advanced parallel task execution;
b) Vendor-locking their users to Nx wrappers for everything (Nx plugins). E.g. using Angular? Your build system should use @nx/angular instead of Angular CLI. Angular workspace? No, Nx workspace, please;
c) Selling their cloud services;

Nx isn't just a layer on top of NPM workspaces, in fact it has nothing to do with NPM workspaces.

Their documentation says exactly what I mentioned: https://nx.dev/recipes/adopting-nx/adding-to-monorepo

Nx brings a lot of value, especially to a mono repository this big.

Could you please be more specific?
Again, their documentations declares that these are the only benefits when added on top of NPM/PNPM monorepo:

fast task scheduling
support for task pipelines
caching
optionally remote caching with Nx Cloud
optionally distributed task execution with Nx Cloud

Which are either:
a) completely redundant because tsc --build already does all of that, except for parallel task execution, which is fragile and error-prone.
b) promoting their commercial cloud services

In addition to that, I personally don't consider this specific project as a big mono repository.

Also, this won't and can't solve the most annoying thing about the current mono repo setup which is package linking.

Could you please clarify what do you mean? Installation-time linking or publish-time linking?

@alpharder
Copy link
Contributor Author

The main idea is

  1. If you need monorepo, use NPM/PNPM workspaces,
  2. If you feel CI builds are slow, add Turborepo on top of that,
  3. If you want to over-engineer your monorepo and reinvent a wheel, replace above with Nx

@marcus-sa
Copy link
Contributor

marcus-sa commented Oct 21, 2023

Now, since package managers do provide their own workspaces

Package managers have been providing their own workspaces for years, it's just NPM that has been behind.

Could you please clarify what do you mean? Installation-time linking or publish-time linking?

We're relying on dependencies of packages to be built and put into that package's node_modules for module resolution to work during development.
This is cumbersome since you have to rely on the packages to have been built before you can do anything else.
You want to test package A based of changes to package B? Sure, you just have to rebuild all of it's dependencies before you can do that.

fast task scheduling
support for task pipelines
caching
optionally remote caching with Nx Cloud
optionally distributed task execution with Nx Cloud

Yes, those are in my opinion especially important for developer experience.

a) completely redundant because tsc --build already does all of that, except for parallel task execution, which is fragile and error-prone.

If you have enabled incremental builds, but I don't see what that has got to do with the forementioned features, because incremental builds does none of that.

@marcus-sa
Copy link
Contributor

marcus-sa commented Oct 21, 2023

Also, what you seem to have completely missed, which is one of the most important aspects of Nx, is that you have architecture and structure that you must abide by.
Nx is battle-tested and has for a very long time been considered the defacto standard for mono repos in the JS ecosystem.
It's a whole lot easier to contribute to a project when there's extensive documentation on the tooling and concepts that must be followed.

@marcus-sa
Copy link
Contributor

marcus-sa commented Oct 21, 2023

If you feel CI builds are slow, add Turborepo on top of that

In my opinion the most useless tooling that has been created.
You have Bazel, Nx or moonrepo which are far superior.

Turborepo wasn't that great back when I used it, but perhaps it has become better.

@marcus-sa
Copy link
Contributor

marcus-sa commented Oct 21, 2023

Vendor-locking their users to Nx wrappers for everything (Nx plugins). E.g. using Angular? Your build system should use @nx/angular instead of Angular CLI. Angular workspace? No, Nx workspace, please;

Of course you're gonna be vendor locked in when you use a build system or a tool. What'd you expect?
Maybe it's just me, but I don't see how that is bad.

@marcus-sa
Copy link
Contributor

marcus-sa commented Oct 21, 2023

If you want to over-engineer your monorepo and reinvent a wheel, replace above with Nx

What exactly is being over engineered and reinvented?

"composite": true,
"types": [
"pg",
"sqlstring"
Copy link
Member

Choose a reason for hiding this comment

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

why is it needed with this change to list all external types in this tsconfig:types section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Packages' tsconfig.json files were updated to have compilerOptions.types correctly set, preventing redundant TS compilation from checking types of every node_modules/@types/* package (even not used in a particular monorepo package), which would occur every time a monorepo package is being built. This is because of having a single common node_modules shared across monorepo packages.

@marcj marcj merged commit 0317cec into deepkit:master Oct 22, 2023
@marcj
Copy link
Member

marcj commented Oct 22, 2023

Thanks, merged with b3655bb

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.

3 participants