-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
chore: add nx monorepo tooling #3465
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
Conversation
Thanks for the PR, @JamesHenry! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
Nx Cloud ReportCI ran the following commands for commit 830930c. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch
Sent with 💌 from NxCloud. |
Something we may or may not want to address here: how come the unit tests are split out into individual invocations per project? |
@bradzacher Ok cool, let's punt the testing point for now then - I believe the nx parallelized logs are a little easier to scan than the lerna ones, but I'll do a dedicated evaluation in a follow up draft |
Updated dependency graph for the repo: (Docs on how you render this: https://nx.dev/latest/cli/dep-graph) |
"check:docs": "lerna run check:docs", | ||
"check:format": "prettier --list-different \"./**/*.{ts,js,json,md}\"", | ||
"check:spelling": "cspell --config=.cspell.json \"**/*.{md,ts,js}\"", | ||
"build": "npx nx prebuild @typescript-eslint/types && nx run-many --target=build --all --parallel", |
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.
For now, we need to include an explicit reference to the prebuild
script of @typescript-eslint/types
because it is unusual in that it has a side-effect on disk outside of dist/
or build/
(the common output locations Nx checks for) and it therefore does not play well with the automated caching behavior based on simplistic npm scripts (it would be configurable if we moved to full Nx targets in workspace.json).
Feel free to ping me on slack if you want to cover this in more detail and I can walk you through where this is today vs how it can be configured
@bradzacher As discussed, starting with the most lightweight implementation possible where we just execute essentially the same "targets" (Nx term) as we do currently but going through Nx (+ Nx Cloud) gets us distributed caching for free.
As well as providing monetary contributions every month https://nrwl.io have kindly sponsored us with 10,000 hours of free savings (i.e. not usage, only hours we have truly saved by utilizing the cache) on Nx Cloud.
I have sent you an email invite to add you to the Nx Cloud organization.
NOTE: The only thing about the current setup that is not really compatible with Nx is using
:
in the names of "targets". This is because:
is used as the separator between project names and target names when invoking targets. E.g. to run the "bar" target on the project "foo", the full command would be:As well as updating the targets which are immediately being utilized with Nx, I updated all remaining usage of
:
to use-
for consistency.We can evolve this further in future by making more use of
nx affected
and its knowledge of the dependency graph of the monorepo.